make spring boot honor the standard environment variables for maps (#…

…11000)

instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/properties/SpringConfigProperties.java

@@ -10,6 +10,7 @@
 import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
 import java.time.Duration;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import javax.annotation.Nullable;
@@ -24,21 +25,21 @@ public class SpringConfigProperties implements ConfigProperties {
   private final OtlpExporterProperties otlpExporterProperties;
   private final OtelResourceProperties resourceProperties;
   private final PropagationProperties propagationProperties;
-  private final ConfigProperties fallback;
+  private final ConfigProperties otelSdkProperties;
 
   public SpringConfigProperties(
       Environment environment,
       ExpressionParser parser,
       OtlpExporterProperties otlpExporterProperties,
       OtelResourceProperties resourceProperties,
       PropagationProperties propagationProperties,
-      ConfigProperties fallback) {
+      ConfigProperties otelSdkProperties) {
     this.environment = environment;
     this.parser = parser;
     this.otlpExporterProperties = otlpExporterProperties;
     this.resourceProperties = resourceProperties;
     this.propagationProperties = propagationProperties;
-    this.fallback = fallback;
+    this.otelSdkProperties = otelSdkProperties;
   }
 
   // visible for testing
@@ -66,31 +67,31 @@ public String getString(String name) {
       // in specification to default to `http/protobuf`
       return OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF;
     }
-    return or(value, fallback.getString(name));
+    return or(value, otelSdkProperties.getString(name));
   }
 
   @Nullable
   @Override
   public Boolean getBoolean(String name) {
-    return or(environment.getProperty(name, Boolean.class), fallback.getBoolean(name));
+    return or(environment.getProperty(name, Boolean.class), otelSdkProperties.getBoolean(name));
   }
 
   @Nullable
   @Override
   public Integer getInt(String name) {
-    return or(environment.getProperty(name, Integer.class), fallback.getInt(name));
+    return or(environment.getProperty(name, Integer.class), otelSdkProperties.getInt(name));
   }
 
   @Nullable
   @Override
   public Long getLong(String name) {
-    return or(environment.getProperty(name, Long.class), fallback.getLong(name));
+    return or(environment.getProperty(name, Long.class), otelSdkProperties.getLong(name));
   }
 
   @Nullable
   @Override
   public Double getDouble(String name) {
-    return or(environment.getProperty(name, Double.class), fallback.getDouble(name));
+    return or(environment.getProperty(name, Double.class), otelSdkProperties.getDouble(name));
   }
 
   @SuppressWarnings("unchecked")
@@ -100,15 +101,15 @@ public List<String> getList(String name) {
       return propagationProperties.getPropagators();
     }
 
-    return or(environment.getProperty(name, List.class), fallback.getList(name));
+    return or(environment.getProperty(name, List.class), otelSdkProperties.getList(name));
   }
 
   @Nullable
   @Override
   public Duration getDuration(String name) {
     String value = getString(name);
     if (value == null) {
-      return fallback.getDuration(name);
+      return otelSdkProperties.getDuration(name);
     }
     return DefaultConfigProperties.createFromMap(Collections.singletonMap(name, value))
         .getDuration(name);
@@ -117,29 +118,45 @@ public Duration getDuration(String name) {
   @SuppressWarnings("unchecked")
   @Override
   public Map<String, String> getMap(String name) {
+    Map<String, String> otelSdkMap = otelSdkProperties.getMap(name);
     // maps from config properties are not supported by Environment, so we have to fake it
     switch (name) {
       case "otel.resource.attributes":
-        return resourceProperties.getAttributes();
+        return mergeWithOtel(resourceProperties.getAttributes(), otelSdkMap);
       case "otel.exporter.otlp.headers":
-        return otlpExporterProperties.getHeaders();
+        return mergeWithOtel(otlpExporterProperties.getHeaders(), otelSdkMap);
       case "otel.exporter.otlp.logs.headers":
-        return otlpExporterProperties.getLogs().getHeaders();
+        return mergeWithOtel(otlpExporterProperties.getLogs().getHeaders(), otelSdkMap);
       case "otel.exporter.otlp.metrics.headers":
-        return otlpExporterProperties.getMetrics().getHeaders();
+        return mergeWithOtel(otlpExporterProperties.getMetrics().getHeaders(), otelSdkMap);
       case "otel.exporter.otlp.traces.headers":
-        return otlpExporterProperties.getTraces().getHeaders();
+        return mergeWithOtel(otlpExporterProperties.getTraces().getHeaders(), otelSdkMap);
       default:
         break;
     }
 
     String value = environment.getProperty(name);
     if (value == null) {
-      return fallback.getMap(name);
+      return otelSdkMap;
     }
     return (Map<String, String>) parser.parseExpression(value).getValue();
   }
 
+  /**
+   * If you specify the environment variable <code>OTEL_RESOURCE_ATTRIBUTES_POD_NAME</code>, then
+   * Spring Boot will ignore <code>OTEL_RESOURCE_ATTRIBUTES</code>, which violates the principle of
+   * least surprise. This method merges the two maps, giving precedence to <code>
+   * OTEL_RESOURCE_ATTRIBUTES_POD_NAME</code>, which is more specific and which is also the value
+   * that Spring Boot will use (and which will honor <a
+   * href="https://docs.spring.io/spring-framework/docs/3.2.x/spring-framework-reference/html/expressions.html">SpEL</a>).
+   */
+  private static Map<String, String> mergeWithOtel(
+      Map<String, String> springMap, Map<String, String> otelSdkMap) {
+    Map<String, String> merged = new HashMap<>(otelSdkMap);
+    merged.putAll(springMap);
+    return merged;
+  }
+
   @Nullable
   private static <T> T or(@Nullable T first, @Nullable T second) {
     return first != null ? first : second;

instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/resources/SpringConfigPropertiesTest.java

@@ -8,6 +8,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.entry;
 
+import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
 import io.opentelemetry.instrumentation.spring.autoconfigure.properties.OtelResourceProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.properties.OtlpExporterProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.properties.PropagationProperties;
@@ -17,6 +18,7 @@
 import java.util.Map;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
+import org.springframework.boot.autoconfigure.AutoConfigurations;
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
 import org.springframework.core.env.Environment;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
@@ -28,29 +30,33 @@ class SpringConfigPropertiesTest {
   @DisplayName("when map is set in properties in a row it should be available in config")
   void shouldInitializeAttributesByMapInArow() {
     this.contextRunner
+        .withConfiguration(AutoConfigurations.of(OpenTelemetryAutoConfiguration.class))
         .withPropertyValues(
-            "otel.springboot.test.map={'environment':'dev','xyz':'foo','service.instance.id':'id-example'}")
+            "otel.resource.attributes.environment=dev",
+            "otel.resource.attributes.xyz=foo",
+            "otel.resource.attributes.service.instance.id=id-example")
         .run(
             context -> {
               Environment env = context.getBean("environment", Environment.class);
               Map<String, String> fallback = new HashMap<>();
               fallback.put("fallback", "fallbackVal");
-              fallback.put("otel.springboot.test.map", "hidden");
+              fallback.put("otel.resource.attributes", "foo=fallback");
 
               SpringConfigProperties config =
                   new SpringConfigProperties(
                       env,
                       new SpelExpressionParser(),
-                      new OtlpExporterProperties(),
-                      new OtelResourceProperties(),
-                      new PropagationProperties(),
+                      context.getBean(OtlpExporterProperties.class),
+                      context.getBean(OtelResourceProperties.class),
+                      context.getBean(PropagationProperties.class),
                       DefaultConfigProperties.createFromMap(fallback));
 
-              assertThat(config.getMap("otel.springboot.test.map"))
+              assertThat(config.getMap("otel.resource.attributes"))
                   .contains(
                       entry("environment", "dev"),
                       entry("xyz", "foo"),
-                      entry("service.instance.id", "id-example"));
+                      entry("service.instance.id", "id-example"),
+                      entry("foo", "fallback"));
 
               assertThat(config.getString("fallback")).isEqualTo("fallbackVal");
             });

smoke-tests-otel-starter/src/test/java/io/opentelemetry/smoketest/OtelSpringStarterSmokeTest.java

@@ -62,7 +62,7 @@
       // We set the export interval of the logs to 100 ms. The default value is 1 second.
       "otel.blrp.schedule.delay=100",
       // The headers are simply set here to make sure that headers can be parsed
-      "otel.exporter.otlp.headers=a=1,b=2",
+      "otel.exporter.otlp.headers.c=3",
       "otel.traces.exporter=memory",
       "otel.metrics.exporter=memory",
       "otel.logs.exporter=memory"
@@ -163,10 +163,12 @@ void propertyConversion() {
             otlpExporterProperties,
             otelResourceProperties,
             propagationProperties,
-            DefaultConfigProperties.createFromMap(Collections.emptyMap()));
+            DefaultConfigProperties.createFromMap(
+                Collections.singletonMap("otel.exporter.otlp.headers", "a=1,b=2")));
     assertThat(configProperties.getMap("otel.exporter.otlp.headers"))
         .containsEntry("a", "1")
-        .containsEntry("b", "2");
+        .containsEntry("b", "2")
+        .containsEntry("c", "3");
     assertThat(configProperties.getList("otel.propagators")).containsExactly("b3");
   }