Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ public Iterable<String> keys(Map<String, String> carrier) {
}
List<String> result = new ArrayList<>(carrier.size());
for (String key : carrier.keySet()) {
result.add(EnvironmentSetter.normalizeKey(key));
if (EnvironmentSetter.isNormalizedKey(key)) {
result.add(key);
}
}
return Collections.unmodifiableList(result);
}
Expand All @@ -83,20 +85,7 @@ public String get(@Nullable Map<String, String> carrier, String key) {
return null;
}
String normalizedKey = EnvironmentSetter.normalizeKey(key);
// first, perform an optimistic lookup for an exact match on the normalized key
String value = carrier.get(normalizedKey);
if (value != null) {
return value;
}
// next, iterate over the carrier normalizing each entry and evaluating for a match
// if memory allocation becomes an issue, can implement using iterative normalization, comparing
// an entry character by character to the normalized key, normalizing along the way.
for (Map.Entry<String, String> entry : carrier.entrySet()) {
if (EnvironmentSetter.normalizeKey(entry.getKey()).equals(normalizedKey)) {
return entry.getValue();
}
}
return null;
return carrier.get(normalizedKey);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,33 @@ public void set(@Nullable Map<String, String> carrier, String key, String value)
carrier.put(normalizedKey, value);
}

/**
* Determine if a key is a valid normalized 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>Does not start with a digit
* </ul>
*/
Comment on lines +69 to +78

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this could be reworded to sound a bit more like a predicate

Suggested change
/**
* Determine if a key is a valid normalized 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>Does not start with a digit
* </ul>
*/
/**
* Determine if a key is a valid normalized environment variable name. Returns {@code true} if
* {@code key} is non-empty, contains only uppercase ASCII letters, digits, and underscores, and
* does not start with a digit.
*/

static boolean isNormalizedKey(String key) {
if (key.isEmpty()) {
return false;
}
char first = key.charAt(0);
if (first >= '0' && first <= '9') {
return false;
}
for (int i = 0; i < key.length(); i++) {
char ch = key.charAt(i);
if (!((ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '_')) {
return false;
}
}
return true;
}

/**
* Normalizes a key to be a valid environment variable name.
*
Expand All @@ -77,6 +104,9 @@ public void set(@Nullable Map<String, String> carrier, String key, String value)
* </ul>
*/
static String normalizeKey(String key) {
if (isNormalizedKey(key)) {
return key;
}
StringBuilder sb = new StringBuilder(key.length() + 1);
for (int i = 0; i < key.length(); i++) {
char ch = key.charAt(i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,17 @@ void get() {
@Test
void get_normalization() {
Map<String, String> carrier = new HashMap<>();
// Carrier entries are expected to have normalized keys (i.e. set via EnvironmentSetter).
carrier.put("OTEL_TRACE_ID", "val1");
carrier.put("otel-baggage-key", "val2");
carrier.put("OTEL_BAGGAGE_KEY", "val2");
// Carrier entry with an unnormalized key is not reachable via get().
carrier.put("otel-unreachable-key", "val3");

// Lookup keys are normalized before the carrier map is consulted.
assertThat(EnvironmentGetter.getInstance().get(carrier, "otel.trace.id")).isEqualTo("val1");
assertThat(EnvironmentGetter.getInstance().get(carrier, "otel-baggage-key")).isEqualTo("val2");
// Carrier entries with unnormalized keys are not enumerated.
assertThat(EnvironmentGetter.getInstance().get(carrier, "otel-unreachable-key")).isNull();
}

@Test
Expand All @@ -52,16 +58,14 @@ void get_null() {

@Test
@SuppressLogger(EnvironmentGetter.class)
void keys_valuesAreNormalized() {
void keys_onlyNormalizedKeysIncluded() {
Map<String, String> carrier = new HashMap<>();
carrier.put("otel.trace.id", "V1");
carrier.put("otel-baggage-key", "V2");
carrier.put("OTEL_FOO", "V2");
carrier.put("OTEL_FOO", "V3");

// 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");
// Non-normalized keys are excluded; only already-normalized keys are returned.
assertThat(EnvironmentGetter.getInstance().keys(carrier)).containsExactlyInAnyOrder("OTEL_FOO");
for (String key : EnvironmentGetter.getInstance().keys(carrier)) {
assertThat(EnvironmentGetter.getInstance().get(carrier, key)).isNotNull();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@

import java.util.HashMap;
import java.util.Map;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

class EnvironmentSetterTest {

Expand Down Expand Up @@ -62,6 +66,27 @@ void set_valuesAreUnmodified() {
assertThat(carrier).containsEntry("KEY6", "value\u0080non-ascii");
}

@ParameterizedTest
@MethodSource("isNormalizedKeyCases")
void isNormalizedKey(String key, boolean expected) {
assertThat(EnvironmentSetter.isNormalizedKey(key)).isEqualTo(expected);
}

static Stream<Arguments> isNormalizedKeyCases() {
return Stream.of(
Arguments.argumentSet("uppercase letters", "TRACEPARENT", true),
Arguments.argumentSet("uppercase with underscores", "OTEL_TRACE_ID", true),
Arguments.argumentSet("single letter", "A", true),
Arguments.argumentSet("letter then digit", "A0", true),
Arguments.argumentSet("mixed uppercase digits underscores", "A_B_0", true),
Arguments.argumentSet("empty string", "", false),
Arguments.argumentSet("starts with digit", "0ABC", false),
Arguments.argumentSet("lowercase letters", "traceparent", false),
Arguments.argumentSet("dots", "otel.trace.id", false),
Arguments.argumentSet("hyphens", "otel-baggage-key", false),
Arguments.argumentSet("space", "OTEL TRACE", false));
}

@Test
void testToString() {
assertThat(EnvironmentSetter.getInstance().toString()).isEqualTo("EnvironmentSetter");
Expand Down
Loading