improve handling of general properties, signal if declarative config is available at all

Author: zeitlinger

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/config/internal/CommonConfig.java

@@ -7,13 +7,17 @@
 
 import static java.util.Collections.emptyMap;
 
+import io.opentelemetry.api.incubator.config.ConfigProvider;
+import io.opentelemetry.api.incubator.config.InstrumentationConfigUtil;
 import io.opentelemetry.instrumentation.api.incubator.log.LoggingContextConstants;
 import io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceResolver;
 import io.opentelemetry.instrumentation.api.internal.HttpConstants;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
 /**
  * This class is internal and is hence not for public use. Its APIs are unstable and can change at
@@ -39,16 +43,32 @@ public final class CommonConfig {
   public CommonConfig(InstrumentationConfig config) {
     peerServiceResolver =
         PeerServiceResolver.create(
-            config.getMap("otel.instrumentation.common.peer-service-mapping", emptyMap()));
+            getFromConfigProviderOrFallback(
+                config,
+                InstrumentationConfigUtil::peerServiceMapping,
+                () ->
+                    config.getMap("otel.instrumentation.common.peer-service-mapping", emptyMap())));
 
     clientRequestHeaders =
-        config.getList("otel.instrumentation.http.client.capture-request-headers");
+        getFromConfigProviderOrFallback(
+            config,
+            InstrumentationConfigUtil::httpClientRequestCapturedHeaders,
+            () -> config.getList("otel.instrumentation.http.client.capture-request-headers"));
     clientResponseHeaders =
-        config.getList("otel.instrumentation.http.client.capture-response-headers");
+        getFromConfigProviderOrFallback(
+            config,
+            InstrumentationConfigUtil::httpClientResponseCapturedHeaders,
+            () -> config.getList("otel.instrumentation.http.client.capture-response-headers"));
     serverRequestHeaders =
-        config.getList("otel.instrumentation.http.server.capture-request-headers");
+        getFromConfigProviderOrFallback(
+            config,
+            InstrumentationConfigUtil::httpServerRequestCapturedHeaders,
+            () -> config.getList("otel.instrumentation.http.server.capture-request-headers"));
     serverResponseHeaders =
-        config.getList("otel.instrumentation.http.server.capture-response-headers");
+        getFromConfigProviderOrFallback(
+            config,
+            InstrumentationConfigUtil::httpSeverResponseCapturedHeaders,
+            () -> config.getList("otel.instrumentation.http.server.capture-response-headers"));
     knownHttpRequestMethods =
         new HashSet<>(
             config.getList(
@@ -130,4 +150,15 @@ public String getSpanIdKey() {
   public String getTraceFlagsKey() {
     return loggingTraceFlagsKey;
   }
+
+  private static <T> T getFromConfigProviderOrFallback(
+      InstrumentationConfig config,
+      Function<ConfigProvider, T> getFromConfigProvider,
+      Supplier<T> fallback) {
+    ConfigProvider configProvider = config.getConfigProvider();
+    if (configProvider != null) {
+      return getFromConfigProvider.apply(configProvider);
+    }
+    return fallback.get();
+  }
 }

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/config/internal/InstrumentationConfig.java

@@ -7,6 +7,7 @@
 
 import static java.util.Collections.emptyList;
 
+import io.opentelemetry.api.incubator.config.ConfigProvider;
 import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
 import java.time.Duration;
 import java.util.List;
@@ -111,15 +112,23 @@ default List<String> getList(String name) {
 
   /**
    * Returns a {@link DeclarativeConfigProperties} for the given instrumentation name, or {@code
-   * null} if no declarative configuration is available for that instrumentation.
+   * null} if no declarative configuration <b>not used at all</b>.
    *
    * <p>Declarative configuration is used to configure instrumentation properties in a declarative
    * way, such as through YAML or JSON files.
    *
-   * @param instrumentationName the name of the instrumentation
-   * @return the declarative configuration properties for the given instrumentation name, or {@code
-   *     null} if not available
+   * @param node the name of the instrumentation (e.g. "log4j"), the vendor name (e.g. "google"), or
+   *     "common" for common Java settings that don't apply to other languages
+   * @return the declarative configuration properties for the given node name, or {@code * null} if
+   *     no declarative configuration <b>not used at all</b>.
+   */
+  DeclarativeConfigProperties getDeclarativeConfig(String node);
+
+  /**
+   * Returns the {@link ConfigProvider} if declarative configuration is available,
+   *
+   * @return the {@link ConfigProvider} or {@code null} if no provider is available
    */
   @Nullable
-  DeclarativeConfigProperties getDeclarativeConfig(String instrumentationName);
+  ConfigProvider getConfigProvider();
 }

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

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties;
 
+import io.opentelemetry.api.incubator.config.ConfigProvider;
 import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
 import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
@@ -112,7 +113,14 @@ public Map<String, String> getMap(String name, Map<String, String> defaultValue)
 
   @Nullable
   @Override
-  public DeclarativeConfigProperties getDeclarativeConfig(String instrumentationName) {
+  public DeclarativeConfigProperties getDeclarativeConfig(String node) {
+    // create a spring boot bridge for DeclarativeConfigProperties
+    return null;
+  }
+
+  @Nullable
+  @Override
+  public ConfigProvider getConfigProvider() {
     // create a spring boot bridge for DeclarativeConfigProperties
     return null;
   }

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/bootstrap/internal/EmptyInstrumentationConfig.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.bootstrap.internal;
 
+import io.opentelemetry.api.incubator.config.ConfigProvider;
 import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
 import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
 import java.time.Duration;
@@ -62,7 +63,13 @@ public Map<String, String> getMap(String name, Map<String, String> defaultValue)
 
   @Nullable
   @Override
-  public DeclarativeConfigProperties getDeclarativeConfig(String instrumentationName) {
+  public DeclarativeConfigProperties getDeclarativeConfig(String node) {
+    return null;
+  }
+
+  @Nullable
+  @Override
+  public ConfigProvider getConfigProvider() {
     return null;
   }
 }

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

@@ -15,9 +15,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.function.BiFunction;
-import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /**
@@ -58,52 +56,19 @@ public final class DeclarativeConfigPropertiesBridge implements ConfigProperties
   private static final String OTEL_JAVA_AGENT_PREFIX = "otel.javaagent.";
 
   private static final Map<String, String> JAVA_MAPPING_RULES = new HashMap<>();
-  private static final Map<String, String> GENERAL_MAPPING_RULES = new HashMap<>();
 
   // The node at .instrumentation.java
   private final DeclarativeConfigProperties instrumentationJavaNode;
 
-  private final DeclarativeConfigProperties instrumentationGeneralNode;
-
   static {
     JAVA_MAPPING_RULES.put("otel.instrumentation.common.default-enabled", "common.default.enabled");
     JAVA_MAPPING_RULES.put(
         "otel.javaagent.logging.application.logs-buffer-max-records",
         "agent.logging.output.application.logs_buffer_max_records");
-
-    // todo not supported in SDK yet (this is strictly typed)
-    //    GENERAL_MAPPING_RULES.put("otel.instrumentation.http.known-methods",
-    // "http.known_methods");
-    GENERAL_MAPPING_RULES.put(
-        "otel.instrumentation.http.client.capture-request-headers",
-        "http.client.request_captured_headers");
-    GENERAL_MAPPING_RULES.put(
-        "otel.instrumentation.http.client.capture-response-headers",
-        "http.client.response_captured_headers");
-    GENERAL_MAPPING_RULES.put(
-        "otel.instrumentation.http.server.capture-request-headers",
-        "http.server.request_captured_headers");
-    GENERAL_MAPPING_RULES.put(
-        "otel.instrumentation.http.server.capture-response-headers",
-        "http.server.response_captured_headers");
   }
 
   private final Map<String, Object> earlyInitProperties;
 
-  private static Map<String, String> getPeerServiceMapping(
-      DeclarativeConfigPropertiesBridge bridge) {
-    List<DeclarativeConfigProperties> configProperties =
-        bridge
-            .instrumentationGeneralNode
-            .getStructured("peer", empty())
-            .getStructuredList("service_mapping", Collections.emptyList());
-    return configProperties.stream()
-        .collect(
-            Collectors.toMap(
-                e -> Objects.requireNonNull(e.getString("peer"), "peer must not be null"),
-                e -> Objects.requireNonNull(e.getString("service"), "service must not be null")));
-  }
-
   public DeclarativeConfigPropertiesBridge(
       ConfigProvider configProvider, Map<String, Object> earlyInitProperties) {
     this.earlyInitProperties = earlyInitProperties;
@@ -112,7 +77,6 @@ public DeclarativeConfigPropertiesBridge(
       inst = DeclarativeConfigProperties.empty();
     }
     instrumentationJavaNode = inst.getStructured("java", empty());
-    instrumentationGeneralNode = inst.getStructured("general", empty());
   }
 
   @Nullable
@@ -174,10 +138,6 @@ public List<String> getList(String propertyName) {
 
   @Override
   public Map<String, String> getMap(String propertyName) {
-    if ("otel.instrumentation.common.peer-service-mapping".equals(propertyName)) {
-      return getPeerServiceMapping(this);
-    }
-
     DeclarativeConfigProperties propertyValue =
         getPropertyValue(propertyName, DeclarativeConfigProperties::getStructured);
     if (propertyValue == null) {
@@ -200,15 +160,7 @@ public Map<String, String> getMap(String propertyName) {
   @Nullable
   private <T> T getPropertyValue(
       String property, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
-    String generalPath = GENERAL_MAPPING_RULES.get(property);
-    if (generalPath != null) {
-      return splitOnDot(generalPath, instrumentationGeneralNode, extractor);
-    }
-    String javaPath = getJavaPath(property);
-    if (javaPath != null) {
-      return splitOnDot(javaPath, instrumentationJavaNode, extractor);
-    }
-    return null;
+    return splitOnDot(getJavaPath(property), instrumentationJavaNode, extractor);
   }
 
   private static <T> T splitOnDot(

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

@@ -5,6 +5,8 @@
 
 package io.opentelemetry.javaagent.tooling.config;
 
+import static io.opentelemetry.api.incubator.config.DeclarativeConfigProperties.empty;
+
 import io.opentelemetry.api.incubator.config.ConfigProvider;
 import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
 import io.opentelemetry.api.incubator.config.InstrumentationConfigUtil;
@@ -111,9 +113,26 @@ public Map<String, String> getMap(String name, Map<String, String> defaultValue)
 
   @Nullable
   @Override
-  public DeclarativeConfigProperties getDeclarativeConfig(String instrumentationName) {
-    return configProvider != null
-        ? InstrumentationConfigUtil.javaInstrumentationConfig(configProvider, instrumentationName)
-        : null;
+  public DeclarativeConfigProperties getDeclarativeConfig(String node) {
+    if (configProvider == null) {
+      // declarative config is not supported at all
+      return null;
+    }
+
+    DeclarativeConfigProperties config =
+        InstrumentationConfigUtil.javaInstrumentationConfig(configProvider, node);
+    if (config == null) {
+      // there is no declarative config for this node
+      // this needs to be a different value than null to avoid confusion with
+      // the case when declarative config is not supported at all
+      return empty();
+    }
+    return config;
+  }
+
+  @Nullable
+  @Override
+  public ConfigProvider getConfigProvider() {
+    return configProvider;
   }
 }