[Maven-Extension] Use Auto Configure Otel SDK Builder (#132)

cyrille-leclerc · web-flow · commit 92998b67912a · 2021-11-27T12:06:55.000+09:00
* Use Auto Configuration Otel SDK Builder

* Cleanup

* Cleanup

* set default otel exporter to "none" rather than "otlp" if no otlp endpoint is defined

* Bump dependency

* Cleanup

* Cleanup

* fix bug, use uppercase for env vars

* Rely on the OkHttp impl of gRPC bundled with otel SDK 1.9

* Rollback renaming to ease PR review

* Rollback change to ease PR review

* formatting

* Simplify configuration logic: default `-Dotel.traces.exporter` / `OTEL_TRACES_EXPORTER` to `none

* spotless glitch

* Code cleanup
diff --git a/maven-extension/README.md b/maven-extension/README.md
@@ -18,7 +18,8 @@ Add the Maven OpenTelemetry Extension to `${maven.home}/lib/ext` or to the class
 ```
 mvn dependency:copy -Dartifact=io.opentelemetry.contrib:opentelemetry-maven-extension:1.7.0-alpha
 
-export OTEL_EXPORTER_OTLP_ENDPOINT="http://localhost:4317"
+export OTEL_TRACES_EXPORTER="otlp"
+export OTEL_EXPORTER_OTLP_ENDPOINT="http://otel.example.com:4317"
 
 mvn -Dmaven.ext.class.path=target/dependency/opentelemetry-maven-extension-1.7.0-alpha.jar verify
 ```
@@ -44,25 +45,32 @@ Add the Maven OpenTelemetry Extension in the `pom.xml` file:
 ```
 
 ```
-export OTEL_EXPORTER_OTLP_ENDPOINT="http://localhost:4317"
+export OTEL_TRACES_EXPORTER="otlp"
+export OTEL_EXPORTER_OTLP_ENDPOINT="http://otel.example.com:4317"
 
 mvn verify
 ```
 
 ## Configuration
 
+❕ The setting `-Dotel.traces.exporter` / `OTEL_TRACES_EXPORTER` MUST be defined for the Maven OpenTelemetry Extension to export traces.
+
+Without this setting, the traces won't be exported and the OpenTelemetry Maven Extension will behave as a NoOp extension. `otlp` is currently the only supported exporter.
+
 The Maven OpenTelemetry Extension supports a subset of the [OpenTelemetry auto configuration environment variables and JVM system properties](https://github.com/open-telemetry/opentelemetry-java/tree/main/sdk-extensions/autoconfigure).
 
-| System property              | Environment variable        | Description                                                               |
-|------------------------------|-----------------------------|---------------------------------------------------------------------------|
-| otel.exporter.otlp.endpoint  | OTEL_EXPORTER_OTLP_ENDPOINT | The OTLP traces and metrics endpoint to connect to. Must be a URL with a scheme of either `http` or `https` based on the use of TLS. Example `http://localhost:4317`.            |
-| otel.exporter.otlp.headers   | OTEL_EXPORTER_OTLP_HEADERS  | Key-value pairs separated by commas to pass as request headers on OTLP trace and metrics requests.        |
-| otel.exporter.otlp.timeout   | OTEL_EXPORTER_OTLP_TIMEOUT  | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. Default is `10000`.  |
-| otel.resource.attributes | OTEL_RESOURCE_ATTRIBUTES | Specify resource attributes in the following format: key1=val1,key2=val2,key3=val3 |
-| otel.instrumentation.maven.mojo.enabled | OTEL_INSTRUMENTATION_MAVEN_MOJO_ENABLED | Whether to create spans for mojo goal executions, `true` or `false`. Can be configured to reduce the number of spans created for large builds. Default value: `true` |
+
+| System property              | Environment variable        | Default value           | Description                                                               |
+|------------------------------|-----------------------------|-------------------------|---------------------------------------------------------------------------|
+| otel.traces.exporter         | OTEL_TRACES_EXPORTER        |  `none`          | Select the OpenTelemetry exporter for tracing, the currently only supported values are `none` and `otlp`. `none` makes the instrumentation NoOp |
+| otel.exporter.otlp.endpoint  | OTEL_EXPORTER_OTLP_ENDPOINT | `http://localhost:4317` | The OTLP traces and metrics endpoint to connect to. Must be a URL with a scheme of either `http` or `https` based on the use of TLS. |
+| otel.exporter.otlp.headers   | OTEL_EXPORTER_OTLP_HEADERS  |                         | Key-value pairs separated by commas to pass as request headers on OTLP trace and metrics requests. |
+| otel.exporter.otlp.timeout   | OTEL_EXPORTER_OTLP_TIMEOUT  | `10000`                 | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. |
+| otel.resource.attributes     | OTEL_RESOURCE_ATTRIBUTES    |                         | Specify resource attributes in the following format: key1=val1,key2=val2,key3=val3 |
+| otel.instrumentation.maven.mojo.enabled | OTEL_INSTRUMENTATION_MAVEN_MOJO_ENABLED | `true` | Whether to create spans for mojo goal executions, `true` or `false`. Can be configured to reduce the number of spans created for large builds. |
 
 
-ℹ️ The `service.name` is set by default to `maven`, it can be overwritten specifying resource attributes.
+ℹ️ The `service.name` is set to `maven` and the `service.version` to the version of the Maven runtime in use.
 
 
 ## Examples
diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java
@@ -10,11 +10,13 @@
 import io.opentelemetry.context.propagation.ContextPropagators;
 import io.opentelemetry.maven.semconv.MavenOtelSemanticAttributes;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
-import io.opentelemetry.sdk.autoconfigure.OpenTelemetrySdkAutoConfiguration;
+import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
+import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder;
 import io.opentelemetry.sdk.common.CompletableResultCode;
+import io.opentelemetry.sdk.resources.Resource;
 import io.opentelemetry.sdk.trace.export.SpanExporter;
 import io.opentelemetry.semconv.resource.attributes.ResourceAttributes;
-import java.util.Map;
+import java.util.Collections;
 import java.util.concurrent.TimeUnit;
 import org.apache.maven.rtinfo.RuntimeInformation;
 import org.codehaus.plexus.component.annotations.Component;
@@ -25,16 +27,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/**
- * Service to configure the {@link OpenTelemetry} instance.
- *
- * <p>Rely on the OpenTelemetry SDK AutoConfiguration extension. Parameters are passed as system
- * properties.
- *
- * <p>TODO: verify how we could use a composite {@link
- * io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties} combining the config passed by JVM
- * system properties and environment variables with overrides injected by the Otel Maven Extension
- */
+/** Service to configure the {@link OpenTelemetry} instance. */
 @Component(role = OpenTelemetrySdkService.class, hint = "opentelemetry-service")
 public final class OpenTelemetrySdkService implements Initializable, Disposable {
 
@@ -89,60 +82,49 @@ public synchronized void dispose() {
   @Override
   public void initialize() throws InitializationException {
     logger.debug("OpenTelemetry: initialize OpenTelemetrySdkService...");
-    if (StringUtils.isBlank(
-        OtelUtils.getSystemPropertyOrEnvironmentVariable(
-            "otel.exporter.otlp.endpoint", "OTEL_EXPORTER_OTLP_ENDPOINT", null))) {
+    AutoConfiguredOpenTelemetrySdkBuilder autoConfiguredSdkBuilder =
+        AutoConfiguredOpenTelemetrySdk.builder();
+
+    // SDK CONFIGURATION PROPERTIES
+    autoConfiguredSdkBuilder.addPropertiesSupplier(
+        () -> {
+          // Change default of "otel.traces.exporter" from "otlp" to "none"
+          // The impacts are
+          // * If no otel exporter settings are passed, then the Maven extension will not export
+          //   rather than exporting on OTLP GRPC to http://localhost:4317
+          // * If OTEL_EXPORTER_OTLP_ENDPOINT is defined but OTEL_TRACES_EXPORTER is not, then don't
+          //   export
+          return Collections.singletonMap("otel.traces.exporter", "none");
+        });
+
+    // SDK RESOURCE
+    // Don't use the `io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider` framework because we
+    // need to get the `RuntimeInformation` component injected by Plexus
+    autoConfiguredSdkBuilder.addResourceCustomizer(
+        (resource, configProperties) ->
+            Resource.builder()
+                .putAll(resource)
+                .put(
+                    ResourceAttributes.SERVICE_NAME, MavenOtelSemanticAttributes.SERVICE_NAME_VALUE)
+                .put(ResourceAttributes.SERVICE_VERSION, runtimeInformation.getMavenVersion())
+                .build());
+
+    // BUILD SDK
+    AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk =
+        autoConfiguredSdkBuilder.build();
+    if (logger.isDebugEnabled()) {
       logger.debug(
-          "OpenTelemetry: No -Dotel.exporter.otlp.endpoint property or OTEL_EXPORTER_OTLP_ENDPOINT "
-              + "environment variable found, use a NOOP OpenTelemetry SDK");
-    } else {
-      {
-        // Don't use a {@code io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider} to inject
-        // Maven runtime attributes due to a classloading issue when loading the Maven OpenTelemetry
-        // extension as a pom.xml {@code <extension>}.
-        String initialCommaSeparatedAttributes =
-            OtelUtils.getSystemPropertyOrEnvironmentVariable(
-                "otel.resource.attributes", "OTEL_RESOURCE_ATTRIBUTES", "");
-        Map<String, String> attributes =
-            OtelUtils.getCommaSeparatedMap(initialCommaSeparatedAttributes);
-
-        // service.name
-        String serviceName =
-            OtelUtils.getSystemPropertyOrEnvironmentVariable(
-                "otel.service.name", "OTEL_SERVICE_NAME", null);
-
-        if (!attributes.containsKey(ResourceAttributes.SERVICE_NAME.getKey())
-            && StringUtils.isBlank(serviceName)) {
-          // service.name is not defined in passed configuration, we define it
-          attributes.put(
-              ResourceAttributes.SERVICE_NAME.getKey(),
-              MavenOtelSemanticAttributes.ServiceNameValues.SERVICE_NAME_VALUE);
-        }
-
-        // service.version
-        final String mavenVersion = this.runtimeInformation.getMavenVersion();
-        if (!attributes.containsKey(ResourceAttributes.SERVICE_VERSION.getKey())) {
-          attributes.put(ResourceAttributes.SERVICE_VERSION.getKey(), mavenVersion);
-        }
-
-        String newCommaSeparatedAttributes = OtelUtils.getCommaSeparatedString(attributes);
-        logger.debug(
-            "OpenTelemetry: Initial resource attributes: {}", initialCommaSeparatedAttributes);
-        logger.debug("OpenTelemetry: Use resource attributes: {}", newCommaSeparatedAttributes);
-        System.setProperty("otel.resource.attributes", newCommaSeparatedAttributes);
-      }
-
-      this.openTelemetrySdk = OpenTelemetrySdkAutoConfiguration.initialize(false);
-      this.openTelemetry = this.openTelemetrySdk;
+          "OpenTelemetry: OpenTelemetry SDK initialized with  "
+              + OtelUtils.prettyPrintSdkConfiguration(autoConfiguredOpenTelemetrySdk));
     }
-
-    String mojosInstrumentationEnabledAsString =
-        System.getProperty(
-            "otel.instrumentation.maven.mojo.enabled",
-            System.getenv("OTEL_INSTRUMENTATION_MAVEN_MOJO_ENABLED"));
-    this.mojosInstrumentationEnabled =
-        Boolean.parseBoolean(
-            StringUtils.defaultIfBlank(mojosInstrumentationEnabledAsString, "true"));
+    this.openTelemetrySdk = autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk();
+    this.openTelemetry = this.openTelemetrySdk;
+
+    Boolean mojoSpansEnabled =
+        autoConfiguredOpenTelemetrySdk
+            .getConfig()
+            .getBoolean("otel.instrumentation.maven.mojo.enabled");
+    this.mojosInstrumentationEnabled = mojoSpansEnabled == null ? true : mojoSpansEnabled;
 
     this.tracer = this.openTelemetry.getTracer("io.opentelemetry.contrib.maven");
   }
diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OtelUtils.java b/maven-extension/src/main/java/io/opentelemetry/maven/OtelUtils.java
@@ -5,64 +5,46 @@
 
 package io.opentelemetry.maven;
 
-import java.util.AbstractMap;
+import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
+import io.opentelemetry.sdk.resources.Resource;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
-import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
 
 final class OtelUtils {
+  protected static String prettyPrintSdkConfiguration(
+      AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) {
+    List<String> configAttributeNames =
+        Arrays.asList(
+            "otel.traces.exporter",
+            "otel.metrics.exporter",
+            "otel.exporter.otlp.endpoint",
+            "otel.exporter.otlp.traces.endpoint",
+            "otel.exporter.otlp.metrics.endpoint",
+            "otel.exporter.jaeger.endpoint",
+            "otel.exporter.prometheus.port",
+            "otel.resource.attributes",
+            "otel.service.name");
 
-  public static String getCommaSeparatedString(Map<String, String> keyValuePairs) {
-    return keyValuePairs.entrySet().stream()
-        .map(keyValuePair -> keyValuePair.getKey() + "=" + keyValuePair.getValue())
-        .collect(Collectors.joining(","));
-  }
-
-  public static Map<String, String> getCommaSeparatedMap(String commaSeparatedKeyValuePairs) {
-    if (StringUtils.isBlank(commaSeparatedKeyValuePairs)) {
-      return new HashMap<>();
+    ConfigProperties sdkConfig = autoConfiguredOpenTelemetrySdk.getConfig();
+    Map<String, String> configurationAttributes = new LinkedHashMap<>();
+    for (String attributeName : configAttributeNames) {
+      final String attributeValue = sdkConfig.getString(attributeName);
+      if (attributeValue != null) {
+        configurationAttributes.put(attributeName, attributeValue);
+      }
     }
-    return filterBlanksAndNulls(commaSeparatedKeyValuePairs.split(",")).stream()
-        .map(keyValuePair -> filterBlanksAndNulls(keyValuePair.split("=", 2)))
-        .map(
-            splitKeyValuePairs -> {
-              if (splitKeyValuePairs.size() != 2) {
-                throw new RuntimeException(
-                    "Invalid key-value pair: " + commaSeparatedKeyValuePairs);
-              }
-              return new AbstractMap.SimpleImmutableEntry<>(
-                  splitKeyValuePairs.get(0), splitKeyValuePairs.get(1));
-            })
-        // If duplicate keys, prioritize later ones similar to duplicate system properties on a
-        // Java command line.
-        .collect(
-            Collectors.toMap(
-                Map.Entry::getKey, Map.Entry::getValue, (first, next) -> next, LinkedHashMap::new));
-  }
 
-  private static List<String> filterBlanksAndNulls(String[] values) {
-    return Arrays.stream(values)
-        .map(String::trim)
-        .filter(s -> !s.isEmpty())
-        .collect(Collectors.toList());
-  }
+    Resource sdkResource = autoConfiguredOpenTelemetrySdk.getResource();
 
-  @CheckForNull
-  public static String getSystemPropertyOrEnvironmentVariable(
-      String systemPropertyName, String environmentVariableName, @Nullable String defaultValue) {
-    String systemProperty = System.getProperty(systemPropertyName);
-    if (StringUtils.isNotBlank(systemProperty)) {
-      return systemProperty;
-    }
-    String environmentVariable = System.getenv(environmentVariableName);
-    if (StringUtils.isNotBlank(environmentVariable)) {
-      return environmentVariable;
-    }
-    return defaultValue;
+    return "Configuration: "
+        + configurationAttributes.entrySet().stream()
+            .map(entry -> entry.getKey() + "=" + entry.getValue())
+            .collect(Collectors.joining(", "))
+        + ", Resource: "
+        + sdkResource.getAttributes();
   }
 }
diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/semconv/MavenOtelSemanticAttributes.java b/maven-extension/src/main/java/io/opentelemetry/maven/semconv/MavenOtelSemanticAttributes.java
@@ -30,9 +30,5 @@ public class MavenOtelSemanticAttributes {
   public static final AttributeKey<String> MAVEN_EXECUTION_LIFECYCLE_PHASE =
       stringKey("maven.execution.lifecyclePhase");
 
-  public static final class ServiceNameValues {
-    public static final String SERVICE_NAME_VALUE = "maven";
-
-    private ServiceNameValues() {}
-  }
+  public static final String SERVICE_NAME_VALUE = "maven";
 }
diff --git a/maven-extension/src/test/java/io/opentelemetry/maven/OtelUtilsTest.java b/maven-extension/src/test/java/io/opentelemetry/maven/OtelUtilsTest.java
