fix: Align environment carriers with spec requirements

Author: Bhagirath00

api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetter.java

@@ -10,17 +10,32 @@
 import java.util.Locale;
 import java.util.Map;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
 
 /**
  * A {@link TextMapGetter} that extracts context from a map carrier, intended for use with
  * environment variables.
  *
  * <p>Standard environment variable names are uppercase (e.g., {@code TRACEPARENT}, {@code
- * TRACESTATE}, {@code BAGGAGE}). This getter translates keys to uppercase before looking them up in
- * the carrier.
+ * TRACESTATE}, {@code BAGGAGE}). This getter translates keys to uppercase and replaces characters
+ * not allowed in environment variables (e.g., {@code .} and {@code -}) with underscores before
+ * looking them up in the carrier.
+
+ * @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 enum EnvironmentGetter implements TextMapGetter<Map<String, String>> {
-  INSTANCE;
+@Immutable
+public final class EnvironmentGetter implements TextMapGetter<Map<String, String>> {
+
+  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) {
@@ -36,8 +51,11 @@ public String get(@Nullable Map<String, String> carrier, String key) {
     if (carrier == null || key == null) {
       return null;
     }
-    // Spec recommends using uppercase for environment variable names.
-    return carrier.get(key.toUpperCase(Locale.ROOT));
+    // Spec recommends using uppercase and underscores for environment variable
+    // names for maximum
+    // cross-platform compatibility.
+    String sanitizedKey = key.replace('.', '_').replace('-', '_').toUpperCase(Locale.ROOT);
+    return carrier.get(sanitizedKey);
   }
 
   @Override

api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetter.java

@@ -9,25 +9,43 @@
 import java.util.Locale;
 import java.util.Map;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
 
 /**
  * A {@link TextMapSetter} that injects context into a map carrier, intended for use with
  * environment variables.
  *
  * <p>Standard environment variable names are uppercase (e.g., {@code TRACEPARENT}, {@code
- * TRACESTATE}, {@code BAGGAGE}). This setter translates keys to uppercase before inserting them
- * into the carrier.
+ * TRACESTATE}, {@code BAGGAGE}). This setter translates keys to uppercase and replaces characters
+ * not allowed in environment variables (e.g., {@code .} and {@code -}) with underscores before
+ * inserting them into the carrier.
+ *
+ * @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 enum EnvironmentSetter implements TextMapSetter<Map<String, String>> {
-  INSTANCE;
+@Immutable
+public final class EnvironmentSetter implements TextMapSetter<Map<String, String>> {
+
+  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;
     }
-    // Spec recommends using uppercase for environment variable names.
-    carrier.put(key.toUpperCase(Locale.ROOT), value);
+    // 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);
   }
 
   @Override

api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetterTest.java

@@ -22,16 +22,26 @@ void get() {
     carrier.put("BAGGAGE", "val3");
     carrier.put("OTHER", "val4");
 
-    assertThat(EnvironmentGetter.INSTANCE.get(carrier, "traceparent")).isEqualTo("val1");
-    assertThat(EnvironmentGetter.INSTANCE.get(carrier, "TRACESTATE")).isEqualTo("val2");
-    assertThat(EnvironmentGetter.INSTANCE.get(carrier, "Baggage")).isEqualTo("val3");
-    assertThat(EnvironmentGetter.INSTANCE.get(carrier, "other")).isEqualTo("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.INSTANCE.get(null, "key")).isNull();
-    assertThat(EnvironmentGetter.INSTANCE.get(Collections.emptyMap(), null)).isNull();
+    assertThat(EnvironmentGetter.getInstance().get(null, "key")).isNull();
+    assertThat(EnvironmentGetter.getInstance().get(Collections.emptyMap(), null)).isNull();
   }
 
   @Test
@@ -40,12 +50,12 @@ void keys() {
     carrier.put("K1", "V1");
     carrier.put("K2", "V2");
 
-    assertThat(EnvironmentGetter.INSTANCE.keys(carrier)).containsExactlyInAnyOrder("K1", "K2");
-    assertThat(EnvironmentGetter.INSTANCE.keys(null)).isEmpty();
+    assertThat(EnvironmentGetter.getInstance().keys(carrier)).containsExactlyInAnyOrder("K1", "K2");
+    assertThat(EnvironmentGetter.getInstance().keys(null)).isEmpty();
   }
 
   @Test
   void testToString() {
-    assertThat(EnvironmentGetter.INSTANCE.toString()).isEqualTo("EnvironmentGetter");
+    assertThat(EnvironmentGetter.getInstance().toString()).isEqualTo("EnvironmentGetter");
   }
 }

api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetterTest.java

@@ -16,26 +16,36 @@ class EnvironmentSetterTest {
   @Test
   void set() {
     Map<String, String> carrier = new HashMap<>();
-    EnvironmentSetter.INSTANCE.set(carrier, "traceparent", "val1");
-    EnvironmentSetter.INSTANCE.set(carrier, "TRACESTATE", "val2");
-    EnvironmentSetter.INSTANCE.set(carrier, "Baggage", "val3");
+    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.INSTANCE.set(null, "key", "val");
-    EnvironmentSetter.INSTANCE.set(carrier, null, "val");
-    EnvironmentSetter.INSTANCE.set(carrier, "key", null);
+    EnvironmentSetter.getInstance().set(null, "key", "val");
+    EnvironmentSetter.getInstance().set(carrier, null, "val");
+    EnvironmentSetter.getInstance().set(carrier, "key", null);
     assertThat(carrier).isEmpty();
   }
 
   @Test
   void testToString() {
-    assertThat(EnvironmentSetter.INSTANCE.toString()).isEqualTo("EnvironmentSetter");
+    assertThat(EnvironmentSetter.getInstance().toString()).isEqualTo("EnvironmentSetter");
   }
 }