-
Notifications
You must be signed in to change notification settings - Fork 944
Implement Environment Variable Context Propagation carriers in api/in… #8074
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
Open
Bhagirath00
wants to merge
6
commits into
open-telemetry:main
Choose a base branch
from
Bhagirath00:feature/env-var-propagation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+354
−0
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
df9a72b
Implement Environment Variable Context Propagation carriers in api/in…
Bhagirath00 4cfffac
fix: Align environment carriers with spec requirements
Bhagirath00 b1c2d19
chore: Fix spotless formatting violations
Bhagirath00 c9cdbe4
fix: refactor environment carriers to standard singleton and fix CI f…
Bhagirath00 521c041
[incubator] Add format restriction handling for EnvironmentGetter/Setter
Bhagirath00 b8d7f67
Remove duplicate isValidHttpHeaderValue and fix log message quoting
Bhagirath00 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
91 changes: 91 additions & 0 deletions
91
...incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetter.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.api.incubator.propagation; | ||
|
|
||
| import io.opentelemetry.context.propagation.TextMapGetter; | ||
| import java.util.Collections; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
| import javax.annotation.Nullable; | ||
|
|
||
| /** | ||
| * A {@link TextMapGetter} that extracts context from a map carrier, intended for use with | ||
| * environment variables in child processes. | ||
| * | ||
| * <p>This is useful when a child process needs to extract propagated context from its environment. | ||
| * For example: | ||
| * | ||
| * <pre>{@code | ||
| * Map<String, String> env = System.getenv(); | ||
| * Context context = contextPropagators.getTextMapPropagator() | ||
| * .extract(Context.current(), env, EnvironmentGetter.getInstance()); | ||
| * }</pre> | ||
| * | ||
| * <p>This getter automatically sanitizes keys to match environment variable naming conventions: | ||
| * | ||
| * <ul> | ||
| * <li>Converts keys to uppercase (e.g., {@code traceparent} becomes {@code TRACEPARENT}) | ||
| * <li>Replaces {@code .} and {@code -} with underscores | ||
| * </ul> | ||
| * | ||
| * <p>Values are validated to contain only characters valid in HTTP header fields per <a | ||
| * href="https://datatracker.ietf.org/doc/html/rfc9110#section-5.5">RFC 9110</a> (visible ASCII | ||
| * characters, space, and horizontal tab). Values containing invalid characters are treated as | ||
| * absent and {@code null} is returned. | ||
| * | ||
| * @see <a href= | ||
| * "https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/env-carriers.md#format-restrictions">Environment | ||
| * Variable Format Restrictions</a> | ||
| */ | ||
| public final class EnvironmentGetter implements TextMapGetter<Map<String, String>> { | ||
|
|
||
| private static final Logger logger = Logger.getLogger(EnvironmentGetter.class.getName()); | ||
| private static final EnvironmentGetter INSTANCE = new EnvironmentGetter(); | ||
|
|
||
| private EnvironmentGetter() {} | ||
|
|
||
| /** Returns the singleton instance of {@link EnvironmentGetter}. */ | ||
| public static EnvironmentGetter getInstance() { | ||
| return INSTANCE; | ||
| } | ||
|
|
||
| @Override | ||
| public Iterable<String> keys(Map<String, String> carrier) { | ||
| if (carrier == null) { | ||
| return Collections.emptyList(); | ||
| } | ||
| return carrier.keySet(); | ||
| } | ||
|
|
||
| @Nullable | ||
| @Override | ||
| public String get(@Nullable Map<String, String> carrier, String key) { | ||
| if (carrier == null || key == null) { | ||
| return null; | ||
| } | ||
| // Spec recommends using uppercase and underscores for environment variable | ||
| // names for maximum | ||
| // cross-platform compatibility. | ||
| String sanitizedKey = key.replace('.', '_').replace('-', '_').toUpperCase(Locale.ROOT); | ||
| String value = carrier.get(sanitizedKey); | ||
| if (value != null && !EnvironmentSetter.isValidHttpHeaderValue(value)) { | ||
| logger.log( | ||
| Level.FINE, | ||
| "Ignoring environment variable '{0}': " | ||
| + "value contains characters not valid in HTTP header fields per RFC 9110.", | ||
| sanitizedKey); | ||
| return null; | ||
| } | ||
| return value; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "EnvironmentGetter"; | ||
| } | ||
| } | ||
102 changes: 102 additions & 0 deletions
102
...incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetter.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.api.incubator.propagation; | ||
|
|
||
| import io.opentelemetry.context.propagation.TextMapSetter; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
| import javax.annotation.Nullable; | ||
|
|
||
| /** | ||
| * A {@link TextMapSetter} that injects context into a map carrier, intended for use with | ||
| * environment variables when spawning child processes. | ||
| * | ||
| * <p>This is useful when an application needs to propagate context to sub-processes via their | ||
| * environment. For example, when using {@link ProcessBuilder}: | ||
| * | ||
| * <pre>{@code | ||
| * Map<String, String> env = new HashMap<>(); | ||
| * contextPropagators.getTextMapPropagator().inject(context, env, EnvironmentSetter.getInstance()); | ||
| * ProcessBuilder processBuilder = new ProcessBuilder(); | ||
| * processBuilder.environment().putAll(env); | ||
| * }</pre> | ||
| * | ||
| * <p>This setter automatically sanitizes keys to be compatible with environment variable naming | ||
| * conventions: | ||
| * | ||
| * <ul> | ||
| * <li>Converts keys to uppercase (e.g., {@code traceparent} becomes {@code TRACEPARENT}) | ||
| * <li>Replaces {@code .} and {@code -} with underscores | ||
| * </ul> | ||
| * | ||
| * <p>Values are validated to contain only characters valid in HTTP header fields per <a | ||
| * href="https://datatracker.ietf.org/doc/html/rfc9110#section-5.5">RFC 9110</a> (visible ASCII | ||
| * characters, space, and horizontal tab). Values containing invalid characters are silently | ||
| * skipped. | ||
| * | ||
| * <p><strong>Size limitations:</strong> Environment variable sizes are platform-dependent (e.g., | ||
| * Windows limits name=value pairs to 32,767 characters). Callers are responsible for being aware of | ||
| * platform-specific limits when injecting context. | ||
| * | ||
| * @see <a href= | ||
| * "https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/env-carriers.md#format-restrictions">Environment | ||
| * Variable Format Restrictions</a> | ||
| */ | ||
| public final class EnvironmentSetter implements TextMapSetter<Map<String, String>> { | ||
|
|
||
| private static final Logger logger = Logger.getLogger(EnvironmentSetter.class.getName()); | ||
| private static final EnvironmentSetter INSTANCE = new EnvironmentSetter(); | ||
|
|
||
| private EnvironmentSetter() {} | ||
|
|
||
| /** Returns the singleton instance of {@link EnvironmentSetter}. */ | ||
| public static EnvironmentSetter getInstance() { | ||
| return INSTANCE; | ||
| } | ||
|
|
||
| @Override | ||
| public void set(@Nullable Map<String, String> carrier, String key, String value) { | ||
| if (carrier == null || key == null || value == null) { | ||
| return; | ||
| } | ||
| if (!isValidHttpHeaderValue(value)) { | ||
| logger.log( | ||
| Level.FINE, | ||
| "Skipping environment variable injection for key ''{0}'': " | ||
| + "value contains characters not valid in HTTP header fields per RFC 9110.", | ||
| key); | ||
| return; | ||
| } | ||
| // Spec recommends using uppercase and underscores for environment variable | ||
| // names for maximum | ||
| // cross-platform compatibility. | ||
| String sanitizedKey = key.replace('.', '_').replace('-', '_').toUpperCase(Locale.ROOT); | ||
| carrier.put(sanitizedKey, value); | ||
| } | ||
|
|
||
| /** | ||
| * Checks whether a string contains only characters valid in HTTP header field values per <a | ||
| * href="https://datatracker.ietf.org/doc/html/rfc9110#section-5.5">RFC 9110 Section 5.5</a>. | ||
| * Valid characters are: visible ASCII (0x21-0x7E), space (0x20), and horizontal tab (0x09). | ||
| */ | ||
| static boolean isValidHttpHeaderValue(String value) { | ||
| for (int i = 0; i < value.length(); i++) { | ||
| char ch = value.charAt(i); | ||
| // VCHAR (0x21-0x7E), SP (0x20), HTAB (0x09) | ||
| if (ch != '\t' && (ch < ' ' || ch > '~')) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "EnvironmentSetter"; | ||
| } | ||
| } |
85 changes: 85 additions & 0 deletions
85
...bator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetterTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.api.incubator.propagation; | ||
|
|
||
| import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class EnvironmentGetterTest { | ||
|
|
||
| @Test | ||
| void get() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| carrier.put("TRACEPARENT", "val1"); | ||
| carrier.put("TRACESTATE", "val2"); | ||
| carrier.put("BAGGAGE", "val3"); | ||
| carrier.put("OTHER", "val4"); | ||
|
|
||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "traceparent")).isEqualTo("val1"); | ||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "TRACESTATE")).isEqualTo("val2"); | ||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "Baggage")).isEqualTo("val3"); | ||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "other")).isEqualTo("val4"); | ||
| } | ||
|
|
||
| @Test | ||
| void get_sanitization() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| carrier.put("OTEL_TRACE_ID", "val1"); | ||
| carrier.put("OTEL_BAGGAGE_KEY", "val2"); | ||
|
|
||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "otel.trace.id")).isEqualTo("val1"); | ||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "otel-baggage-key")).isEqualTo("val2"); | ||
| } | ||
|
|
||
| @Test | ||
| void get_null() { | ||
| assertThat(EnvironmentGetter.getInstance().get(null, "key")).isNull(); | ||
| assertThat(EnvironmentGetter.getInstance().get(Collections.emptyMap(), null)).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void keys() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| carrier.put("K1", "V1"); | ||
| carrier.put("K2", "V2"); | ||
|
|
||
| assertThat(EnvironmentGetter.getInstance().keys(carrier)).containsExactlyInAnyOrder("K1", "K2"); | ||
| assertThat(EnvironmentGetter.getInstance().keys(null)).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void get_validHeaderValues() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| carrier.put("KEY1", "simple-value"); | ||
| carrier.put("KEY2", "value with spaces"); | ||
| carrier.put("KEY3", "value\twith\ttabs"); | ||
|
|
||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "key1")).isEqualTo("simple-value"); | ||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "key2")).isEqualTo("value with spaces"); | ||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "key3")).isEqualTo("value\twith\ttabs"); | ||
| } | ||
|
|
||
| @Test | ||
| void get_invalidHeaderValues() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| carrier.put("KEY1", "value\u0000with\u0001control"); | ||
| carrier.put("KEY2", "value\nwith\nnewlines"); | ||
| carrier.put("KEY3", "value\u0080non-ascii"); | ||
|
|
||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "key1")).isNull(); | ||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "key2")).isNull(); | ||
| assertThat(EnvironmentGetter.getInstance().get(carrier, "key3")).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void testToString() { | ||
| assertThat(EnvironmentGetter.getInstance().toString()).isEqualTo("EnvironmentGetter"); | ||
| } | ||
| } |
76 changes: 76 additions & 0 deletions
76
...bator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetterTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.api.incubator.propagation; | ||
|
|
||
| import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class EnvironmentSetterTest { | ||
|
|
||
| @Test | ||
| void set() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| EnvironmentSetter.getInstance().set(carrier, "traceparent", "val1"); | ||
| EnvironmentSetter.getInstance().set(carrier, "TRACESTATE", "val2"); | ||
| EnvironmentSetter.getInstance().set(carrier, "Baggage", "val3"); | ||
|
|
||
| assertThat(carrier).containsEntry("TRACEPARENT", "val1"); | ||
| assertThat(carrier).containsEntry("TRACESTATE", "val2"); | ||
| assertThat(carrier).containsEntry("BAGGAGE", "val3"); | ||
| } | ||
|
|
||
| @Test | ||
| void set_sanitization() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| EnvironmentSetter.getInstance().set(carrier, "otel.trace.id", "val1"); | ||
| EnvironmentSetter.getInstance().set(carrier, "otel-baggage-key", "val2"); | ||
|
|
||
| assertThat(carrier).containsEntry("OTEL_TRACE_ID", "val1"); | ||
| assertThat(carrier).containsEntry("OTEL_BAGGAGE_KEY", "val2"); | ||
| } | ||
|
|
||
| @Test | ||
| void set_null() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| EnvironmentSetter.getInstance().set(null, "key", "val"); | ||
| EnvironmentSetter.getInstance().set(carrier, null, "val"); | ||
| EnvironmentSetter.getInstance().set(carrier, "key", null); | ||
| assertThat(carrier).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void set_validHeaderValues() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| // Printable ASCII and tab are valid per RFC 9110 | ||
| EnvironmentSetter.getInstance().set(carrier, "key1", "simple-value"); | ||
| EnvironmentSetter.getInstance().set(carrier, "key2", "value with spaces"); | ||
| EnvironmentSetter.getInstance().set(carrier, "key3", "value\twith\ttabs"); | ||
|
|
||
| assertThat(carrier).containsEntry("KEY1", "simple-value"); | ||
| assertThat(carrier).containsEntry("KEY2", "value with spaces"); | ||
| assertThat(carrier).containsEntry("KEY3", "value\twith\ttabs"); | ||
| } | ||
|
|
||
| @Test | ||
| void set_invalidHeaderValues() { | ||
| Map<String, String> carrier = new HashMap<>(); | ||
| // Control characters and non-ASCII are invalid per RFC 9110 | ||
| EnvironmentSetter.getInstance().set(carrier, "key1", "value\u0000with\u0001control"); | ||
| EnvironmentSetter.getInstance().set(carrier, "key2", "value\nwith\nnewlines"); | ||
| EnvironmentSetter.getInstance().set(carrier, "key3", "value\rwith\rcarriage"); | ||
| EnvironmentSetter.getInstance().set(carrier, "key4", "value\u0080non-ascii"); | ||
|
|
||
| assertThat(carrier).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void testToString() { | ||
| assertThat(EnvironmentSetter.getInstance().toString()).isEqualTo("EnvironmentSetter"); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since EnvironmentGetter reads from the environment, keeping a copy, shouldn't this be
toLowerCaseinstead oftoUpperCase? Upper case I don't think will end up matching header values since the spec for w3c for example istraceparentnotTRACEPARENT. environment variables in the environment should be uppercase, _ separated, but to auto be mapped to the w3c spec the normalized to lower.Hopefully this makes sense, and hopefully I'm reading this right. I haven't touched Java in a long time.