revert to using system props for early init with declarative config to avoid classloading issues

Author: zeitlinger

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java

@@ -8,18 +8,14 @@
 import static io.opentelemetry.api.incubator.config.DeclarativeConfigProperties.empty;
 
 import io.opentelemetry.api.incubator.config.ConfigProvider;
-import io.opentelemetry.api.incubator.config.DeclarativeConfigException;
 import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import java.time.Duration;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 import java.util.function.BiFunction;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
@@ -60,8 +56,6 @@ public final class DeclarativeConfigPropertiesBridge implements ConfigProperties
 
   private static final Map<String, String> JAVA_MAPPING_RULES = new HashMap<>();
   private static final Map<String, String> GENERAL_MAPPING_RULES = new HashMap<>();
-  private static final Set<String> AGENT_LOGGING_OUTPUTS =
-      new HashSet<>(Arrays.asList("application", "simple"));
 
   // The node at .instrumentation.java
   private final DeclarativeConfigProperties instrumentationJavaNode;
@@ -91,6 +85,8 @@ public final class DeclarativeConfigPropertiesBridge implements ConfigProperties
         "http.server.response_captured_headers");
   }
 
+  private final Map<String, Object> earlyInitProperties;
+
   private static Map<String, String> getPeerServiceMapping(
       DeclarativeConfigPropertiesBridge bridge) {
     List<DeclarativeConfigProperties> configProperties =
@@ -105,7 +101,9 @@ private static Map<String, String> getPeerServiceMapping(
                 e -> Objects.requireNonNull(e.getString("service"), "service must not be null")));
   }
 
-  public DeclarativeConfigPropertiesBridge(ConfigProvider configProvider) {
+  public DeclarativeConfigPropertiesBridge(
+      ConfigProvider configProvider, Map<String, Object> earlyInitProperties) {
+    this.earlyInitProperties = earlyInitProperties;
     DeclarativeConfigProperties inst = configProvider.getInstrumentationConfig();
     if (inst == null) {
       inst = DeclarativeConfigProperties.empty();
@@ -117,16 +115,20 @@ public DeclarativeConfigPropertiesBridge(ConfigProvider configProvider) {
   @Nullable
   @Override
   public String getString(String propertyName) {
-    if ("otel.javaagent.logging".equals(propertyName)) {
-      return agentLoggerName();
+    Object value = earlyInitProperties.get(propertyName);
+    if (value != null) {
+      return value.toString();
     }
-
     return getPropertyValue(propertyName, DeclarativeConfigProperties::getString);
   }
 
   @Nullable
   @Override
   public Boolean getBoolean(String propertyName) {
+    Object value = earlyInitProperties.get(propertyName);
+    if (value != null) {
+      return (Boolean) value;
+    }
     return getPropertyValue(propertyName, DeclarativeConfigProperties::getBoolean);
   }
 
@@ -237,40 +239,4 @@ private static String getJavaPath(String property) {
     }
     return null;
   }
-
-  @Nullable
-  private String agentLoggerName() {
-    DeclarativeConfigProperties logOutput = getLogOutput();
-    Set<String> names = logOutput.getPropertyKeys();
-
-    if (names.isEmpty()) {
-      // no log output configured
-      return null;
-    }
-
-    if (names.size() > 1) {
-      throw new DeclarativeConfigException(
-          "Multiple log output formats are configured: "
-              + String.join(", ", names)
-              + ". Please choose one of them.");
-    }
-
-    String name = names.iterator().next();
-    if (!AGENT_LOGGING_OUTPUTS.contains(name)) {
-      throw new DeclarativeConfigException(
-          "Unsupported log output format: '"
-              + name
-              + "' . Supported formats are: "
-              + String.join(", ", AGENT_LOGGING_OUTPUTS));
-    }
-    return name;
-  }
-
-  private DeclarativeConfigProperties getLogOutput() {
-    return getAgent().getStructured("logging", empty()).getStructured("output", empty());
-  }
-
-  private DeclarativeConfigProperties getAgent() {
-    return instrumentationJavaNode.getStructured("agent", empty());
-  }
 }

javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java

@@ -6,17 +6,16 @@
 package io.opentelemetry.javaagent.extension;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.data.MapEntry.entry;
 
-import io.opentelemetry.api.incubator.config.DeclarativeConfigException;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.DeclarativeConfiguration;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.SdkConfigProvider;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.InstrumentationModel;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel;
 import java.time.Duration;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -25,30 +24,29 @@
 import org.junit.jupiter.api.Test;
 
 class DeclarativeConfigPropertiesBridgeTest {
-
   private ConfigProperties bridge;
   private ConfigProperties emptyBridge;
 
   @BeforeEach
   void setup() {
-    bridge = create("config.yaml");
+    bridge = create("config.yaml", Collections.emptyMap());
 
     OpenTelemetryConfigurationModel emptyModel =
         new OpenTelemetryConfigurationModel()
             .withAdditionalProperty("instrumentation/development", new InstrumentationModel());
-    SdkConfigProvider emptyConfigProvider = SdkConfigProvider.create(emptyModel);
     emptyBridge =
-        new DeclarativeConfigPropertiesBridge(Objects.requireNonNull(emptyConfigProvider));
+        new DeclarativeConfigPropertiesBridge(
+            Objects.requireNonNull(SdkConfigProvider.create(emptyModel)), Collections.emptyMap());
   }
 
-  private static DeclarativeConfigPropertiesBridge create(String name) {
+  private static DeclarativeConfigPropertiesBridge create(
+      String name, Map<String, Object> earlyInitProperties) {
     OpenTelemetryConfigurationModel model =
         DeclarativeConfiguration.parse(
             DeclarativeConfigPropertiesBridgeTest.class.getClassLoader().getResourceAsStream(name));
     SdkConfigProvider configProvider = SdkConfigProvider.create(model);
-    DeclarativeConfigPropertiesBridge configPropertiesBridge =
-        new DeclarativeConfigPropertiesBridge(Objects.requireNonNull(configProvider));
-    return configPropertiesBridge;
+    return new DeclarativeConfigPropertiesBridge(
+        Objects.requireNonNull(configProvider), earlyInitProperties);
   }
 
   @Test
@@ -143,15 +141,13 @@ void common() {
 
   @Test
   void agent() {
+    Map<String, Object> earlyInitProperties = new HashMap<>();
+    earlyInitProperties.put("otel.javaagent.debug", true);
+    earlyInitProperties.put("otel.javaagent.logging", "application");
+    DeclarativeConfigPropertiesBridge bridge = create("config.yaml", earlyInitProperties);
+
     assertThat(bridge.getBoolean("otel.javaagent.debug")).isTrue();
     assertThat(bridge.getBoolean("otel.javaagent.experimental.indy")).isTrue();
-
     assertThat(bridge.getString("otel.javaagent.logging")).isEqualTo("application");
-    assertThat(bridge.getInt("otel.javaagent.logging.application.logs-buffer-max-records"))
-        .isEqualTo(1000);
-    assertThat(create("agent-logging-simple.yaml").getString("otel.javaagent.logging"))
-        .isEqualTo("simple");
-    assertThatThrownBy(() -> create("agent-logging-both.yaml").getString("otel.javaagent.logging"))
-        .isInstanceOf(DeclarativeConfigException.class);
   }
 }

javaagent-extension-api/src/test/resources/agent-logging-both.yaml

@@ -1,6 +1,4 @@
 file_format: 0.4
-disabled: true
-log_level: DEBUG
 instrumentation/development:
   general:
     peer:
@@ -32,10 +30,6 @@ instrumentation/development:
     agent:
       experimental:
         indy: true
-      logging:
-        output:
-          application:
-            logs_buffer_max_records: 1000
     common:
       default:
         enabled: false

javaagent-extension-api/src/test/resources/agent-logging-simple.yaml

@@ -19,6 +19,8 @@
 import io.opentelemetry.sdk.resources.Resource;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 import javax.annotation.Nullable;
 import org.jetbrains.annotations.NotNull;
 
@@ -70,7 +72,7 @@ public AutoConfiguredOpenTelemetrySdk installOpenTelemetrySdk(ClassLoader extens
     OpenTelemetryConfigurationModel model = loadConfigurationModel(configurationFile);
     SdkConfigProvider configProvider = SdkConfigProvider.create(model);
     DeclarativeConfigPropertiesBridge configProperties =
-        new DeclarativeConfigPropertiesBridge(configProvider);
+        new DeclarativeConfigPropertiesBridge(configProvider, getEarlyInitProperties());
     OpenTelemetrySdk sdk =
         DeclarativeConfiguration.create(
             model, SpiHelper.serviceComponentLoader(extensionClassLoader));
@@ -82,4 +84,13 @@ public AutoConfiguredOpenTelemetrySdk installOpenTelemetrySdk(ClassLoader extens
         sdk, Resource.getDefault(), configProperties, configProvider);
   }
 
+  // these properties are used to initialize the SDK before the configuration file is loaded
+  // for consistency, we pass them to the bridge, so that they can be read later with the same
+  // value from the {@link DeclarativeConfigPropertiesBridge}
+  private Map<String, Object> getEarlyInitProperties() {
+    Map<String, Object> earlyInitProperties = new HashMap<>();
+    earlyInitProperties.put("otel.javaagent.debug", this.getBoolean("otel.javaagent.debug", false));
+    earlyInitProperties.put("otel.javaagent.logging", this.getString("otel.javaagent.logging"));
+    return earlyInitProperties;
+  }
 }

javaagent-extension-api/src/test/resources/config.yaml

@@ -39,7 +39,7 @@ static EarlyInitAgentConfig create() {
 
     return configurationFile != null
         ? DeclarativeConfigEarlyInitAgentConfig.create(configurationFile)
-        : new LegacyConfigFileEarlyInitAgentConfig();
+        : LegacyConfigFileEarlyInitAgentConfig.create();
   }
 
   static void setForceFlush(OpenTelemetrySdk sdk) {

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

@@ -20,7 +20,11 @@ public final class LegacyConfigFileEarlyInitAgentConfig implements EarlyInitAgen
 
   private final Map<String, String> configFileContents;
 
-  LegacyConfigFileEarlyInitAgentConfig() {
+  public static LegacyConfigFileEarlyInitAgentConfig create() {
+    return new LegacyConfigFileEarlyInitAgentConfig();
+  }
+
+  private LegacyConfigFileEarlyInitAgentConfig() {
     this.configFileContents = ConfigurationFile.getProperties();
   }