Skip to content

Clean up implementation guidelines for environment variable propagation carriers#5166

Merged
pellared merged 11 commits into
open-telemetry:mainfrom
pellared:sup-guide-updt
Jun 23, 2026
Merged

Clean up implementation guidelines for environment variable propagation carriers#5166
pellared merged 11 commits into
open-telemetry:mainfrom
pellared:sup-guide-updt

Conversation

@pellared

@pellared pellared commented Jun 19, 2026

Copy link
Copy Markdown
Member

Address #5165 (comment)

This PR cleans up the non-normative implementation guidance for environment variable propagation carriers.

It keeps the guidance aligned with #5144 by removing implementation-shape bullets whose Getter guidance was no longer accurate after the specification was updated to require environment variable carriers to read and return only normalized environment variable names.

Motivation

#5144 changed the expected behavior for environment variable carrier lookup: Get normalizes the key requested by the propagator and reads that normalized environment variable name directly, while Keys returns only names that are already normalized.

After that change, review feedback in #5165 called out that the previous supplementary guidance could be misleading. In particular, the Getter bullet suggested creating an in-memory copy of all current environment variables before reading context from it. That pattern can imply full environment enumeration, which is exactly what #5144 was intended to avoid for extraction. It also made the guidance sound more prescriptive than it needed to be, even though different language implementations expose environment-variable propagation through different carrier, getter, setter, or helper shapes.

Changes

  • Rename "Supplementary Guidelines" to "Implementation Guidelines" so the section title better matches its purpose.
  • Update the implementation-shape bullets to follow env var carriers to only read and return normalized names #5144 and remove whose remaining content repeated the surrounding text.
  • Reword "Examples" to "Example implementations".
  • Add the OpenTelemetry JavaScript implementation to the example list.

Notes

This does not add new normative requirements. It only updates explanatory guidance so it does not contradict or over-specify the behavior introduced by #5144.

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

This PR updates the non-normative documentation for environment-variable context propagation carriers to stay aligned with the normalization behavior introduced in #5144, removing potentially misleading implementation-shape guidance and refreshing examples.

Changes:

  • Renames “Supplementary Guidelines” to “Implementation Guidelines” and adjusts surrounding wording for clarity.
  • Removes outdated/misleading implementation-shape bullets (notably around Getter) to avoid implying full environment enumeration during extraction.
  • Expands the example implementation list to include OpenTelemetry JavaScript, and records the doc change in the changelog.

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 Renames and streamlines non-normative implementation guidance; updates example implementation links (adds JS).
CHANGELOG.md Adds an Unreleased changelog entry describing the documentation cleanup.

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

Comment thread CHANGELOG.md Outdated
@pellared pellared changed the title Cleanup implementation guidelines for environment variable propagation carriers Clean up implementation guidelines for environment variable propagation carriers Jun 19, 2026
@pellared

Copy link
Copy Markdown
Member Author

@open-telemetry/semconv-cicd-approvers, @maryliag, @jack-berg, @marcalff, @xrmx, @pichlermarc, @martincostello, PTAL

Comment thread specification/context/env-carriers.md
@pellared
pellared requested a review from kamphaus June 19, 2026 08:38
Comment thread specification/context/env-carriers.md Outdated
@pellared
pellared enabled auto-merge June 23, 2026 11:12
@pellared
pellared added this pull request to the merge queue Jun 23, 2026
Merged via the queue into open-telemetry:main with commit 049a2d2 Jun 23, 2026
8 of 9 checks passed
@pellared
pellared deleted the sup-guide-updt branch June 23, 2026 11:13
pellared added a commit to pellared/opentelemetry-specification that referenced this pull request Jun 29, 2026
…y#5179)

## Changes

This PR removes the non-normative implementation guidance that suggested
environment variable propagation carriers could cache environment
variables or individual `Get` lookup results.

The environment carrier specification already frames environment
variables as process-startup input: applications should extract context
during initialization, then continue using the extracted `Context`. With
that usage model, the specification does not need to recommend caching
as an implementation pattern. Implementations can still use owned
storage or explicit snapshots where their language API requires it, but
that should be a language-specific design choice rather than general
spec guidance.

Non-caching implementations so far:
- Go:
https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/propagators/envcar/carrier.go
- Swift:
https://github.com/open-telemetry/opentelemetry-swift-core/blob/main/Sources/OpenTelemetrySdk/Trace/Propagation/EnvironmentContextPropagator.swift
- JavaScript:
open-telemetry/opentelemetry-js#6853

## Motivation

This follows the same direction as the discussion that led to open-telemetry#5143 and
open-telemetry#5144. The concern raised in the open-telemetry#5142 review was that environment
carrier extraction should not require inspecting every environment
variable. Full-carrier enumeration is inefficient, especially when
OpenTelemetry is present but no environment propagation variables are
configured.

open-telemetry#5144 addressed that by requiring `Get` to normalize the requested
propagator key and read only that normalized environment variable name.
`Keys` remains the operation that may enumerate keys, and it returns
only names that are already normalized.

The caching examples added in open-telemetry#5166 can be read as encouraging
implementations to reintroduce hidden first-use state, such as:

- loading the whole environment into a cache during initialization or on
first use, or
- caching individual `Get` results by normalized key.

That guidance is unnecessary and can work against the intent of
open-telemetry#5143/open-telemetry#5144:

- Standard extraction typically reads only a small set of keys such as
`TRACEPARENT`, `TRACESTATE`, and `BAGGAGE`.
- Repeated extraction from the process environment is not the intended
hot path; users should extract once at startup and reuse the resulting
`Context`.
- Whole-environment snapshots can cost more than direct lookup for the
common extraction path.
- Hidden caches can make later environment changes invisible in a way
that is surprising unless the API explicitly describes itself as a
snapshot.

This also aligns with
[Performance](https://opentelemetry.io/community/mission/#we-value-performance),
which is a core OpenTelemetry Engineering Value.

The Go PR reviews showed the same issue in an implementation: review
feedback called out that `Get` should do a direct lookup of the
normalized key and leave enumeration to `Keys`, instead of enumerating
and storing the process environment on the first `Get`.
-
open-telemetry/opentelemetry-go-contrib#9112 (comment)
-
open-telemetry/opentelemetry-go-contrib#9112 (comment)

Also see:
open-telemetry#5142 (comment)

## Compatibility

This PR does not add or remove normative requirements. It only removes
non-normative guidance that could steer language implementations toward
default caching.

Language implementations remain free to use internal storage when
required by their API shape, and users can still choose explicit
snapshots or caller-provided maps where that is the clearer model. The
specification should avoid presenting caching as the recommended default
for environment carrier extraction.

## Related

- open-telemetry#5142
- open-telemetry#5143
- open-telemetry#5144
- open-telemetry#5166
- open-telemetry#5040
- open-telemetry#4771
@carlosalberto carlosalberto mentioned this pull request Jul 8, 2026
pull Bot pushed a commit to CodeWeaver13/opentelemetry-specification that referenced this pull request Jul 10, 2026
### Context

- Clarify that environment variable propagation operational guidance is
  non-normative and should be documented by language implementations.

([open-telemetry#5165](open-telemetry#5165))
- Clean up implementation guidelines for environment variable
propagation
  carriers.

([open-telemetry#5166](open-telemetry#5166))
- Remove caching behavior suggestions for environment variable
propagation
  carrier implementations.

([open-telemetry#5179](open-telemetry#5179))
- Mark "Environment Variables as Context Propagation Carriers" as
Release
  Candidate.

([open-telemetry#5142](open-telemetry#5142))

### Traces

- Amend the description of Composite/Composable samplers.

([open-telemetry#5161](open-telemetry#5161))

### Logs

- Add ETW (Event Tracing for Windows) example mapping to the logs data
  model appendix, including the ETW level to `SeverityNumber` mapping.

([open-telemetry#5159](open-telemetry#5159))

### Profiles

- Add Profiles data model (data-model.md).

([open-telemetry#4965](open-telemetry#4965))

### Supplementary Guidelines

- Add non-normative supplementary guidelines for SDK self-observability.

([open-telemetry#5135](open-telemetry#5135))

### OTEPs

- Add OTEP proposing a central OpenTelemetry benchmarks repository.

([open-telemetry#5118](open-telemetry#5118))
- Introduce Policies into the specification.

([open-telemetry#4738](open-telemetry#4738))

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.

8 participants