Fix the order of configuration file application (open-telemetry#6697)

Fixes open-telemetry#6696

javaagent-tooling/build.gradle.kts

@@ -54,6 +54,7 @@ dependencies {
 
   testImplementation(project(":testing-common"))
   testImplementation("com.google.guava:guava")
+  testImplementation("org.junit-pioneer:junit-pioneer")
 }
 
 testing {

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java

@@ -15,7 +15,6 @@
 import io.opentelemetry.javaagent.bootstrap.AgentInitializer;
 import io.opentelemetry.javaagent.bootstrap.AgentLogEmitterProvider;
 import io.opentelemetry.javaagent.bootstrap.OpenTelemetrySdkAccess;
-import io.opentelemetry.javaagent.tooling.config.ConfigurationFileLoader;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder;
@@ -31,11 +30,9 @@ public final class OpenTelemetryInstaller {
    *
    * @return the {@link AutoConfiguredOpenTelemetrySdk}
    */
-  static AutoConfiguredOpenTelemetrySdk installOpenTelemetrySdk() {
+  public static AutoConfiguredOpenTelemetrySdk installOpenTelemetrySdk() {
     AutoConfiguredOpenTelemetrySdkBuilder builder =
-        AutoConfiguredOpenTelemetrySdk.builder()
-            .setResultAsGlobal(true)
-            .addPropertiesSupplier(new ConfigurationFileLoader());
+        AutoConfiguredOpenTelemetrySdk.builder().setResultAsGlobal(true);
 
     ClassLoader classLoader = AgentInitializer.getExtensionsClassLoader();
     if (classLoader != null) {

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigurationFileLoader.java

@@ -8,7 +8,10 @@
 import static java.util.Collections.emptyMap;
 import static java.util.logging.Level.SEVERE;
 
+import com.google.auto.service.AutoService;
 import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
+import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizer;
+import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -17,19 +20,23 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Map;
 import java.util.Properties;
-import java.util.function.Supplier;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
-public final class ConfigurationFileLoader implements Supplier<Map<String, String>> {
+@AutoService(AutoConfigurationCustomizerProvider.class)
+public final class ConfigurationFileLoader implements AutoConfigurationCustomizerProvider {
 
   private static final Logger logger = Logger.getLogger(ConfigurationFileLoader.class.getName());
 
   static final String CONFIGURATION_FILE_PROPERTY = "otel.javaagent.configuration-file";
 
   @Override
-  public Map<String, String> get() {
+  public void customize(AutoConfigurationCustomizer autoConfiguration) {
+    autoConfiguration.addPropertiesSupplier(ConfigurationFileLoader::loadConfigFile);
+  }
 
+  // visible for tests
+  static Map<String, String> loadConfigFile() {
     // Reading from system property first and from env after
     String configurationFilePath = ConfigPropertiesUtil.getString(CONFIGURATION_FILE_PROPERTY);
     if (configurationFilePath == null) {
@@ -63,4 +70,10 @@ public Map<String, String> get() {
     return properties.entrySet().stream()
         .collect(Collectors.toMap(e -> e.getKey().toString(), e -> e.getValue().toString()));
   }
+
+  @Override
+  public int order() {
+    // make sure it runs after all the user-provided customizers
+    return Integer.MAX_VALUE;
+  }
 }

javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/config/ConfigurationFileTest.groovy

@@ -22,15 +22,13 @@ class ConfigurationFileTest extends Specification {
   @Shared
   public File tmpDir
 
-  ConfigurationFileLoader loader = new ConfigurationFileLoader()
-
   def "should use env property"() {
     given:
     def path = createFile("config", "property1=val-env")
     environmentVariables.set("OTEL_JAVAAGENT_CONFIGURATION_FILE", path)
 
     when:
-    def properties = loader.get()
+    def properties = ConfigurationFileLoader.loadConfigFile()
 
     then:
     properties.get("property1") == "val-env"
@@ -42,7 +40,7 @@ class ConfigurationFileTest extends Specification {
     System.setProperty("otel.javaagent.configuration-file", path)
 
     when:
-    def properties = loader.get()
+    def properties = ConfigurationFileLoader.loadConfigFile()
 
     then:
     properties.get("property1") == "val-sys"
@@ -57,7 +55,7 @@ class ConfigurationFileTest extends Specification {
     System.setProperty("otel.javaagent.configuration-file", pathSys)
 
     when:
-    def properties = loader.get()
+    def properties = ConfigurationFileLoader.loadConfigFile()
 
     then:
     properties.get("property1") == "val-sys"
@@ -69,15 +67,15 @@ class ConfigurationFileTest extends Specification {
     environmentVariables.set("OTEL_JAVAAGENT_CONFIGURATION_FILE", "somePath")
 
     when:
-    def properties = loader.get()
+    def properties = ConfigurationFileLoader.loadConfigFile()
 
     then:
     properties.isEmpty()
   }
 
   def "should return empty properties if property is not set"() {
     when:
-    def properties = loader.get()
+    def properties = ConfigurationFileLoader.loadConfigFile()
 
     then:
     properties.isEmpty()

javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/config/ConfigurationFileLoaderTest.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.tooling.config;
+
+import static java.util.Collections.singleton;
+import static java.util.Collections.singletonMap;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.javaagent.tooling.OpenTelemetryInstaller;
+import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
+import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizer;
+import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junitpioneer.jupiter.ClearSystemProperty;
+import org.junitpioneer.jupiter.SetSystemProperty;
+
+@ClearSystemProperty(key = ConfigurationFileLoader.CONFIGURATION_FILE_PROPERTY)
+class ConfigurationFileLoaderTest {
+
+  @BeforeAll
+  @AfterAll
+  static void cleanUp() {
+    GlobalOpenTelemetry.resetForTest();
+  }
+
+  // regression for https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/6696
+  @SetSystemProperty(key = "otel.experimental.sdk.enabled", value = "false") // don't setup the SDK
+  @Test
+  void fileConfigOverwritesUserPropertiesSupplier(@TempDir Path tempDir) throws IOException {
+    // given
+    Path configFile = tempDir.resolve("test-config.properties");
+    Files.write(configFile, singleton("custom.key = 42"));
+    System.setProperty(ConfigurationFileLoader.CONFIGURATION_FILE_PROPERTY, configFile.toString());
+
+    // when
+    AutoConfiguredOpenTelemetrySdk autoConfiguredSdk =
+        OpenTelemetryInstaller.installOpenTelemetrySdk();
+
+    // then
+    assertThat(autoConfiguredSdk.getConfig().getString("custom.key")).isEqualTo("42");
+  }
+
+  // SPI used in test
+  public static class UserCustomPropertiesSupplier implements AutoConfigurationCustomizerProvider {
+
+    @Override
+    public void customize(AutoConfigurationCustomizer autoConfiguration) {
+      autoConfiguration.addPropertiesSupplier(() -> singletonMap("custom.key", "123"));
+    }
+  }
+}

javaagent-tooling/src/test/resources/META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider

@@ -0,0 +1,2 @@
+io.opentelemetry.javaagent.tooling.config.ConfigurationFileLoader
+io.opentelemetry.javaagent.tooling.config.ConfigurationFileLoaderTest$UserCustomPropertiesSupplier