pull the plug on breakdown metrics (#384)

* pull the plug on breakdown metrics
elastic · Aug 21, 2024 · 5a2b10c · 5a2b10c
1 parent edaf005
commit 5a2b10c
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 91 deletions.
diff --git a/README.md b/README.md
@@ -79,53 +79,6 @@ The minimum span duration can be configured with `elastic.otel.span.stack.trace.
 Experimental runtime metrics are enabled by default.
 Set `otel.instrumentation.runtime-telemetry.emit-experimental-telemetry` to `false` to disable them.
 
-### Breakdown metrics
-
-Breakdown metrics currently require a custom Elasticsearch ingest pipeline.
-
-```
-PUT _ingest/pipeline/metrics-apm.app@custom
-{
-  "processors": [
-    {
-      "script": {
-        "lang": "painless",
-        "source": """
-
-if(ctx.span == null){
-  ctx.span = [:];
-}
-if(ctx.transaction == null){
-  ctx.transaction = [:];
-}
-if(ctx.labels != null){
-  if(ctx.labels.elastic_span_type != null){
-    ctx.span.type = ctx.labels.elastic_span_type;
-  }
-  if(ctx.labels.elastic_span_subtype != null){
-    ctx.span.subtype = ctx.labels.elastic_span_subtype;
-  }
-  if(ctx.labels.elastic_local_root_type != null){
-    ctx.transaction.type = ctx.labels.elastic_local_root_type;
-  }
-  if(ctx.labels.elastic_local_root_name != null){
-    ctx.transaction.name = ctx.labels.elastic_local_root_name;
-  }
-}
-
-if(ctx.numeric_labels != null && ctx.numeric_labels.elastic_span_self_time != null){
-  def value = ctx.numeric_labels.elastic_span_self_time/1000;
-  def sum = [ 'us': value];
-  ctx.span.self_time =  [ 'count': 0, 'sum': sum];
-}
-
-        """
-      }
-    }
-  ]
-}
-```
-
 # License
 
 The Elastic Distribution of OpenTelemetry Java is licensed under [Apache License, Version 2.0](https://www.apache.org/licenses/LICENSE-2.0.html).

diff --git a/custom/breakdown-metrics.md b/custom/breakdown-metrics.md
@@ -0,0 +1,48 @@
+### Breakdown metrics
+
+Status: feature has been disabled and code is only kept for future reference.
+
+Breakdown metrics currently require a custom Elasticsearch ingest pipeline.
+
+```
+PUT _ingest/pipeline/metrics-apm.app@custom
+{
+  "processors": [
+    {
+      "script": {
+        "lang": "painless",
+        "source": """
+
+if(ctx.span == null){
+  ctx.span = [:];
+}
+if(ctx.transaction == null){
+  ctx.transaction = [:];
+}
+if(ctx.labels != null){
+  if(ctx.labels.elastic_span_type != null){
+    ctx.span.type = ctx.labels.elastic_span_type;
+  }
+  if(ctx.labels.elastic_span_subtype != null){
+    ctx.span.subtype = ctx.labels.elastic_span_subtype;
+  }
+  if(ctx.labels.elastic_local_root_type != null){
+    ctx.transaction.type = ctx.labels.elastic_local_root_type;
+  }
+  if(ctx.labels.elastic_local_root_name != null){
+    ctx.transaction.name = ctx.labels.elastic_local_root_name;
+  }
+}
+
+if(ctx.numeric_labels != null && ctx.numeric_labels.elastic_span_self_time != null){
+  def value = ctx.numeric_labels.elastic_span_self_time/1000;
+  def sum = [ 'us': value];
+  ctx.span.self_time =  [ 'count': 0, 'sum': sum];
+}
+
+        """
+      }
+    }
+  ]
+}
+```
diff --git a/custom/src/main/java/co/elastic/otel/ElasticAgentListener.java b/custom/src/main/java/co/elastic/otel/ElasticAgentListener.java
@@ -18,13 +18,13 @@
  */
 package co.elastic.otel;
 
-import com.google.auto.service.AutoService;
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.javaagent.extension.AgentListener;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 
-@AutoService(AgentListener.class)
+// @AutoService(AgentListener.class)
+@Deprecated
 public class ElasticAgentListener implements AgentListener {
   @Override
   public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) {
@@ -37,14 +37,5 @@ public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetr
     OpenTelemetry openTelemetry = GlobalOpenTelemetry.get();
 
     ElasticExtension.INSTANCE.registerOpenTelemetry(openTelemetry);
-
-    Runtime.getRuntime()
-        .addShutdownHook(
-            new Thread() {
-              @Override
-              public void run() {
-                ElasticExtension.INSTANCE.shutdown();
-              }
-            });
   }
 }
diff --git a/custom/src/main/java/co/elastic/otel/ElasticAutoConfigurationCustomizerProvider.java b/custom/src/main/java/co/elastic/otel/ElasticAutoConfigurationCustomizerProvider.java
@@ -37,18 +37,8 @@ public class ElasticAutoConfigurationCustomizerProvider
 
   @Override
   public void customize(AutoConfigurationCustomizer autoConfiguration) {
-
-    autoConfiguration
-        .addTracerProviderCustomizer(
-            (sdkTracerProviderBuilder, configProperties) ->
-                // span processor registration
-                sdkTracerProviderBuilder.addSpanProcessor(
-                    ElasticExtension.INSTANCE.getSpanProcessor()))
-        .addPropertiesCustomizer(ElasticAutoConfigurationCustomizerProvider::propertiesCustomizer)
-        .addSpanExporterCustomizer(
-            (spanExporter, configProperties) ->
-                // wrap the original span exporter
-                ElasticExtension.INSTANCE.wrapSpanExporter(spanExporter));
+    autoConfiguration.addPropertiesCustomizer(
+        ElasticAutoConfigurationCustomizerProvider::propertiesCustomizer);
   }
 
   static Map<String, String> propertiesCustomizer(ConfigProperties configProperties) {

diff --git a/custom/src/main/java/co/elastic/otel/ElasticBreakdownMetrics.java b/custom/src/main/java/co/elastic/otel/ElasticBreakdownMetrics.java
@@ -83,7 +83,7 @@ public String getLocalRootSpanName() {
   }
 
   public ElasticBreakdownMetrics() {
-    elasticSpanData = new ConcurrentHashMap<>();
+    this.elasticSpanData = (ConcurrentHashMap<SpanContext, BreakdownData>) new ConcurrentHashMap();
   }
 
   public void registerOpenTelemetry(OpenTelemetry openTelemetry) {
@@ -182,11 +182,14 @@ public void onSpanEnd(ReadableSpan span) {
           buildCounterAttributes(spanData.getAttributes())
               .put(ElasticAttributes.LOCAL_ROOT_TYPE, spanContextData.getLocalRootSpanType())
               .put(ElasticAttributes.LOCAL_ROOT_NAME, spanContextData.getLocalRootSpanName())
-              // put measured metric as span attribute to allow using an ingest pipeline to alter
-              // storage
-              // ingest pipelines do not have access to _source and thus can't read the metric
-              // as-is.
-              .put(ElasticAttributes.SELF_TIME, selfTime);
+          // put measured metric as span attribute to allow using an ingest pipeline to alter
+          // storage ingest pipelines do not have access to _source and thus can't read the
+          // metric as-is.
+          //
+          // (ab)using metric attributes for this breaks due to high cardinality
+          // see https://github.com/elastic/elastic-otel-java/issues/383 for details
+          // .put(ElasticAttributes.SELF_TIME, selfTime)
+          ;
 
       // unfortunately here we get a read-only span that has already been ended, thus even a cast to
       // ReadWriteSpan

diff --git a/custom/src/main/java/co/elastic/otel/ElasticExtension.java b/custom/src/main/java/co/elastic/otel/ElasticExtension.java
@@ -18,27 +18,25 @@
  */
 package co.elastic.otel;
 
-import co.elastic.otel.common.util.ExecutorUtils;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.sdk.trace.SpanProcessor;
 import io.opentelemetry.sdk.trace.export.SpanExporter;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 
+/**
+ * Deprecated as breakdown metrics is experimental and has <a
+ * href="https://github.com/elastic/elastic-otel-java/issues/383">at least one issue</a>
+ */
+@Deprecated
 public class ElasticExtension {
 
   public static final ElasticExtension INSTANCE = new ElasticExtension();
   private final ElasticBreakdownMetrics breakdownMetrics;
   private final ElasticSpanProcessor spanProcessor;
-  private final ExecutorService asyncInitExecutor;
   private ElasticSpanExporter spanExporter;
 
   private ElasticExtension() {
     this.breakdownMetrics = new ElasticBreakdownMetrics();
     this.spanProcessor = new ElasticSpanProcessor(breakdownMetrics);
-
-    this.asyncInitExecutor =
-        Executors.newSingleThreadExecutor(ExecutorUtils.threadFactory("resource-init", true));
   }
 
   public void registerOpenTelemetry(OpenTelemetry openTelemetry) {
@@ -55,8 +53,4 @@ public SpanExporter wrapSpanExporter(SpanExporter toWrap) {
     spanProcessor.registerSpanExporter(spanExporter);
     return spanExporter;
   }
-
-  public void shutdown() {
-    ExecutorUtils.shutdownAndWaitTermination(asyncInitExecutor);
-  }
 }
diff --git a/smoke-tests/src/test/java/com/example/javaagent/smoketest/AgentFeaturesSmokeTest.java b/smoke-tests/src/test/java/com/example/javaagent/smoketest/AgentFeaturesSmokeTest.java
@@ -55,11 +55,12 @@ public void healthcheck() throws InterruptedException {
     spans.forEach(
         span -> {
           assertThat(getAttributes(span.getAttributesList()))
-              .containsKeys(
-                  "elastic.span.is_local_root",
-                  "elastic.span.local_root.id",
+              // span breakdown feature disabled
+              .doesNotContainKeys(
                   "elastic.span.self_time",
-                  "code.stacktrace");
+                  "elastic.span.is_local_root",
+                  "elastic.span.local_root.id")
+              .containsKeys("code.stacktrace");
         });
   }
 }
diff --git a/testing/integration-tests/universal-profiling-test/build.gradle.kts b/testing/integration-tests/universal-profiling-test/build.gradle.kts
@@ -19,6 +19,8 @@ tasks.withType<Test>() {
     .toAbsolutePath().toString()
 
   jvmArgs(
+    // TODO work-around: exporter is required to make chaining batch processor work
+    "-Dotel.traces.exporter=logging",
     //"-Dotel.javaagent.debug=true",
     "-Dotel.service.name=testing",
     "-Delastic.otel.universal.profiling.integration.socket.dir=${tmpDir}"