Skip to content

env var carriers to only read and return normalized names#5144

Merged
pellared merged 7 commits into
open-telemetry:mainfrom
pellared:env-car-normalizedo-only
Jun 12, 2026
Merged

env var carriers to only read and return normalized names#5144
pellared merged 7 commits into
open-telemetry:mainfrom
pellared:env-car-normalizedo-only

Conversation

@pellared

@pellared pellared commented Jun 9, 2026

Copy link
Copy Markdown
Member

Fixes #5143

Prototypes:

Rationale

The previous wording required environment variable carriers to normalize every key present in the carrier before matching a requested propagation key.

This PR removes that requirement for two reasons:

  • Efficiency: extraction should not require reading every environment variable
  • Determinism: if multiple environment variable names normalize to the same key, selecting one is ambiguous and implementation-dependent.

Instead, carriers normalize the key requested by the propagator and read only the corresponding normalized environment variable name. Keys similarly returns only environment variable names that are already normalized.

This keeps injection behavior unchanged while making extraction predictable and avoiding unnecessary environment enumeration.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the environment-variable carrier specification to avoid requiring environment enumeration during extraction by defining that carriers only operate on normalized environment variable names (for reads and key listing), addressing the efficiency and determinism concerns raised in issue #5143.

Changes:

  • Redefine env-var carrier Get behavior to read only the normalized key name (no matching against non-normalized env var names).
  • Redefine Keys behavior to return only already-normalized environment variable names.
  • Add a changelog entry documenting this specification clarification.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
specification/context/env-carriers.md Tightens normalization semantics so Get reads only normalized env var names and Keys returns only already-normalized names.
CHANGELOG.md Records the env-var carrier normalization behavior change in the Unreleased Context section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread specification/context/env-carriers.md Outdated
@pellared
pellared marked this pull request as ready for review June 9, 2026 10:17
@pellared
pellared requested a review from a team as a code owner June 9, 2026 10:17
@pellared
pellared marked this pull request as draft June 9, 2026 10:18
@pellared
pellared marked this pull request as ready for review June 9, 2026 10:20
@pellared

pellared commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

CC @open-telemetry/semconv-cicd-approvers @marcalff @jack-berg

Comment thread specification/context/env-carriers.md
Comment thread specification/context/env-carriers.md

@adrielp adrielp 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.

Just left a couple comments.

@pellared
pellared requested a review from adrielp June 9, 2026 11:51

@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.

LGTM,

Thanks for supporting this simplification.

Comment thread specification/context/env-carriers.md
Comment thread specification/context/env-carriers.md Outdated
@jack-berg

jack-berg commented Jun 9, 2026

Copy link
Copy Markdown
Member

This PR removes that requirement for two reasons:

Efficiency: extraction should not require reading every environment variable

Check out the java implementation of this. Some notes:

  • get has an optimistic lookup against the single requested key. It only reads additional env vars if that optimistic lookup fails, and even so, exits early once it finds a match. The optimistic lookup should be common, I think.
  • keys has no choice but to iterate through all env vars and normalize, but keys in not commonly called. The only built-in propagators which use it are the depreceated ottrace and jaeger propagators.
  • There's an additional optimization available in the normalize function itself. Currently, java assumes that normalization is needed. It could instead do a first pass to discover if any keys are present which require normalization. If not, avoid allocation.

Finally, starting a new process that needs env var context propagation seems like orders of magnitude more resource utilization that what we're talking about for reading / normalizing all env vars. Does this really matter?

Determinism: if multiple environment variable names normalize to the same key, selecting one is ambiguous and implementation-dependent.

Does this solve that?

The real inefficiency comes from keys(), which in the current implementation has to iterate / normalize all

@pellared

Copy link
Copy Markdown
Member Author

@jack-berg, regarding #5144 (comment) I suggest continuing the discussion in #5143. Notably please check #5143 (comment) which should answer your concern.

This was referenced Jul 17, 2026
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.

Propagation: avoid env carrier enumeration