[API] make EnvironmentCarrier spec-compliant (#4190)

api/include/opentelemetry/context/propagation/environment_carrier.h

@@ -27,12 +27,35 @@ namespace propagation
 // The carrier supports two usage scenarios:
 //
 // 1. Extract (default constructor): Reads context from environment variables.
-//    Get() reads from TRACEPARENT, TRACESTATE, BAGGAGE environment variables.
-//    Set() is a no-op. Values are cached on first access for lifetime management.
+//    Get() reads directly from the process environment on every call. Callers should
+//    create a carrier and call Extract() once during process initialization and retain
+//    the resulting Context for the lifetime of the process or task. Keys are normalized
+//    to environment variable names before lookup (e.g. "traceparent" -> "TRACEPARENT").
+//    Set() is a no-op.
 //
 // 2. Inject (shared_ptr constructor): Writes context to a provided map.
-//    Set() writes to the provided std::map. Keys are automatically converted
-//    from lowercase header names to uppercase environment variable names.
+//    Set() writes to the provided std::map. Keys are automatically converted from
+//    lowercase header names to uppercase environment variable names. The resulting map
+//    is then typically merged into the environment of a child process before spawning.
+//
+// Operational guidance (non-normative):
+//
+// - Initialization-time extraction: Extract context once at program startup and thread
+//   the resulting Context through the rest of the application. Do not repeatedly call
+//   Get() on a long-lived carrier; environment variables are intended to be set by the
+//   parent before this process starts and should not change during process lifetime.
+//
+// - Child process environment handling: To propagate context to a child process, create
+//   a carrier with an env_map, call Inject() to populate the map, then merge the map
+//   into the child process environment (e.g., via execve/posix_spawn env, or equivalent
+//   platform APIs) before spawning. Create separate maps for child processes with
+//   different contexts.
+//
+// - Security considerations: Environment variables are visible to all code within the
+//   process and may be visible to other processes or users with sufficient permissions.
+//   Do not propagate sensitive information (credentials, secrets) via environment
+//   variable carriers. In multi-tenant environments, consider the extra exposure risk
+//   of environment variables being accessible to co-located processes.
 
 class EnvironmentCarrier : public TextMapCarrier
 {
@@ -45,8 +68,16 @@ class EnvironmentCarrier : public TextMapCarrier
       : env_map_ptr_(std::move(env_map))
   {}
 
-  // Returns the value associated with the passed key.
-  // The key is normalized per spec before lookup and caching.
+  // Returns the value of the environment variable corresponding to the normalized form of key.
+  // The key is normalized per spec before lookup (e.g. "traceparent" -> "TRACEPARENT").
+  // Returns an empty string_view if the variable is not set.
+  //
+  // The returned string_view is valid for the lifetime of this carrier object.
+  //
+  // Note: values are cached on first access. This is safe because environment variables used
+  // for context propagation are set by the parent process before this process starts and are
+  // not expected to change during process lifetime. Caching also ensures that the returned
+  // string_view remains valid for the lifetime of the carrier, regardless of when it is read.
   nostd::string_view Get(nostd::string_view key) const noexcept override
   {
     std::string env_name = NormalizeKey(key);
@@ -84,13 +115,23 @@ class EnvironmentCarrier : public TextMapCarrier
   mutable std::map<std::string, std::string> cache_;
 
   // Normalizes a key to an environment variable name per the OTel spec:
+  //   - empty key is normalized to "_"
   //   - ASCII letters are uppercased
   //   - characters that are not ASCII letters, digits, or '_' are replaced with '_'
   //   - a leading '_' is prepended if the first character is a digit
-  // e.g., "traceparent" -> "TRACEPARENT", "x-b3-traceid" -> "X_B3_TRACEID",
-  //        "1bad" -> "_1BAD"
+  // Examples:
+  //   ""            -> "_"
+  //   "traceparent" -> "TRACEPARENT"
+  //   "x-b3-traceid"-> "X_B3_TRACEID"
+  //   "1bad"        -> "_1BAD"
   static std::string NormalizeKey(nostd::string_view key)
   {
+    // Spec: empty key normalizes to "_"
+    if (key.empty())
+    {
+      return "_";
+    }
+
     std::string result(key);
     for (auto &c : result)
     {
@@ -104,7 +145,7 @@ class EnvironmentCarrier : public TextMapCarrier
         c = '_';
       }
     }
-    if (!result.empty() && result[0] >= '0' && result[0] <= '9')
+    if (result[0] >= '0' && result[0] <= '9')
     {
       result.insert(result.begin(), '_');
     }

api/test/context/propagation/environment_carrier_test.cc

@@ -103,7 +103,8 @@ TEST_F(EnvironmentCarrierTest, GetCachesValues)
   auto value1 = carrier.Get("traceparent");
   EXPECT_EQ(value1, "00-4bf92f3577b34da6a3ce929d0e0e4736-0102030405060708-01");
 
-  // Change environment - cached value should be returned
+  // Cached value is returned even after the environment variable changes.
+  // This ensures the returned string_view remains valid for the lifetime of the carrier.
   test_setenv("TRACEPARENT", "changed-value");
   auto value2 = carrier.Get("traceparent");
   EXPECT_EQ(value2, "00-4bf92f3577b34da6a3ce929d0e0e4736-0102030405060708-01");
@@ -195,6 +196,29 @@ TEST_F(EnvironmentCarrierTest, GetCacheKeyedByNormalizedName)
   test_unsetenv("X_B3_TRACEID");
 }
 
+TEST_F(EnvironmentCarrierTest, NormalizeKeyEmpty)
+{
+  // Per spec: an empty key name normalizes to "_"
+  auto env_map = std::make_shared<std::map<std::string, std::string>>();
+  context::propagation::EnvironmentCarrier carrier(env_map);
+
+  carrier.Set("", "some-value");
+  EXPECT_EQ(env_map->count("_"), 1u);
+  EXPECT_EQ(env_map->at("_"), "some-value");
+}
+
+TEST_F(EnvironmentCarrierTest, GetEmptyKeyNormalizesToUnderscore)
+{
+  // Per spec: Get("") normalizes the key to "_" and reads the "_" environment variable.
+  test_setenv("_", "underscore-value");
+
+  context::propagation::EnvironmentCarrier carrier;
+  auto value = carrier.Get("");
+  EXPECT_EQ(value, "underscore-value");
+
+  test_unsetenv("_");
+}
+
 TEST_F(EnvironmentCarrierTest, ExtractTraceContext)
 {
   test_setenv("TRACEPARENT", "00-4bf92f3577b34da6a3ce929d0e0e4736-0102030405060708-01");