pr review

Author: zeitlinger

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

@@ -10,9 +10,9 @@
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.MapConverter;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.SdkEnabled;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtelResourceProperties;
+import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtelSpringProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtlpExporterProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.SpringConfigProperties;
-import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.SpringOtelProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.resources.DistroVersionResourceProvider;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.resources.SpringResourceProvider;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
@@ -51,7 +51,7 @@
 @EnableConfigurationProperties({
   OtlpExporterProperties.class,
   OtelResourceProperties.class,
-  SpringOtelProperties.class
+  OtelSpringProperties.class
 })
 public class OpenTelemetryAutoConfiguration {
 
@@ -90,7 +90,7 @@ public AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk(
         Environment env,
         OtlpExporterProperties otlpExporterProperties,
         OtelResourceProperties resourceProperties,
-        SpringOtelProperties springOtelProperties,
+        OtelSpringProperties otelSpringProperties,
         OpenTelemetrySdkComponentLoader componentLoader) {
 
       return AutoConfigureUtil.setComponentLoader(
@@ -101,7 +101,7 @@ public AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk(
                           env,
                           otlpExporterProperties,
                           resourceProperties,
-                          springOtelProperties,
+                          otelSpringProperties,
                           c)),
               componentLoader)
           .build();

instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/SpringOtelProperties.java renamed to instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/OtelSpringProperties.java

@@ -14,7 +14,7 @@
  * any time.
  */
 @ConfigurationProperties(prefix = "otel")
-public final class SpringOtelProperties {
+public final class OtelSpringProperties {
 
   /**
    * This class is internal and is hence not for public use. Its APIs are unstable and can change at

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

@@ -31,68 +31,73 @@ public class SpringConfigProperties implements ConfigProperties {
   private final ExpressionParser parser;
   private final OtlpExporterProperties otlpExporterProperties;
   private final OtelResourceProperties resourceProperties;
-  private final SpringOtelProperties springOtelProperties;
+  private final OtelSpringProperties otelSpringProperties;
   private final ConfigProperties otelSdkProperties;
-  private final ConfigProperties customizedResourceProviderProperties;
+  private final ConfigProperties customizedListProperties;
 
-  private static final Map<String, Function<SpringOtelProperties, List<String>>> LIST_PROPERTIES =
+  private static final Map<String, Function<OtelSpringProperties, List<String>>> LIST_PROPERTIES =
       new HashMap<>();
 
   static final String DISABLED_KEY = "otel.java.disabled.resource.providers";
   static final String ENABLED_KEY = "otel.java.enabled.resource.providers";
 
   static {
-    LIST_PROPERTIES.put(ENABLED_KEY, SpringOtelProperties::getJavaEnabledResourceProviders);
-    LIST_PROPERTIES.put(DISABLED_KEY, SpringOtelProperties::getJavaDisabledResourceProviders);
+    LIST_PROPERTIES.put(ENABLED_KEY, OtelSpringProperties::getJavaEnabledResourceProviders);
+    LIST_PROPERTIES.put(DISABLED_KEY, OtelSpringProperties::getJavaDisabledResourceProviders);
     LIST_PROPERTIES.put(
         "otel.experimental.metrics.view.config",
-        SpringOtelProperties::getExperimentalMetricsViewConfig);
+        OtelSpringProperties::getExperimentalMetricsViewConfig);
     LIST_PROPERTIES.put(
         "otel.experimental.resource.disabled.keys",
-        SpringOtelProperties::getExperimentalResourceDisabledKeys);
-    LIST_PROPERTIES.put("otel.propagators", SpringOtelProperties::getPropagators);
+        OtelSpringProperties::getExperimentalResourceDisabledKeys);
+    LIST_PROPERTIES.put("otel.propagators", OtelSpringProperties::getPropagators);
   }
 
   public SpringConfigProperties(
       Environment environment,
       ExpressionParser parser,
       OtlpExporterProperties otlpExporterProperties,
       OtelResourceProperties resourceProperties,
-      SpringOtelProperties springOtelProperties,
+      OtelSpringProperties otelSpringProperties,
       ConfigProperties otelSdkProperties) {
     this.environment = environment;
     this.parser = parser;
     this.otlpExporterProperties = otlpExporterProperties;
     this.resourceProperties = resourceProperties;
-    this.springOtelProperties = springOtelProperties;
+    this.otelSpringProperties = otelSpringProperties;
     this.otelSdkProperties = otelSdkProperties;
-    this.customizedResourceProviderProperties =
-        createCustomizedProperties(otelSdkProperties, springOtelProperties);
+    this.customizedListProperties =
+        createCustomizedListProperties(otelSdkProperties, otelSpringProperties);
   }
 
-  private static void addList(
-      Map<String, String> map, String key, List<String> list, ConfigProperties configProperties) {
-    if (!list.isEmpty()) {
-      map.put(key, String.join(",", list));
+  private static Map<String, String> addList(
+      String key, List<String> springList, ConfigProperties configProperties) {
+    if (!springList.isEmpty()) {
+      return Collections.singletonMap(key, String.join(",", springList));
     } else {
-      String defaultValue = configProperties.getString(key);
-      if (defaultValue != null) {
-        map.put(key, defaultValue);
+      String otelList = configProperties.getString(key);
+      if (otelList != null) {
+        return Collections.singletonMap(key, otelList);
       }
     }
+    return Collections.emptyMap();
   }
 
-  private static ConfigProperties createCustomizedProperties(
-      ConfigProperties configProperties, SpringOtelProperties springOtelProperties) {
-    // find a better way to do this
-    Map<String, String> map = new HashMap<>();
-    addList(
-        map, ENABLED_KEY, springOtelProperties.getJavaEnabledResourceProviders(), configProperties);
-    addList(
-        map,
-        DISABLED_KEY,
-        springOtelProperties.getJavaDisabledResourceProviders(),
-        configProperties);
+  private static ConfigProperties createCustomizedListProperties(
+      ConfigProperties configProperties, OtelSpringProperties otelSpringProperties) {
+    // io.opentelemetry.instrumentation.resources.ResourceProviderPropertiesCustomizer
+    // has already been applied before this point, so we have to apply the same logic here
+    Map<String, String> map =
+        new HashMap<>(
+            addList(
+                ENABLED_KEY,
+                otelSpringProperties.getJavaEnabledResourceProviders(),
+                configProperties));
+    map.putAll(
+        addList(
+            DISABLED_KEY,
+            otelSpringProperties.getJavaDisabledResourceProviders(),
+            configProperties));
 
     return DefaultConfigProperties.createFromMap(
         new ResourceProviderPropertiesCustomizer()
@@ -104,14 +109,14 @@ public static ConfigProperties create(
       Environment env,
       OtlpExporterProperties otlpExporterProperties,
       OtelResourceProperties resourceProperties,
-      SpringOtelProperties springOtelProperties,
+      OtelSpringProperties otelSpringProperties,
       ConfigProperties fallback) {
     return new SpringConfigProperties(
         env,
         new SpelExpressionParser(),
         otlpExporterProperties,
         resourceProperties,
-        springOtelProperties,
+        otelSpringProperties,
         fallback);
   }
 
@@ -166,13 +171,13 @@ public List<String> getList(String name) {
 
     String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(name);
 
-    Function<SpringOtelProperties, List<String>> get = LIST_PROPERTIES.get(normalizedName);
-    if (get != null) {
-      List<String> c = customizedResourceProviderProperties.getList(name);
+    Function<OtelSpringProperties, List<String>> getList = LIST_PROPERTIES.get(normalizedName);
+    if (getList != null) {
+      List<String> c = customizedListProperties.getList(name);
       if (!c.isEmpty()) {
         return c;
       }
-      return get.apply(springOtelProperties);
+      return getList.apply(otelSpringProperties);
     }
 
     return or(environment.getProperty(normalizedName, List.class), otelSdkProperties.getList(name));

instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/OtlpExporterPropertiesTest.java

@@ -92,7 +92,7 @@ private static ConfigProperties getConfig(AssertableApplicationContext context)
         new SpelExpressionParser(),
         context.getBean(OtlpExporterProperties.class),
         new OtelResourceProperties(),
-        new SpringOtelProperties(),
+        new OtelSpringProperties(),
         DefaultConfigProperties.createFromMap(Collections.emptyMap()));
   }
 }

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

@@ -10,9 +10,9 @@
 
 import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtelResourceProperties;
+import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtelSpringProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtlpExporterProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.SpringConfigProperties;
-import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.SpringOtelProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
 import java.util.HashMap;
 import java.util.Map;
@@ -51,7 +51,7 @@ void shouldInitializeAttributesByMapInArow() {
                       new SpelExpressionParser(),
                       context.getBean(OtlpExporterProperties.class),
                       context.getBean(OtelResourceProperties.class),
-                      context.getBean(SpringOtelProperties.class),
+                      context.getBean(OtelSpringProperties.class),
                       DefaultConfigProperties.createFromMap(fallback));
 
               assertThat(config.getMap("otel.resource.attributes"))

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

@@ -9,9 +9,9 @@
 
 import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtelResourceProperties;
+import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtelSpringProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtlpExporterProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.SpringConfigProperties;
-import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.SpringOtelProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
 import io.opentelemetry.sdk.testing.assertj.AttributesAssert;
@@ -68,7 +68,7 @@ private static AttributesAssert assertResourceAttributes(AssertableApplicationCo
             context.getBean(Environment.class),
             new OtlpExporterProperties(),
             new OtelResourceProperties(),
-            new SpringOtelProperties(),
+            new OtelSpringProperties(),
             DefaultConfigProperties.createFromMap(Collections.emptyMap()));
 
     return assertThat(

smoke-tests-otel-starter/spring-boot-common/src/main/java/io/opentelemetry/spring/smoketest/AbstractOtelSpringStarterSmokeTest.java

@@ -15,7 +15,7 @@
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtelResourceProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtlpExporterProperties;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.SpringConfigProperties;
-import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.SpringOtelProperties;
+import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.OtelSpringProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
@@ -59,7 +59,7 @@ class AbstractOtelSpringStarterSmokeTest extends AbstractSpringStarterSmokeTest
   @Autowired private TestRestTemplate testRestTemplate;
 
   @Autowired private Environment environment;
-  @Autowired private SpringOtelProperties springOtelProperties;
+  @Autowired private OtelSpringProperties otelSpringProperties;
   @Autowired private OtelResourceProperties otelResourceProperties;
   @Autowired private OtlpExporterProperties otlpExporterProperties;
   @Autowired private RestTemplateBuilder restTemplateBuilder;
@@ -128,7 +128,7 @@ void propertyConversion() {
             environment,
             otlpExporterProperties,
             otelResourceProperties,
-            springOtelProperties,
+            otelSpringProperties,
             DefaultConfigProperties.createFromMap(
                 Collections.singletonMap("otel.exporter.otlp.headers", "a=1,b=2")));
     assertThat(configProperties.getMap("otel.exporter.otlp.headers"))