From 713acfdcc55d195cf7ae50d284c6608e65b4007c Mon Sep 17 00:00:00 2001
From: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Date: Thu, 18 Jan 2024 16:22:44 +0100
Subject: [PATCH] header values are be expected to be W3C baggage encoded

---
 .../exporters/otlp/OtlpExporterUtil.java      | 22 +++++++++++++++----
 ...OtlpSpanExporterAutoConfigurationTest.java | 20 +++++++++++++++--
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpExporterUtil.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpExporterUtil.java
index 151862501bc8..4a3d88a23f5f 100644
--- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpExporterUtil.java
+++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpExporterUtil.java
@@ -6,7 +6,10 @@
 package io.opentelemetry.instrumentation.spring.autoconfigure.exporters.otlp;
 
 import io.opentelemetry.exporter.otlp.internal.OtlpConfigUtil;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
 import java.time.Duration;
+import java.util.AbstractMap;
 import java.util.Map;
 import java.util.function.BiConsumer;
 import java.util.function.Function;
@@ -79,10 +82,21 @@ static <G, H, E> E applySignalProperties(
       headers = properties.getHeaders();
     }
     for (Map.Entry<String, String> entry : headers.entrySet()) {
-      if (isHttpProtobuf) {
-        addHttpHeader.accept(httpBuilder, entry);
-      } else {
-        addGrpcHeader.accept(grpcBuilder, entry);
+      String value = entry.getValue();
+      try {
+        // headers are encoded as URL - see
+        // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#specifying-headers-via-environment-variables
+        Map.Entry<String, String> decoded =
+            new AbstractMap.SimpleEntry<>(
+                entry.getKey(), URLDecoder.decode(value, StandardCharsets.UTF_8.displayName()));
+
+        if (isHttpProtobuf) {
+          addHttpHeader.accept(httpBuilder, decoded);
+        } else {
+          addGrpcHeader.accept(grpcBuilder, decoded);
+        }
+      } catch (Exception e) {
+        throw new IllegalArgumentException("Cannot decode header value: " + value, e);
       }
     }
 
diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java
index 54449e437cbc..c5dd90b311f6 100644
--- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java
+++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java
@@ -17,11 +17,15 @@
 import java.util.stream.Collectors;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mockito;
 import org.springframework.boot.autoconfigure.AutoConfigurations;
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.boot.test.system.CapturedOutput;
+import org.springframework.boot.test.system.OutputCaptureExtension;
 
 /** Spring Boot auto configuration test for {@link OtlpSpanExporterAutoConfiguration}. */
+@ExtendWith(OutputCaptureExtension.class)
 class OtlpSpanExporterAutoConfigurationTest {
 
   private final OtlpHttpSpanExporterBuilder otlpHttpSpanExporterBuilder =
@@ -118,15 +122,27 @@ void useHttpWithEnv() {
         .withPropertyValues(
             "otel.exporter.otlp.enabled=true", "otel.exporter.otlp.protocol=http/protobuf")
         // are similar to environment variables in that they use the same converters
-        .withSystemProperties("otel.exporter.otlp.headers=x=1,y=2")
+        .withSystemProperties("otel.exporter.otlp.headers=x=1,y=2%203")
         .run(context -> {});
 
     Mockito.verify(otlpHttpSpanExporterBuilder).build();
     Mockito.verify(otlpHttpSpanExporterBuilder).addHeader("x", "1");
-    Mockito.verify(otlpHttpSpanExporterBuilder).addHeader("y", "2");
+    Mockito.verify(otlpHttpSpanExporterBuilder).addHeader("y", "2 3");
     Mockito.verifyNoMoreInteractions(otlpHttpSpanExporterBuilder);
   }
 
+  @Test
+  @DisplayName("OTLP header with illegal % encoding causes an exception")
+  void decodingError(CapturedOutput output) {
+    this.contextRunner
+        .withBean(OtlpHttpSpanExporterBuilder.class, () -> otlpHttpSpanExporterBuilder)
+        .withSystemProperties("otel.exporter.otlp.headers=x=%-1")
+        .run(context -> {});
+
+    // spring catches the exception and logs it
+    assertThat(output).contains("Cannot decode header value: %-1");
+  }
+
   @Test
   @DisplayName("use grpc when protocol set")
   void useGrpc() {