Allow vendor distribution to provide default configuration (#1243)

javaagent-spi/src/main/java/io/opentelemetry/javaagent/spi/config/PropertySource.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.opentelemetry.javaagent.spi.config;
+
+import java.util.Map;
+
+/**
+ * A service provider that allows to override default OTel agent configuration. Properties returned
+ * by implementations of this interface will be used after the following methods fail to find a
+ * non-empty property value: system properties, environment variables, properties configuration
+ * file.
+ */
+public interface PropertySource {
+  /**
+   * @return all properties whose default values are overridden by this property source. Key of the
+   *     map is the propertyName (same as system property name, e.g. {@code otel.exporter}), value
+   *     is the property value.
+   */
+  Map<String, String> getProperties();
+}

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

@@ -78,9 +78,13 @@ public final class ConfigBuilder
     return configMap;
   }
 
-  ConfigBuilder readPropertiesFromAllSources(Properties configurationFile) {
+  ConfigBuilder readPropertiesFromAllSources(
+      Properties spiConfiguration, Properties configurationFile) {
     // ordering from least to most important
-    return readProperties(configurationFile).readEnvironmentVariables().readSystemProperties();
+    return readProperties(spiConfiguration)
+        .readProperties(configurationFile)
+        .readEnvironmentVariables()
+        .readSystemProperties();
   }
 
   @Override

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

@@ -17,11 +17,14 @@
 package io.opentelemetry.javaagent.tooling.config;
 
 import io.opentelemetry.instrumentation.api.config.Config;
+import io.opentelemetry.javaagent.spi.config.PropertySource;
+import io.opentelemetry.javaagent.tooling.AgentInstaller;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.FileReader;
 import java.io.IOException;
 import java.util.Properties;
+import java.util.ServiceLoader;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -33,7 +36,19 @@ public final class ConfigInitializer {
 
   public static void initialize() {
     Config.internalInitializeConfig(
-        new ConfigBuilder().readPropertiesFromAllSources(loadConfigurationFile()).build());
+        new ConfigBuilder()
+            .readPropertiesFromAllSources(loadSpiConfiguration(), loadConfigurationFile())
+            .build());
+  }
+
+  /** Retrieves all default configuration overloads using SPI and initializes Config. */
+  private static Properties loadSpiConfiguration() {
+    Properties propertiesFromSpi = new Properties();
+    for (PropertySource propertySource :
+        ServiceLoader.load(PropertySource.class, AgentInstaller.class.getClassLoader())) {
+      propertiesFromSpi.putAll(propertySource.getProperties());
+    }
+    return propertiesFromSpi;
   }
 
   /**

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

@@ -31,7 +31,7 @@ class ConfigBuilderTest extends AgentSpecification {
   def "should use defaults"() {
     when:
     def config = new ConfigBuilder()
-      .readPropertiesFromAllSources(new Properties())
+      .readPropertiesFromAllSources(new Properties(), new Properties())
       .build()
 
     then:
@@ -53,66 +53,105 @@ class ConfigBuilderTest extends AgentSpecification {
     config.endpointPeerServiceMapping.isEmpty()
   }
 
-  def "should use configuration file properties (takes precedence over defaults)"() {
+  def "should use configuration from SPI (takes precedence over defaults)"() {
     given:
-    def configurationFile = new Properties()
+    def spiConfiguration = new Properties()
-    configurationFile.put(ConfigBuilder.EXPORTER, "zipkin")
+    spiConfiguration.put(ConfigBuilder.EXPORTER, "zipkin")
-    configurationFile.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true")
+    spiConfiguration.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true")
-    configurationFile.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs")
+    spiConfiguration.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs")
+    spiConfiguration.put(ConfigBuilder.TRACE_METHODS, "mypackage.MyClass[myMethod]")
 
     when:
     def config = new ConfigBuilder()
-      .readPropertiesFromAllSources(configurationFile)
+      .readPropertiesFromAllSources(spiConfiguration, new Properties())
       .build()
 
     then:
     config.exporter == "zipkin"
     config.hystrixTagsEnabled
     config.endpointPeerServiceMapping == ["1.2.3.4": "cats", "dogs.com": "dogs"]
+    config.traceMethods == "mypackage.MyClass[myMethod]"
   }
 
-  def "should use environment variables (takes precedence over configuration file)"() {
+  def "should use configuration file properties (takes precedence over SPI)"() {
     given:
-    def configurationFile = new Properties()
+    def spiConfiguration = new Properties()
-    configurationFile.put(ConfigBuilder.EXPORTER, "zipkin")
+    spiConfiguration.put(ConfigBuilder.EXPORTER, "zipkin")
-    configurationFile.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true")
+    spiConfiguration.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true")
-    configurationFile.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs")
+    spiConfiguration.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs")
+    spiConfiguration.put(ConfigBuilder.TRACE_METHODS, "mypackage.MyClass[myMethod]")
 
-    environmentVariables.set("OTEL_EXPORTER", "logging")
+    def configurationFile = new Properties()
-    environmentVariables.set("OTEL_ENDPOINT_PEER_SERVICE_MAPPING", "4.2.4.2=elephants.com")
+    configurationFile.put(ConfigBuilder.EXPORTER, "logging")
+    configurationFile.put(ConfigBuilder.TRACE_METHODS, "mypackage2.MyClass2[myMethod2]")
 
     when:
     def config = new ConfigBuilder()
-      .readPropertiesFromAllSources(configurationFile)
+      .readPropertiesFromAllSources(spiConfiguration, configurationFile)
       .build()
 
     then:
     config.exporter == "logging"
     config.hystrixTagsEnabled
-    config.endpointPeerServiceMapping == ["4.2.4.2": "elephants.com"]
+    config.endpointPeerServiceMapping == ["1.2.3.4": "cats", "dogs.com": "dogs"]
+    config.traceMethods == "mypackage2.MyClass2[myMethod2]"
   }
 
-  def "should use system properties (takes precedence over environment variables)"() {
+  def "should use environment variables (takes precedence over configuration file)"() {
     given:
+    def spiConfiguration = new Properties()
+    spiConfiguration.put(ConfigBuilder.EXPORTER, "zipkin")
+    spiConfiguration.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true")
+    spiConfiguration.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs")
+    spiConfiguration.put(ConfigBuilder.TRACE_METHODS, "mypackage.MyClass[myMethod]")
+
     def configurationFile = new Properties()
-    configurationFile.put(ConfigBuilder.EXPORTER, "zipkin")
+    configurationFile.put(ConfigBuilder.EXPORTER, "logging")
-    configurationFile.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true")
+    configurationFile.put(ConfigBuilder.TRACE_METHODS, "mypackage2.MyClass2[myMethod2]")
-    configurationFile.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs")
 
-    environmentVariables.set("OTEL_EXPORTER", "logging")
+    environmentVariables.set("OTEL_EXPORTER", "jaeger")
     environmentVariables.set("OTEL_ENDPOINT_PEER_SERVICE_MAPPING", "4.2.4.2=elephants.com")
 
-    System.setProperty(ConfigBuilder.EXPORTER, "jaeger")
-
     when:
     def config = new ConfigBuilder()
-      .readPropertiesFromAllSources(configurationFile)
+      .readPropertiesFromAllSources(spiConfiguration, configurationFile)
       .build()
 
     then:
     config.exporter == "jaeger"
     config.hystrixTagsEnabled
     config.endpointPeerServiceMapping == ["4.2.4.2": "elephants.com"]
+    config.traceMethods == "mypackage2.MyClass2[myMethod2]"
+  }
+
+  def "should use system properties (takes precedence over environment variables)"() {
+    given:
+    def spiConfiguration = new Properties()
+    spiConfiguration.put(ConfigBuilder.EXPORTER, "zipkin")
+    spiConfiguration.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true")
+    spiConfiguration.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs")
+    spiConfiguration.put(ConfigBuilder.TRACE_METHODS, "mypackage.MyClass[myMethod]")
+
+    def configurationFile = new Properties()
+    configurationFile.put(ConfigBuilder.EXPORTER, "logging")
+    configurationFile.put(ConfigBuilder.TRACE_METHODS, "mypackage2.MyClass2[myMethod2]")
+
+    environmentVariables.set("OTEL_EXPORTER", "jaeger")
+    environmentVariables.set("OTEL_ENDPOINT_PEER_SERVICE_MAPPING", "4.2.4.2=elephants.com")
+
+    System.setProperty(ConfigBuilder.EXPORTER, "otlp")
+    System.setProperty(ConfigBuilder.TRACE_METHODS, "mypackage3.MyClass3[myMethod3]")
+
+    when:
+    def config = new ConfigBuilder()
+      .readPropertiesFromAllSources(spiConfiguration, configurationFile)
+      .build()
+
+    then:
+    config.exporter == "otlp"
+    config.hystrixTagsEnabled
+    config.endpointPeerServiceMapping == ["4.2.4.2": "elephants.com"]
+    config.traceMethods == "mypackage3.MyClass3[myMethod3]"
   }
 
   def "should use defaults in case of parsing failure"() {
@@ -121,7 +160,7 @@ class ConfigBuilderTest extends AgentSpecification {
 
     when:
     def config = new ConfigBuilder()
-      .readPropertiesFromAllSources(new Properties())
+      .readPropertiesFromAllSources(new Properties(), new Properties())
       .build()
 
     then:
@@ -139,7 +178,7 @@ class ConfigBuilderTest extends AgentSpecification {
     System.setProperty("otel.integration.disabled-prop.enabled", "false")
 
     def config = new ConfigBuilder()
-      .readPropertiesFromAllSources(new Properties())
+      .readPropertiesFromAllSources(new Properties(), new Properties())
       .build()
 
     expect: