-
Notifications
You must be signed in to change notification settings - Fork 894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stabilize trace context in non-OTLP formats and define casing #3909
Stabilize trace context in non-OTLP formats and define casing #3909
Conversation
1cb3d02
to
e2edbe9
Compare
90e22de
to
07f2982
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep this open for a while to ensure visibility.
@jmacd raised one potential concern: We want to add some OpenTracing fields (sampling priority?) to this specification, which have dots in them. This would potentially leave us with some fields with dots, and others with underscores. Josh, if there are related issues/PRs, feel free to add them here. |
Can you clarify which OpenTracing fields? Dots are totally fine at namespace separators. We use underscores as word separators. |
I have a couple of concern about the absence of an explicit statement about Here's what that looks like, for example: open-telemetry/semantic-conventions#793 In this case, I have work-in-progress to perform trace sampling based on OTEP 235 (https://github.com/open-telemetry/oteps/blob/main/text/trace/0235-sampling-threshold-in-trace-state.md) inside the collector-contrib probabilistic sampler processor, which applies to both spans and logs. What this means is that the sampler for logs will not try to encode sampling probability using an equivalent to I propose this change before stabilizing: #3923 |
Related: #3924 |
@carlosalberto please see #3923 (comment), thanks! |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
@open-telemetry/technical-committee, is this blocked by anything? |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Ping @jsuereth @arminru @bogdandrutu @tigrannajaryan @jack-berg @pellared (who has previously approved this PR). |
@dashpole It seems only minor edits are needed before we can (finally) merge this ;) |
b2a9725
to
7a9de42
Compare
Co-authored-by: Armin Ruech <[email protected]>
…elemetry#3909) Closes open-telemetry#3303 ## Changes From @tigrannajaryan in open-telemetry#3303 (comment): > I think the conclusion in open-telemetry#3518 was that we keep the names. It has been a while and there is no new evidence that we need to do something else so I suggest that we submit a PR that declares https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/compatibility/logging_trace_context.md "Stable" and closes this issue. This marks the specification for using `trace_id`, `span_id`, and `trace_flags` in non-OTLP logging formats stable.
Closes #3303
Changes
From @tigrannajaryan in #3303 (comment):
This marks the specification for using
trace_id
,span_id
, andtrace_flags
in non-OTLP logging formats stable.@jack-berg