Issue #2689: Unsatisfactory behavior of snapshot selector (#2717)

Author: robsunday

profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProbabilisticSnapshotSelector.java

@@ -25,7 +25,6 @@
 import io.opentelemetry.sdk.extension.incubator.fileconfig.DeclarativeConfigurationCustomizer;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.DeclarativeConfigurationCustomizerProvider;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel;
-import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.PropagatorModel;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.SpanProcessorModel;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.TracerProviderModel;
 import java.util.List;
@@ -52,33 +51,12 @@ OpenTelemetryConfigurationModel customizeModel(OpenTelemetryConfigurationModel m
     if (snapshotProfiling.isEnabled()) {
       initActiveSpansTracking();
       initStackTraceSampler(snapshotProfiling);
-      addSnapshotVolumePropagator(model);
       addSpanProcessors(model);
     }
 
     return model;
   }
 
-  private void addSnapshotVolumePropagator(OpenTelemetryConfigurationModel model) {
-    PropagatorModel propagatorModel = model.getPropagator();
-    if (propagatorModel == null) {
-      propagatorModel = new PropagatorModel();
-      model.withPropagator(propagatorModel);
-    }
-
-    String volumePropagatorName = SnapshotVolumePropagatorComponentProvider.NAME;
-    String propagators = propagatorModel.getCompositeList();
-    // Possible propagator duplicates with propagatorModel.getComposite() are resolved by the
-    // upstream
-    if (propagators == null || propagators.trim().isEmpty()) {
-      propagators = volumePropagatorName;
-    } else {
-      propagators = String.join(",", propagators, volumePropagatorName);
-    }
-
-    propagatorModel.withCompositeList(propagators);
-  }
-
   private void initStackTraceSampler(
       SnapshotProfilingDeclarativeConfiguration snapshotProfilingConfig) {
     StackTraceSamplerInitializer.setupStackTraceSampler(snapshotProfilingConfig);

profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingConfigurationCustomizerProvider.java

@@ -24,7 +24,6 @@
 import io.opentelemetry.sdk.trace.SdkTracerProviderBuilder;
 import java.time.Duration;
 import java.util.Collections;
-import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.BiFunction;
@@ -81,7 +80,6 @@ private SnapshotProfilingSdkCustomizer(
   @Override
   public void customize(AutoConfigurationCustomizer autoConfigurationCustomizer) {
     autoConfigurationCustomizer
-        .addPropertiesCustomizer(autoConfigureSnapshotVolumePropagator())
         .addTracerProviderCustomizer(snapshotProfilingSpanProcessor(registry))
         .addPropertiesCustomizer(setupStackTraceSampler())
         .addPropertiesCustomizer(startTrackingActiveSpans(registry))
@@ -102,39 +100,14 @@ public void customize(AutoConfigurationCustomizer autoConfigurationCustomizer) {
       snapshotProfilingSpanProcessor(TraceRegistry registry) {
     return (builder, properties) -> {
       if (snapshotProfilingEnabled(properties)) {
-        return builder.addSpanProcessor(new SnapshotProfilingSpanProcessor(registry));
-      }
-      return builder;
-    };
-  }
+        double selectionProbability =
+            new SnapshotProfilingEnvVarsConfiguration(properties).getSnapshotSelectionProbability();
 
-  /**
-   * Attempt to autoconfigure the OpenTelemetry propagators to include the Splunk snapshot volume
-   * propagator and ensure it runs after the W3C Baggage propagator and ensure that a trace context
-   * propagator is configured. In addition, take care to retain any propagators explicitly
-   * configured prior.
-   *
-   * <p>The Java agent uses the "otel.propagators" property and the value is assumed to be a comma
-   * seperated list of propagator names. See <a
-   * href="https://opentelemetry.io/docs/languages/java/configuration/#properties-general">OpenTelemetry's
-   * Java Agent Configuration</a> for more details.
-   */
-  private Function<ConfigProperties, Map<String, String>> autoConfigureSnapshotVolumePropagator() {
-    return properties -> {
-      if (snapshotProfilingEnabled(properties)) {
-        Set<String> propagators = new LinkedHashSet<>(properties.getList("otel.propagators"));
-        if (propagators.contains("none")) {
-          return Collections.emptyMap();
-        }
-
-        if (includeTraceContextPropagator(propagators)) {
-          propagators.add("tracecontext");
-        }
-        propagators.add("baggage");
-        propagators.add(SnapshotVolumePropagatorProvider.NAME);
-        return Collections.singletonMap("otel.propagators", String.join(",", propagators));
+        return builder.addSpanProcessor(
+            new SnapshotProfilingSpanProcessor(
+                registry, new TraceIdBasedSnapshotSelector(selectionProbability)));
       }
-      return Collections.emptyMap();
+      return builder;
     };
   }
 

profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java

@@ -18,6 +18,7 @@
 
 import static com.splunk.opentelemetry.profiler.ProfilingSemanticAttributes.SNAPSHOT_PROFILING;
 
+import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.trace.ReadWriteSpan;
@@ -29,19 +30,22 @@
  */
 public class SnapshotProfilingSpanProcessor implements SpanProcessor {
   private final TraceRegistry registry;
+  private final SnapshotSelector selector;
   private final OrphanedTraceCleaner orphanedTraceCleaner;
 
-  SnapshotProfilingSpanProcessor(TraceRegistry registry) {
+  SnapshotProfilingSpanProcessor(TraceRegistry registry, SnapshotSelector selector) {
     this.registry = registry;
+    this.selector = selector;
     this.orphanedTraceCleaner = new OrphanedTraceCleaner(registry);
   }
 
   @Override
   public void onStart(Context context, ReadWriteSpan span) {
     if (isEntry(span)) {
-      Volume volume = Volume.from(context);
-      if (volume == Volume.HIGHEST) {
-        registry.register(span.getSpanContext());
+      SpanContext spanContext = span.getSpanContext();
+      boolean selected = selector.select(spanContext);
+      if (selected) {
+        registry.register(spanContext);
         orphanedTraceCleaner.register(span);
       }
     }

profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java

@@ -47,6 +47,10 @@ public String getName() {
   @Override
   public SnapshotProfilingSpanProcessor create(
       DeclarativeConfigProperties declarativeConfigProperties) {
-    return new SnapshotProfilingSpanProcessor(traceRegistry);
+    double selectionProbability =
+        new SnapshotProfilingDeclarativeConfiguration(declarativeConfigProperties)
+            .getSnapshotSelectionProbability();
+    return new SnapshotProfilingSpanProcessor(
+        traceRegistry, new TraceIdBasedSnapshotSelector(selectionProbability));
   }
 }

profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessorComponentProvider.java

@@ -16,12 +16,8 @@
 
 package com.splunk.opentelemetry.profiler.snapshot;
 
-import io.opentelemetry.context.Context;
+import io.opentelemetry.api.trace.SpanContext;
 
 interface SnapshotSelector {
-  default SnapshotSelector or(SnapshotSelector other) {
-    return context -> select(context) || other.select(context);
-  }
-
-  boolean select(Context context);
+  boolean select(SpanContext context);
 }