Skip to content

Update Env var getter / setter to reflect spec changes #8233

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.

Sign up for GitHub

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
jack-berg wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Update Env var getter / setter to reflect spec changes #8233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9dd9717
Update Env var getter / setter to reflect spec changes
jack-berg Mar 31, 2026
222c4f3
checkstyle
jack-berg Mar 31, 2026
d05e9db
EnvironmentGetter.keys() normalizes values
jack-berg Apr 3, 2026
ff9c5bd
Nomralize results of keys(), log a one time warning
jack-berg Apr 3, 2026
f829e51
Normalize carrier keys in get such all results of keys() return value…
jack-berg Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 21 additions & 18 deletions ...incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -46,7 +46,8 @@
*/
public final class EnvironmentGetter implements TextMapGetter<Map<String, String>> {

private static final Logger logger = Logger.getLogger(EnvironmentGetter.class.getName());
private static final AtomicBoolean LOG_KEYS_CALLED = new AtomicBoolean(false);
private static final Logger LOGGER = Logger.getLogger(EnvironmentGetter.class.getName());
private static final EnvironmentGetter INSTANCE = new EnvironmentGetter();

private EnvironmentGetter() {}
Expand All @@ -58,14 +59,21 @@ public static EnvironmentGetter getInstance() {

@Override
public Iterable<String> keys(Map<String, String> carrier) {
if (LOG_KEYS_CALLED.compareAndSet(false, true)) {
LOGGER.log(
Level.WARNING,
"keys() called on EnvironmentGetter. "
+ "This may produce unexpected results with propagators which depend on case sensitivity or special characters in keys.",
new Throwable());
}
if (carrier == null) {
return Collections.emptyList();
}
List<String> result = new ArrayList<>(carrier.size());
for (String key : carrier.keySet()) {
result.add(key.toLowerCase(Locale.ROOT));
result.add(EnvironmentSetter.normalizeKey(key));
}
return result;
return Collections.unmodifiableList(result);
}

@Nullable
Expand All @@ -74,20 +82,15 @@ 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;
String normalizedKey = EnvironmentSetter.normalizeKey(key);
String[] result = new String[] {null};
carrier.forEach(
(entryKey, entryValue) -> {
if (EnvironmentSetter.normalizeKey(entryKey).equals(normalizedKey)) {
result[0] = entryValue;
Copy link
Copy Markdown
Member

@pellared pellared Apr 3, 2026

Choose a reason for hiding this comment

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

I guess that thus can be done a little more efficient.
We could break when we find a matching key instead of iterating over all env vars.
I guess iterating over carrier.entrySet() and early return the value when normalized key matches.

Copy link
Copy Markdown
Member Author

@jack-berg jack-berg Apr 3, 2026

Choose a reason for hiding this comment

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

Yup meant to do that

Copy link
Copy Markdown
Member Author

@jack-berg jack-berg Apr 3, 2026

Choose a reason for hiding this comment

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

Oh yeah can't exit early from for each. So it's either use a traditional loop which allocated but can exit early, or use forEach which doesn't allocate but which can't exit early

Copy link
Copy Markdown
Member Author

@jack-berg jack-berg Apr 3, 2026

Choose a reason for hiding this comment

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

Given that strings are being allocated for every key, trying to avoid allocating for map iteration is silly optimization. Will switch to tradition for loop and exit early

}
});
Copy link
Copy Markdown
Member Author

@jack-berg jack-berg Apr 3, 2026

Choose a reason for hiding this comment

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

Not great that evertime we read a key, we have to normalize potentially all the keys in the carrier. Go gets around this by computing the normalized keys once and using the carrier as a cache. In java, its not safe to assume that the map is mutable.

I imagine this isn't a real problem though because env var propagation implies your going to start a new process and pass the context to it via env vars, which is much heavier weight than what we're doing here.

Copy link
Copy Markdown
Member

@pellared pellared Apr 3, 2026

Choose a reason for hiding this comment

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

I imagine this isn't a real problem though because env var propagation implies your going to start a new process and pass the context to it via env vars, which is much heavier weight than what we're doing here.

Agreed and at least it is a robust implementation 😁

return result[0];
}

@Override
Expand Down
57 changes: 28 additions & 29 deletions ...incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
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;

/**
Expand All @@ -30,8 +27,10 @@
* conventions:
*
* <ul>
* <li>Converts keys to uppercase (e.g., {@code traceparent} becomes {@code TRACEPARENT})
* <li>Replaces {@code .} and {@code -} with underscores
* <li>ASCII letters are converted to uppercase
* <li>Any character that is not an ASCII letter, digit, or underscore is replaced with an
* underscore
* <li>If the result would start with a digit, an underscore is prepended
* </ul>
*
* <p>Values are validated to contain only characters valid in HTTP header fields per <a
Expand All @@ -49,7 +48,6 @@
*/
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() {}
Expand All @@ -64,35 +62,36 @@ 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);
String normalizedKey = normalizeKey(key);
carrier.put(normalizedKey, 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).
* Normalizes a key to be a valid environment variable name.
*
* <ul>
* <li>ASCII letters are converted to uppercase
* <li>Any character that is not an ASCII letter, digit, or underscore is replaced with an
* underscore (including {@code .}, {@code -}, whitespace, and control characters)
* <li>If the result would start with a digit, an underscore is prepended
* </ul>
*/
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;
static String normalizeKey(String key) {
StringBuilder sb = new StringBuilder(key.length() + 1);
for (int i = 0; i < key.length(); i++) {
char ch = key.charAt(i);
if (ch >= 'a' && ch <= 'z') {
sb.append((char) (ch - 32));
} else if ((ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '_') {
sb.append(ch);
} else {
sb.append('_');
}
}
return true;
if (sb.length() > 0 && sb.charAt(0) >= '0' && sb.charAt(0) <= '9') {
sb.insert(0, '_');
}
return sb.toString();
}

@Override
Expand Down
54 changes: 34 additions & 20 deletions ...bator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,19 @@

import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;

import io.github.netmikey.logunit.api.LogCapturer;
import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

class EnvironmentGetterTest {

@RegisterExtension
LogCapturer logCapturer = LogCapturer.create().captureForType(EnvironmentGetter.class);

@Test
void get() {
Map<String, String> carrier = new HashMap<>();
Expand All @@ -29,10 +35,10 @@ void get() {
}

@Test
void get_sanitization() {
void get_normalization() {
Map<String, String> carrier = new HashMap<>();
carrier.put("OTEL_TRACE_ID", "val1");
carrier.put("OTEL_BAGGAGE_KEY", "val2");
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");
Expand All @@ -45,37 +51,45 @@ void get_null() {
}

@Test
void keys() {
@SuppressLogger(EnvironmentGetter.class)
void keys_valuesAreNormalized() {
Map<String, String> carrier = new HashMap<>();
carrier.put("K1", "V1");
carrier.put("K2", "V2");

assertThat(EnvironmentGetter.getInstance().keys(carrier)).containsExactlyInAnyOrder("k1", "k2");
carrier.put("otel.trace.id", "V1");
carrier.put("otel-baggage-key", "V2");
carrier.put("OTEL_FOO", "V2");

// For a carrier containing keys that are both normalized and not normalized, verify all results
// from keys() return values for get.
assertThat(EnvironmentGetter.getInstance().keys(carrier))
.containsExactlyInAnyOrder("OTEL_TRACE_ID", "OTEL_BAGGAGE_KEY", "OTEL_FOO");
for (String key : EnvironmentGetter.getInstance().keys(carrier)) {
assertThat(EnvironmentGetter.getInstance().get(carrier, key)).isNotNull();
}
assertThat(EnvironmentGetter.getInstance().keys(null)).isEmpty();

assertThat(logCapturer.size()).isEqualTo(1);
logCapturer.assertContains("keys() called on EnvironmentGetter");
}

@Test
void get_validHeaderValues() {
void get_valuesAreUnmodified() {
Map<String, String> carrier = new HashMap<>();
carrier.put("KEY1", "simple-value");
carrier.put("KEY2", "value with spaces");
carrier.put("KEY3", "value\twith\ttabs");
carrier.put("KEY4", "value\u0000with\u0001control");
carrier.put("KEY5", "value\nwith\nnewlines");
carrier.put("KEY6", "value\u0080non-ascii");

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();
assertThat(EnvironmentGetter.getInstance().get(carrier, "key4"))
.isEqualTo("value\u0000with\u0001control");
assertThat(EnvironmentGetter.getInstance().get(carrier, "key5"))
.isEqualTo("value\nwith\nnewlines");
assertThat(EnvironmentGetter.getInstance().get(carrier, "key6"))
.isEqualTo("value\u0080non-ascii");
}

@Test
Expand Down
21 changes: 7 additions & 14 deletions ...bator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,28 +45,21 @@ void set_null() {
}

@Test
void set_validHeaderValues() {
void set_valuesAreUnmodified() {
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");
EnvironmentSetter.getInstance().set(carrier, "key4", "value\u0000with\u0001control");
EnvironmentSetter.getInstance().set(carrier, "key5", "value\nwith\nnewlines");
EnvironmentSetter.getInstance().set(carrier, "key6", "value\u0080non-ascii");

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();
assertThat(carrier).containsEntry("KEY4", "value\u0000with\u0001control");
assertThat(carrier).containsEntry("KEY5", "value\nwith\nnewlines");
assertThat(carrier).containsEntry("KEY6", "value\u0080non-ascii");
}

@Test
Expand Down
Loading