oteps icon indicating copy to clipboard operation
oteps copied to clipboard

[otep] Propose adding env variables as context carriers to specification

Open adrielp opened this issue 1 year ago • 1 comments

Based on conversations last week in the Specification and Semantic Conventions SIGs, I'm opening this duplicate pull request which was originally set as a Draft and hasn't had movement since last November.

There are real use cases that are coming to fruiting, namely in the CI/CD working group, that will benefit from this being accepted. Once accepted we can work on getting the specification added for both general context propagation and baggage.

On the note of baggage; baggage is a form of context propagation and was not originally mentioned directly by name in this OTEP. It is however, absolutely essential. I've had the pleasure of prototyping out tracing within an OpenTofu controller system where context on available in parent/child at the very start of the trace was available. Baggage was the means of transferring this critical context to subsequent siblings that would've not had it otherwise.

Thanks for all the hard work to the original author (@deejgregor) and opening the draft #241

CC. TC sponsors @jsuereth @carlosalberto

adrielp avatar May 25 '24 23:05 adrielp

@adrielp thank you for picking up this work. I appreciate that while I’ve been going through a busy time (and not sure when it will wrap up).

deejgregor avatar May 25 '24 23:05 deejgregor

Overall LGTM, albeit I'd like to have a pair of things called out:

  • On propagating other formats: we are taking for granted we will only want to propagate traceparent (tracecontext), whereas there is a small chance that for legacy reasons b3 or similar is supported in a given environment, hence:
  • Suggest/standardize even the usage of TRACEPARENT, but allow the user to override the environment variable name (useful for overriding purposes + allowing the easy usage of any legacy format).
    • I believe this is mentioned as part of the "Changed" section, but a clearer note would help IMHO.

carlosalberto avatar Oct 08 '24 14:10 carlosalberto

Overall LGTM, albeit I'd like to have a pair of things called out:

  • On propagating other formats: we are taking for granted we will only want to propagate traceparent (tracecontext), whereas there is a small chance that for legacy reasons b3 or similar is supported in a given environment, hence:

  • Suggest/standardize even the usage of TRACEPARENT, but allow the user to override the environment variable name (useful for overriding purposes + allowing the easy usage of any legacy format).

    • I believe this is mentioned as part of the "Changed" section, but a clearer note would help IMHO.

I've adjust the changed section slightly. Let me know if that tracks with what you were hoping for.

adrielp avatar Oct 15 '24 14:10 adrielp

Added some minor comments.

Approving under the assumption that traceparent and tracestate format is intended to be identical to the W3C Trace-Context spec (and that we document it explicitly)

@lmolkova - after the SIG, I pushed another update which hopefully makes this more clear within the document. Please feel free to give it another review and let me know if the changes do indeed add that clarity.

adrielp avatar Oct 15 '24 16:10 adrielp