Skip to content
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

Define TraceState handling for root spans with NewRootTraceState option #4241

Open
jmacd opened this issue Oct 3, 2024 · 1 comment
Open
Labels
sig-issue A specific SIG should look into this before discussing at the spec spec:trace Related to the specification/trace directory

Comments

@jmacd
Copy link
Contributor

jmacd commented Oct 3, 2024

What are you trying to achieve?

As discussed in #4162, there is a pending sampling specification that defines the use of TraceState set before creating a root span. In some respects, #4162 calls for Trace SDKs to have a mechanism for inserting trace randomness before calling the Sampler, since the SDK knows whether the TraceID is random according to Trace Context Level 2. On the other hand, we know there exist use-cases for consistent sampling using randomness provided by the user, which can support a number of coordinating sampling practices.

The OTel tracing API specification is ambiguous on how a user can modify the TraceState of a root span. It states:

Implementations MUST provide an option to create a `Span` as
a root span, and MUST generate a new `TraceId` for each root span created.
For a Span with a parent, the `TraceId` MUST be the same as the parent.
Also, the child span MUST inherit all `TraceState` values of its parent by default.

IOW it defines what the SDK will do for child spans, not for root spans. On the MUST-provided option to create a new root span, I've seen inconsistency in at least one OTel SDK. When the NewRoot option is selected in OTel-Go, the SDK resets the TraceState to an empty value. Otherwise, it leaves the incoming TraceState of the root context. It is possible to control the TraceState of a root span, except not when using NewRoot.

What did you expect to see?

The API specification should have an explicit option such as NewRootTraceState that sets the TraceState of root spans.
SDKs should use NewRootTraceState if set, otherwise an empty TraceState when creating root spans. The incoming TraceState should not be used implicitly in cases where the TraceState is non-empty and the TraceID is invalid.

@jmacd jmacd added the spec:trace Related to the specification/trace directory label Oct 3, 2024
@danielgblanco danielgblanco added sig-issue A specific SIG should look into this before discussing at the spec triage:accepted:ready-with-sponsor Ready to be implemented and has a specification sponsor assigned labels Oct 7, 2024
@danielgblanco
Copy link
Contributor

@jmacd assuming here as well that the Sampling SIG will be working on this. Thanks.

@danielgblanco danielgblanco removed the triage:accepted:ready-with-sponsor Ready to be implemented and has a specification sponsor assigned label Oct 8, 2024
@mtwo mtwo removed this from 🔭 Main Backlog Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig-issue A specific SIG should look into this before discussing at the spec spec:trace Related to the specification/trace directory
Projects
None yet
Development

No branches or pull requests

2 participants