Skip to content

Mark env var carrier as RC#5142

Open
pellared wants to merge 3 commits into
open-telemetry:mainfrom
pellared:env-car-norm-collision
Open

Mark env var carrier as RC#5142
pellared wants to merge 3 commits into
open-telemetry:mainfrom
pellared:env-car-norm-collision

Conversation

@pellared

@pellared pellared commented Jun 9, 2026

Copy link
Copy Markdown
Member

Towards #5040

Please double-check: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/env-carriers.md

Issue tracking the implementations: #4771

Note that this is already implemented in 7 languages (Java, .NET, C++, Go, Python, Swift, JavaScript) and 2 is in progress with approvals (Rust).

Also see #5040 (comment)

@pellared pellared marked this pull request as ready for review June 9, 2026 07:45
@pellared pellared requested a review from a team as a code owner June 9, 2026 07:45
@pellared

pellared commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

CC @open-telemetry/semconv-cicd-approvers

@marcalff marcalff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some concerns about this.

First, process wise, this is too soon.

The spec just was changed last week with PR #5102.
Not every SIG implementation had a chance to implement it yet, and in fact, we found issues with it in C++, leading to the second point.

Secondly, the spec mandates that if a given normalized key (TRACEPARENT) is not found, then the code should:

  • iterate on all keys in the carrier (i.e., inspect every environment variable)
  • normalize the env var name
  • compare the normalized name to the key searched
  • return the value for the key found.

This has two consequences:

  • (a) the SDK is very inefficient. When there are 200 env vars present, everyone of them must be inspected, and this each time a key is looked up.
  • (b) the SDK is unpredictable and non deterministic in case of multiple entries found.

For example, the implementations I have seen would return the first key that matches, so if the following keys are present:

  • traceparent=11,
  • TraceParent=22,
  • TrAcEpArEnT=33

the value returned by implementations (Java for example) are random, and this happens silently.

My proposal is to remove this specific clause from the spec.
When searching for traceparent, only lookup the normalized key (TRACEPARENT), and not consider anything else.

This will improve performances, and make propagation deterministic.

The rest of the spec, including normalizing keys on Get/Set, looks ok.

@marcalff

marcalff commented Jun 9, 2026

Copy link
Copy Markdown
Member

@pellared

pellared commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

@marcalff, thanks! Can you please create a new issue out of #5142 (review) so that we can mark this PR as blocked by it and address it in a separate thread?

@pellared

pellared commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

Marking as draft until this PR is unblocked

@pellared pellared marked this pull request as draft June 9, 2026 08:34
@pellared

pellared commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

First, process wise, this is too soon.

@open-telemetry/semconv-cicd-approvers, @open-telemetry/technical-committee, @open-telemetry/governance-committee

I propose to have this PR merged and additionally create a blog post just before KubeCon NA 2026 (e.g. beginning of November).

@marcalff

marcalff commented Jun 9, 2026

Copy link
Copy Markdown
Member

Related issue, to resolve (one way or another) before stability:

@marcalff marcalff self-requested a review June 12, 2026 22:05
@marcalff

Copy link
Copy Markdown
Member

Now that #5143 is resolved and merged, this change is no longer blocked.

@pellared pellared marked this pull request as ready for review June 16, 2026 14:55
@pellared

pellared commented Jun 16, 2026

Copy link
Copy Markdown
Member Author

SIG meeting: It would be good to have approvals from maintainers of languages that support this.

PTAL @open-telemetry/swift-core-maintainers @open-telemetry/dotnet-maintainers @open-telemetry/python-maintainers @open-telemetry/rust-maintainers @open-telemetry/javascript-maintainers

@pellared

Copy link
Copy Markdown
Member Author

@pellared

Copy link
Copy Markdown
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.