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

Support TraceStates on SamplingResults #2974

Closed
spencerwilson opened this issue May 11, 2022 · 5 comments · Fixed by #3530
Closed

Support TraceStates on SamplingResults #2974

spencerwilson opened this issue May 11, 2022 · 5 comments · Fixed by #3530
Labels
sdk:traces Issues and PRs related to the Traces SDK spec-feature This is a request to implement a new feature which is already specified by the OTel specification

Comments

@spencerwilson
Copy link
Contributor

spencerwilson commented May 11, 2022

Spec v0.7.0 added the expectation that SamplingResult includes a TraceState. Current spec text: A SamplingResult contains...

A Tracestate that will be associated with the Span through the new SpanContext. If the sampler returns an empty Tracestate here, the Tracestate will be cleared, so samplers SHOULD normally return the passed-in Tracestate if they do not intend to change it.

Task: Support ^ by adding a property traceState?: TraceState to SamplingResult (src). Backwards compatibility: If a SamplingResult lacks traceState, assume the value is "current trace state; no changes". Update Tracer to implement the trace state updating described by the spec.

References:

@legendecas legendecas transferred this issue from open-telemetry/opentelemetry-js-api May 16, 2022
@legendecas
Copy link
Member

legendecas commented May 16, 2022

I moved this issue since this is part of the SDK (though the Sampler interface is not moved to this repo yet, tracking in #2961).

@dyladan
Copy link
Member

dyladan commented May 16, 2022

thanks. I was just looking through our labels and we don't have a "required by spec" label. this isn't really a bug because it is working as designed, but the spec changed. Create a new label? Can probably consolidate spec-v1 and spec-v1.1 labels into it.

@legendecas
Copy link
Member

Create a new label? Can probably consolidate spec-v1 and spec-v1.1 labels into it.

Both SGTM. Though, I don't find the semver-minor to be very clear as it has reached 1.9.0 already and our API package is not following the semantics.

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@legendecas
Copy link
Member

Not stale, need to be addressed after landing #3088.

@legendecas legendecas added the sdk:traces Issues and PRs related to the Traces SDK label Sep 1, 2022
@pichlermarc pichlermarc added the spec-feature This is a request to implement a new feature which is already specified by the OTel specification label Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sdk:traces Issues and PRs related to the Traces SDK spec-feature This is a request to implement a new feature which is already specified by the OTel specification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants