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 2 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
2 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
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
27 changes: 3 additions & 24 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 @@ -8,11 +8,7 @@
import io.opentelemetry.context.propagation.TextMapGetter;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -46,7 +42,6 @@
*/
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() {}
Expand All @@ -61,11 +56,7 @@ public Iterable<String> keys(Map<String, String> carrier) {
if (carrier == null) {
return Collections.emptyList();
}
List<String> result = new ArrayList<>(carrier.size());
for (String key : carrier.keySet()) {
result.add(key.toLowerCase(Locale.ROOT));
}
return result;
return new ArrayList<>(carrier.keySet());
Copy link
Copy Markdown
Contributor

@laurit laurit Apr 2, 2026

Choose a reason for hiding this comment

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

could be a bit surprising that this method may return keys that can't be read (because get normalizes keys)

Copy link
Copy Markdown
Member

@pellared pellared Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Do you think we should only returns keys that can be actually read (conform to the normalized name)?

Copy link
Copy Markdown
Contributor

@laurit laurit Apr 2, 2026

Choose a reason for hiding this comment

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

The keys method is only used in deprecated JaegerPropagator and OtTracePropagator so it doesn't really have much of a impact. Even if the environment getter would return empty keys it would probably be fine.
In

opentelemetry-java/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/JaegerPropagator.java

Line 235 in c856c2b

if (key.startsWith(BAGGAGE_PREFIX)) {
there is a check that key starts with uberctx-. If the key is normalized to upper case the check will fail. If it is not then later get will fail to find the value.
In

opentelemetry-java/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/OtTracePropagator.java

Lines 125 to 130 in c856c2b

for (String key : getter.keys(carrier)) {
String lowercaseKey = key.toLowerCase(Locale.ROOT);
if (!lowercaseKey.startsWith(PREFIX_BAGGAGE_HEADER)) {
continue;
}
String value = getter.get(carrier, key);
I think having the keys normalized would actually work.

Copy link
Copy Markdown
Member

@pellared pellared Apr 2, 2026

Choose a reason for hiding this comment

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

Thanks! Then I think that the keys should be normalized.

Moreover, if you add key = key.toLowerCase(Locale.ROOT); before

opentelemetry-java/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/JaegerPropagator.java

Line 235 in c856c2b

if (key.startsWith(BAGGAGE_PREFIX)) {

then even the deprecated JaegerPropagatorwould work.

I am going address the same issue in Go implementation and also add something the the OTel Specification.

Copy link
Copy Markdown
Member

@pellared pellared Apr 2, 2026

Choose a reason for hiding this comment

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

@jack-berg @laurit PTAL open-telemetry/opentelemetry-go-contrib#8761

Copy link
Copy Markdown
Contributor

@laurit laurit Apr 2, 2026

Choose a reason for hiding this comment

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

Actually the OtTracePropagator wouldn't work either since the prefix is ot-baggage- and - gets replaced with _.

Copy link
Copy Markdown
Member Author

@jack-berg jack-berg Apr 2, 2026

Choose a reason for hiding this comment

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

The keys method is only used in deprecated JaegerPropagator and OtTracePropagator so it doesn't really have much of a impact.

Deprecated and only used for baggage propagation within those.

So what I'm hearing is that keys is incompatible with any propagator which calls it and has keys with characters which would be normalized

Options:

  • Throw when keys is called. Probably a non-starter since you don't know which propagators are configured and throwing an unchecked exception would be disruptive. However, if you are doing this propagation, you do know that you're using EnvironmentGetter and we could document that for this use case you need to handle exceptions when you call TextMapPropagator.extract(Context.current(), env, EnvironmentGetter.getInstance()).
  • When keys is called, return empty iterable and:
    • Log a warning. Means that propagation will fail with certain propagators, but at least there's a breadcrumb point to what happened.
    • Do nothing extra. Means that propagation will fail silently with certain propagators.
  • Return normalized or lowercased keys, and adjust JaegerPropagator / OtTracePropagator to reduce dependency on keys and make it more resilient to - vs. _, and:
    • Log a warning when keys is called. This would allow future propagators to have bread crumb if they use keys, which may not function in env var context.
    • Do nothing extra.

I lean towards logging a warning when keys is called, and not very opinionated about what the actual return value of keys is (as is, normalized, lowercased, or empty).

Copy link
Copy Markdown
Member

@pellared pellared Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

If Keys returns keys in normalized form then Get should also normalize the cached env vars. Otherwise we will have similar problem as described in #8233 (comment). This is what is done in open-telemetry/opentelemetry-go-contrib#8761

}

@Nullable
Expand All @@ -74,20 +65,8 @@ 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);
return carrier.get(normalizedKey);
}

@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
25 changes: 11 additions & 14 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 @@ -50,32 +50,29 @@ void keys() {
carrier.put("K1", "V1");
carrier.put("K2", "V2");

assertThat(EnvironmentGetter.getInstance().keys(carrier)).containsExactlyInAnyOrder("k1", "k2");
assertThat(EnvironmentGetter.getInstance().keys(carrier)).containsExactlyInAnyOrder("K1", "K2");
assertThat(EnvironmentGetter.getInstance().keys(null)).isEmpty();
}

@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