feat(propagator-env-carrier): add environment variable carrier helpers by pellared · Pull Request #6774 · open-telemetry/opentelemetry-js

pellared · 2026-05-28T15:30:02Z

Which problem is this PR solving?

Short description of the changes

Implement environment variable carrier following https://opentelemetry.io/docs/specs/otel/context/env-carriers/ and open-telemetry/opentelemetry-specification#5102

This is important for interoperability with other languages (https://github.com/open-telemetry/opentelemetry-specification/blob/main/spec-compliance-matrix.md?plain=1#L254) and tooling.

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Example usage from README.md.

Checklist:

Followed the style guidelines of this project
Unit tests have been added
Documentation has been updated

codecov · 2026-05-28T15:33:47Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.94%. Comparing base (c0bfea7) to head (5e21cb4).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6774      +/-   ##
==========================================
+ Coverage   94.92%   94.94%   +0.01%     
==========================================
  Files         377      378       +1     
  Lines       12817    12858      +41     
  Branches     2924     2932       +8     
==========================================
+ Hits        12167    12208      +41     
  Misses        650      650

Files with missing lines	Coverage Δ
...elemetry-propagator-env-carrier/src/env-carrier.ts	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pellared · 2026-05-28T16:00:02Z

CC @open-telemetry/semconv-cicd-approvers

maryliag

Thank you for working on this! Added a few questions/comments

maryliag · 2026-06-03T12:04:25Z

+    }
+  }
+
+  get(_carrier: unknown, key: string): string | undefined {


can you just not have _carrier here and below if the function don't use them? If you need this format, consider using never as type, so users don't try to pass something

Addressed in 2eb8181

I switched EnvironmentGetter to TextMapGetter<void>. That still prevents callers from passing an object directly, but avoids the awkward undefined as never pattern. This also follows what is already done in

opentelemetry-js/experimental/packages/shim-opencensus/src/propagation.ts

Lines 12 to 24 in 8e4d4ed

class Getter implements TextMapGetter<void> {

private ocGetter: oc.HeaderGetter;

constructor(ocGetter: oc.HeaderGetter) {

this.ocGetter = ocGetter;

}

keys(): string[] {

return [];

}

get(carrier: void, key: string) {

return this.ocGetter.getHeader(key);

}

}

maryliag · 2026-06-03T12:06:04Z

+ */
+export class EnvironmentSetter implements TextMapSetter<EnvironmentCarrierMap> {
+  set(carrier: EnvironmentCarrierMap, key: string, value: string): void {
+    carrier[normalizeKey(key)] = value;


the set uses local carrier, and the get uses the global, this opens for chances of the global one not getting updated and the get not working as expected.

this opens for chances of the global one not getting updated

This is correct (sic!). This is to follow https://opentelemetry.io/docs/specs/otel/context/env-carriers/#operational-guidance and https://opentelemetry.io/docs/specs/otel/context/env-carriers/#process-spawning.

Also see https://opentelemetry.io/docs/specs/otel/context/env-carriers/#supplementary-guidelines

maryliag · 2026-06-03T12:12:12Z

+
+      const getter = new EnvironmentGetter();
+
+      assert.deepStrictEqual(getter.keys({}), ['TRACEPARENT']);


can we have documented somewhere about which one takes precedent? So users can expect the same results if running the test multiple times
right now is dependent on the order to which the key was set, if that is indeed the expectation, make it clear, or change to something that always give the same results, such as returning the keys in alphabetical order

Good call. I strongly believe we have agreed in some reviews or maybe even Spec SIG call that this is an error scenario (name collision) and the behavior is not specified for this case. I think it would be good to cover call it out explicitly in the specification and document it here in EnvironmentGetter. Thoughts?

Improved the docs 5a67cf4

maryliag · 2026-06-03T12:18:15Z

+  const originalEnv = { ...process.env };
+
+  beforeEach(() => {
+    clearEnv();


no need to delete twice, you're already deleting at the end of each test

Note that env vars such as TRACEPARENT can be used by CI/CD automation. For instance GitHub Actions may use it in future and it may make these tests flaky. Therefore, I think it is safer to even clear before the test. Side note: one of the main reason why env var carrier was added was the work of the CI/CD SIG.

in that case, I suggest adding env names that you know won't conflict with pre existing values

What about integration tests under describe('propagator integration' ? Do we want to remove them? Note that there are similar tests in https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/shim-opencensus/test/propagation.test.ts

no need to remove the test, but I want to make them deterministic, if there is a chance an env variable changed outside of the tests could affect the results and cause flaky tests, we want to avoid that.
Even if you delete TRACEPARENT before a test, but you say that other processes might be using it, there is no guarantee that in between the before and the actual test, that value can be added to the env and the test fails.

The majority of your test don't need to use the env TRACEPARENT, do they? you can call yabadabadoo and should still work 😄

For the test of propagator integration, seems like that is the env variable you want to test, so you can leave it there, and if we ever get flaky tests we can decide what to do

pellared · 2026-06-03T19:56:34Z

+/**
+ * TextMapSetter that writes propagation values to an environment map.
+ */
+export class EnvironmentSetter implements TextMapSetter<EnvironmentCarrierMap> {


I think it is safer to have EnvironmentSetter implements TextMapSetter<void> and accept Record<string, string> or EnvironmentCarrierMap in the constructor. This follows the pattern used in

opentelemetry-js/experimental/packages/shim-opencensus/src/propagation.ts

Lines 26 to 36 in 8e4d4ed

class Setter implements TextMapSetter<void> {

private ocSetter: oc.HeaderSetter;

constructor(ocSetter: oc.HeaderSetter) {

this.ocSetter = ocSetter;

}

set(carrier: void, key: string, value: string): void {

this.ocSetter.setHeader(key, value);

}

}

and it does not force users to think what type of carrier they need to pass to the propagator.

See test scenarios it('should inject for reference.

@maryliag, thoughts?

I think this is better 5e21cb4

maryliag · 2026-06-05T20:17:43Z

+  const originalEnv = { ...process.env };
+
+  beforeEach(() => {
+    clearEnv();


no need to remove the test, but I want to make them deterministic, if there is a chance an env variable changed outside of the tests could affect the results and cause flaky tests, we want to avoid that.
Even if you delete TRACEPARENT before a test, but you say that other processes might be using it, there is no guarantee that in between the before and the actual test, that value can be added to the env and the test fails.

The majority of your test don't need to use the env TRACEPARENT, do they? you can call yabadabadoo and should still work 😄

For the test of propagator integration, seems like that is the env variable you want to test, so you can leave it there, and if we ever get flaky tests we can decide what to do

maryliag · 2026-06-05T20:27:22Z

+const ASCII_UNDERSCORE = '_'.charCodeAt(0);
+const ASCII_CASE_OFFSET = ASCII_LOWER_A - ASCII_UPPER_A;
+
+/*


Suggested change

/*

/**

nit

feat(propagator-env-carrier): add environment variable carrier helpers

6b05f66

pellared requested a review from a team as a code owner May 28, 2026 15:30

pellared marked this pull request as draft May 28, 2026 15:30

update changelog

edaf052

pellared added 4 commits May 28, 2026 17:36

clarify normalization

a5c1600

refactor and add test cases

cac39b2

rename file

660f800

refactor normalizeKey

419c3f3

pellared marked this pull request as ready for review May 28, 2026 15:59

kamphaus reviewed May 31, 2026

View reviewed changes

Comment thread experimental/packages/opentelemetry-propagator-env-carrier/src/env-carrier.ts

comment normalizeKey function

39b9eca

pellared requested a review from kamphaus June 1, 2026 11:44

kamphaus approved these changes Jun 1, 2026

View reviewed changes

maryliag reviewed Jun 3, 2026

View reviewed changes

pichlermarc self-assigned this Jun 3, 2026

change unknown to void

2eb8181

pellared commented Jun 3, 2026

View reviewed changes

pellared added 2 commits June 3, 2026 22:12

improve docs

5a67cf4

simplify setter carrier

5e21cb4

pellared requested a review from maryliag June 3, 2026 20:27

maryliag reviewed Jun 5, 2026

View reviewed changes

	class Getter implements TextMapGetter<void> {
	private ocGetter: oc.HeaderGetter;

	constructor(ocGetter: oc.HeaderGetter) {
	this.ocGetter = ocGetter;
	}
	keys(): string[] {
	return [];
	}
	get(carrier: void, key: string) {
	return this.ocGetter.getHeader(key);
	}
	}


		const getter = new EnvironmentGetter();

		assert.deepStrictEqual(getter.keys({}), ['TRACEPARENT']);

	class Setter implements TextMapSetter<void> {
	private ocSetter: oc.HeaderSetter;

	constructor(ocSetter: oc.HeaderSetter) {
	this.ocSetter = ocSetter;
	}

	set(carrier: void, key: string, value: string): void {
	this.ocSetter.setHeader(key, value);
	}
	}

Conversation

pellared commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem is this PR solving?

Short description of the changes

Type of change

How Has This Been Tested?

Checklist:

Uh oh!

codecov Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pellared commented May 28, 2026

Uh oh!

Uh oh!

maryliag left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pellared Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pellared Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pellared Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pellared Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pellared commented May 28, 2026 •

edited

Loading

codecov Bot commented May 28, 2026 •

edited

Loading

pellared Jun 3, 2026 •

edited

Loading

pellared Jun 3, 2026 •

edited

Loading

pellared Jun 3, 2026 •

edited

Loading

pellared Jun 3, 2026 •

edited

Loading