From a0d487f8524b0bb85aff9d71c41512a9e5a1e6a2 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 15:10:38 -0700 Subject: [PATCH 01/35] Add TraceRegistry interface. --- .../profiler/snapshot/ITraceRegistry.java | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ITraceRegistry.java diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ITraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ITraceRegistry.java new file mode 100644 index 000000000..64927dd4b --- /dev/null +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ITraceRegistry.java @@ -0,0 +1,11 @@ +package com.splunk.opentelemetry.profiler.snapshot; + +import io.opentelemetry.api.trace.SpanContext; + +interface ITraceRegistry { + void register(SpanContext spanContext); + + boolean isRegistered(SpanContext spanContext); + + void unregister(SpanContext spanContext); +} From e4c025847b6ef5adaba79c8dcaec589e41d8136a Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 15:16:21 -0700 Subject: [PATCH 02/35] Have TraceRegistry and its extenders implement the ITraceRegistry interface. --- .../splunk/opentelemetry/profiler/snapshot/TraceRegistry.java | 2 +- .../opentelemetry/profiler/snapshot/RecordingTraceRegistry.java | 2 +- .../opentelemetry/profiler/snapshot/TogglableTraceRegistry.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java index 610f721f1..5980d3e5b 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java @@ -21,7 +21,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -class TraceRegistry { +class TraceRegistry implements ITraceRegistry { private final Set traceIds = Collections.newSetFromMap(new ConcurrentHashMap<>()); public void register(SpanContext spanContext) { diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java index 8a6ff9ae5..7ebb61098 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java @@ -25,7 +25,7 @@ * Test only version of {@link TraceRegistry} that keeps a record of every trace ID registered over * the lifetime of the instance. */ -class RecordingTraceRegistry extends TraceRegistry { +class RecordingTraceRegistry extends TraceRegistry implements ITraceRegistry { private final Set registeredTraceIds = new HashSet<>(); @Override diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java index 594579416..846aa0f2f 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java @@ -18,7 +18,7 @@ import io.opentelemetry.api.trace.SpanContext; -class TogglableTraceRegistry extends TraceRegistry { +class TogglableTraceRegistry extends TraceRegistry implements ITraceRegistry { enum State { ON, OFF From 0f35d76f36360a3ce219e05bc6a2f677f862b909 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 15:20:34 -0700 Subject: [PATCH 03/35] Replace references to TraceRegistry with the ITraceRegistry interface. --- .../profiler/snapshot/ActiveSpanTracker.java | 4 ++-- ...nterceptingContextStorageSpanTrackingActivator.java | 2 +- .../snapshot/SnapshotProfilingSdkCustomizer.java | 10 +++++----- .../snapshot/SnapshotProfilingSpanProcessor.java | 4 ++-- .../profiler/snapshot/SpanTrackingActivator.java | 2 +- .../profiler/snapshot/RecordingTraceRegistry.java | 2 +- .../snapshot/SnapshotProfilingFeatureFlagTest.java | 2 +- .../snapshot/SnapshotProfilingLogExportingTest.java | 2 +- .../SnapshotProfilingSdkCustomizerBuilder.java | 4 ++-- .../profiler/snapshot/SpanSamplingTest.java | 2 +- .../profiler/snapshot/TraceRegistrationTest.java | 2 +- 11 files changed, 18 insertions(+), 18 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java index 01ae49601..4b1af904e 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java @@ -29,9 +29,9 @@ class ActiveSpanTracker implements ContextStorage, SpanTracker { private final Cache cache = Cache.weak(); private final ContextStorage delegate; - private final TraceRegistry registry; + private final ITraceRegistry registry; - ActiveSpanTracker(ContextStorage delegate, TraceRegistry registry) { + ActiveSpanTracker(ContextStorage delegate, ITraceRegistry registry) { this.delegate = delegate; this.registry = registry; } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java index 282454785..c619a56ef 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java @@ -35,7 +35,7 @@ class InterceptingContextStorageSpanTrackingActivator implements SpanTrackingAct } @Override - public void activate(TraceRegistry registry) { + public void activate(ITraceRegistry registry) { contextStorageWrappingFunction.accept( contextStorage -> { ActiveSpanTracker tracker = new ActiveSpanTracker(contextStorage, registry); diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java index bc7f51e2c..c4cf1dc85 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java @@ -33,7 +33,7 @@ @AutoService(AutoConfigurationCustomizerProvider.class) public class SnapshotProfilingSdkCustomizer implements AutoConfigurationCustomizerProvider { - private final TraceRegistry registry; + private final ITraceRegistry registry; private final Function samplerProvider; private final SpanTrackingActivator spanTrackingActivator; @@ -61,12 +61,12 @@ private static StagingArea createStagingArea(ConfigProperties properties) { @VisibleForTesting SnapshotProfilingSdkCustomizer( - TraceRegistry registry, StackTraceSampler sampler, SpanTrackingActivator activator) { + ITraceRegistry registry, StackTraceSampler sampler, SpanTrackingActivator activator) { this(registry, properties -> sampler, activator); } private SnapshotProfilingSdkCustomizer( - TraceRegistry registry, + ITraceRegistry registry, Function samplerProvider, SpanTrackingActivator spanTrackingActivator) { this.registry = registry; @@ -94,7 +94,7 @@ public void customize(AutoConfigurationCustomizer autoConfigurationCustomizer) { } private BiFunction - snapshotProfilingSpanProcessor(TraceRegistry registry) { + snapshotProfilingSpanProcessor(ITraceRegistry registry) { return (builder, properties) -> { if (snapshotProfilingEnabled(properties)) { StackTraceSampler sampler = samplerProvider.apply(properties); @@ -141,7 +141,7 @@ private boolean includeTraceContextPropagator(Set configuredPropagators) } private Function> startTrackingActiveSpans( - TraceRegistry registry) { + ITraceRegistry registry) { return properties -> { if (snapshotProfilingEnabled(properties)) { spanTrackingActivator.activate(registry); diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java index 3905df64b..d0f74e3a7 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java @@ -36,10 +36,10 @@ * be profiled at a time. */ public class SnapshotProfilingSpanProcessor implements SpanProcessor { - private final TraceRegistry registry; + private final ITraceRegistry registry; private final Supplier sampler; - SnapshotProfilingSpanProcessor(TraceRegistry registry, Supplier sampler) { + SnapshotProfilingSpanProcessor(ITraceRegistry registry, Supplier sampler) { this.registry = registry; this.sampler = sampler; } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java index 87d098d76..d4f910f12 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java @@ -17,5 +17,5 @@ package com.splunk.opentelemetry.profiler.snapshot; interface SpanTrackingActivator { - void activate(TraceRegistry registry); + void activate(ITraceRegistry registry); } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java index 7ebb61098..89d3af6c5 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java @@ -22,7 +22,7 @@ import java.util.Set; /** - * Test only version of {@link TraceRegistry} that keeps a record of every trace ID registered over + * Test only version of {@link ITraceRegistry} that keeps a record of every trace ID registered over * the lifetime of the instance. */ class RecordingTraceRegistry extends TraceRegistry implements ITraceRegistry { diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java index 2f961e950..dae1c6cdc 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java @@ -26,7 +26,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; class SnapshotProfilingFeatureFlagTest { - private final TraceRegistry registry = new TraceRegistry(); + private final ITraceRegistry registry = new TraceRegistry(); private final SnapshotProfilingSdkCustomizer customizer = Snapshotting.customizer().with(registry).build(); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java index 8f5480a8b..ff7d2ea6c 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java @@ -106,7 +106,7 @@ private Predicate> label(AttributeKey key) { private static class ResetContextStorage implements SpanTrackingActivator, AfterEachCallback { @Override - public void activate(TraceRegistry registry) { + public void activate(ITraceRegistry registry) { ActiveSpanTracker spanTracker = new ActiveSpanTracker(ContextStorage.defaultStorage(), registry); SpanTracker.SUPPLIER.configure(spanTracker); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java index a9479a96a..87a81e193 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java @@ -19,11 +19,11 @@ import java.time.Duration; class SnapshotProfilingSdkCustomizerBuilder { - private TraceRegistry registry = new TraceRegistry(); + private ITraceRegistry registry = new TraceRegistry(); private StackTraceSampler sampler = new ObservableStackTraceSampler(); private SpanTrackingActivator spanTrackingActivator = registry -> {}; - SnapshotProfilingSdkCustomizerBuilder with(TraceRegistry registry) { + SnapshotProfilingSdkCustomizerBuilder with(ITraceRegistry registry) { this.registry = registry; return this; } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java index 2b85fb4e7..e56984239 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java @@ -27,7 +27,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; class SpanSamplingTest { - private final TraceRegistry registry = new TraceRegistry(); + private final ITraceRegistry registry = new TraceRegistry(); private final SnapshotProfilingSdkCustomizer customizer = Snapshotting.customizer().with(registry).build(); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java index 56a15b9a8..1f59816eb 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java @@ -26,7 +26,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; class TraceRegistrationTest { - private final TraceRegistry registry = new TraceRegistry(); + private final ITraceRegistry registry = new TraceRegistry(); private final SnapshotProfilingSdkCustomizer customizer = Snapshotting.customizer().with(registry).build(); From 2b0700a2e0f6329ec3ab40f58ea169d21db30d16 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 15:22:59 -0700 Subject: [PATCH 04/35] Remove TraceRegistry inheritence from TogglableTraceRegistry. --- .../profiler/snapshot/TogglableTraceRegistry.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java index 846aa0f2f..b135810e9 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java @@ -17,8 +17,12 @@ package com.splunk.opentelemetry.profiler.snapshot; import io.opentelemetry.api.trace.SpanContext; +import java.util.HashSet; +import java.util.Set; + +class TogglableTraceRegistry implements ITraceRegistry { + private final Set traceIds = new HashSet<>(); -class TogglableTraceRegistry extends TraceRegistry implements ITraceRegistry { enum State { ON, OFF @@ -29,7 +33,7 @@ enum State { @Override public void register(SpanContext spanContext) { if (state == State.ON) { - super.register(spanContext); + traceIds.add(spanContext.getTraceId()); } } @@ -39,11 +43,11 @@ public void toggle(State state) { @Override public boolean isRegistered(SpanContext spanContext) { - return super.isRegistered(spanContext); + return traceIds.contains(spanContext.getTraceId()); } @Override public void unregister(SpanContext spanContext) { - super.unregister(spanContext); + traceIds.remove(spanContext.getTraceId()); } } From 2600c7533914c78e0473dac05a30439e5b603031 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 15:28:20 -0700 Subject: [PATCH 05/35] Remove TraceRegistry inheritence from RecordingTraceRegistry. --- .../snapshot/RecordingTraceRegistry.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java index 89d3af6c5..3e49fce4a 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java @@ -18,33 +18,33 @@ import io.opentelemetry.api.trace.SpanContext; import java.util.Collections; -import java.util.HashSet; +import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; /** * Test only version of {@link ITraceRegistry} that keeps a record of every trace ID registered over * the lifetime of the instance. */ -class RecordingTraceRegistry extends TraceRegistry implements ITraceRegistry { - private final Set registeredTraceIds = new HashSet<>(); +class RecordingTraceRegistry implements ITraceRegistry { + private final Map traceIds = new ConcurrentHashMap<>(); @Override public void register(SpanContext spanContext) { - registeredTraceIds.add(spanContext.getTraceId()); - super.register(spanContext); + traceIds.put(spanContext.getTraceId(), true); } @Override public boolean isRegistered(SpanContext spanContext) { - return super.isRegistered(spanContext); + return traceIds.getOrDefault(spanContext.getTraceId(), false); } @Override public void unregister(SpanContext spanContext) { - super.unregister(spanContext); + traceIds.put(spanContext.getTraceId(), false); } Set registeredTraceIds() { - return Collections.unmodifiableSet(registeredTraceIds); + return Collections.unmodifiableSet(traceIds.keySet()); } } From 32c296e105c789f913676adbc35ecd9f3f4a74a9 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 15:44:54 -0700 Subject: [PATCH 06/35] Rename TraceRegistry to SimpleTraceRegistry. --- .../snapshot/{TraceRegistry.java => SimpleTraceRegistry.java} | 2 +- .../profiler/snapshot/SnapshotProfilingSdkCustomizer.java | 2 +- .../InterceptingContextStorageSpanTrackingActivatorTest.java | 4 ++-- .../{TraceRegistryTest.java => SimpleTraceRegistryTest.java} | 4 ++-- .../profiler/snapshot/SnapshotProfilingFeatureFlagTest.java | 2 +- .../snapshot/SnapshotProfilingSdkCustomizerBuilder.java | 2 +- .../opentelemetry/profiler/snapshot/SpanSamplingTest.java | 2 +- .../profiler/snapshot/TraceRegistrationTest.java | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) rename profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/{TraceRegistry.java => SimpleTraceRegistry.java} (95%) rename profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/{TraceRegistryTest.java => SimpleTraceRegistryTest.java} (92%) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java similarity index 95% rename from profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java rename to profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java index 5980d3e5b..d68bd14f7 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java @@ -21,7 +21,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -class TraceRegistry implements ITraceRegistry { +class SimpleTraceRegistry implements ITraceRegistry { private final Set traceIds = Collections.newSetFromMap(new ConcurrentHashMap<>()); public void register(SpanContext spanContext) { diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java index c4cf1dc85..dd893d2cc 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java @@ -39,7 +39,7 @@ public class SnapshotProfilingSdkCustomizer implements AutoConfigurationCustomiz public SnapshotProfilingSdkCustomizer() { this( - new TraceRegistry(), + new SimpleTraceRegistry(), stackTraceSamplerProvider(), new InterceptingContextStorageSpanTrackingActivator()); } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivatorTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivatorTest.java index 4c14498a0..d09e30cdc 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivatorTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivatorTest.java @@ -30,13 +30,13 @@ class InterceptingContextStorageSpanTrackingActivatorTest { @Test void interceptContextStorage() { - activator.activate(new TraceRegistry()); + activator.activate(new SimpleTraceRegistry()); assertInstanceOf(ActiveSpanTracker.class, delegate.storage); } @Test void activateSpanTracker() { - activator.activate(new TraceRegistry()); + activator.activate(new SimpleTraceRegistry()); assertInstanceOf(ActiveSpanTracker.class, SpanTracker.SUPPLIER.get()); } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistryTest.java similarity index 92% rename from profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistryTest.java rename to profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistryTest.java index bcd6d5350..5b65b9708 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistryTest.java @@ -21,8 +21,8 @@ import org.junit.jupiter.api.Test; -class TraceRegistryTest { - private final TraceRegistry registry = new TraceRegistry(); +class SimpleTraceRegistryTest { + private final SimpleTraceRegistry registry = new SimpleTraceRegistry(); @Test void registerTrace() { diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java index dae1c6cdc..0aaa8a440 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java @@ -26,7 +26,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; class SnapshotProfilingFeatureFlagTest { - private final ITraceRegistry registry = new TraceRegistry(); + private final ITraceRegistry registry = new SimpleTraceRegistry(); private final SnapshotProfilingSdkCustomizer customizer = Snapshotting.customizer().with(registry).build(); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java index 87a81e193..30e981653 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java @@ -19,7 +19,7 @@ import java.time.Duration; class SnapshotProfilingSdkCustomizerBuilder { - private ITraceRegistry registry = new TraceRegistry(); + private ITraceRegistry registry = new SimpleTraceRegistry(); private StackTraceSampler sampler = new ObservableStackTraceSampler(); private SpanTrackingActivator spanTrackingActivator = registry -> {}; diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java index e56984239..f4b158b47 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java @@ -27,7 +27,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; class SpanSamplingTest { - private final ITraceRegistry registry = new TraceRegistry(); + private final ITraceRegistry registry = new SimpleTraceRegistry(); private final SnapshotProfilingSdkCustomizer customizer = Snapshotting.customizer().with(registry).build(); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java index 1f59816eb..7b31201aa 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java @@ -26,7 +26,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; class TraceRegistrationTest { - private final ITraceRegistry registry = new TraceRegistry(); + private final ITraceRegistry registry = new SimpleTraceRegistry(); private final SnapshotProfilingSdkCustomizer customizer = Snapshotting.customizer().with(registry).build(); From 466173f0fc32f16b6319d7728b128ed2fce7419d Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 15:45:33 -0700 Subject: [PATCH 07/35] Rename ITraceRegistry to TraceRegistry. --- .../profiler/snapshot/ActiveSpanTracker.java | 4 ++-- ...nterceptingContextStorageSpanTrackingActivator.java | 2 +- .../profiler/snapshot/SimpleTraceRegistry.java | 2 +- .../snapshot/SnapshotProfilingSdkCustomizer.java | 10 +++++----- .../snapshot/SnapshotProfilingSpanProcessor.java | 4 ++-- .../profiler/snapshot/SpanTrackingActivator.java | 2 +- .../{ITraceRegistry.java => TraceRegistry.java} | 2 +- .../profiler/snapshot/RecordingTraceRegistry.java | 4 ++-- .../snapshot/SnapshotProfilingFeatureFlagTest.java | 2 +- .../snapshot/SnapshotProfilingLogExportingTest.java | 2 +- .../SnapshotProfilingSdkCustomizerBuilder.java | 4 ++-- .../profiler/snapshot/SpanSamplingTest.java | 2 +- .../profiler/snapshot/TogglableTraceRegistry.java | 2 +- .../profiler/snapshot/TraceRegistrationTest.java | 2 +- 14 files changed, 22 insertions(+), 22 deletions(-) rename profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/{ITraceRegistry.java => TraceRegistry.java} (89%) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java index 4b1af904e..01ae49601 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java @@ -29,9 +29,9 @@ class ActiveSpanTracker implements ContextStorage, SpanTracker { private final Cache cache = Cache.weak(); private final ContextStorage delegate; - private final ITraceRegistry registry; + private final TraceRegistry registry; - ActiveSpanTracker(ContextStorage delegate, ITraceRegistry registry) { + ActiveSpanTracker(ContextStorage delegate, TraceRegistry registry) { this.delegate = delegate; this.registry = registry; } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java index c619a56ef..282454785 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java @@ -35,7 +35,7 @@ class InterceptingContextStorageSpanTrackingActivator implements SpanTrackingAct } @Override - public void activate(ITraceRegistry registry) { + public void activate(TraceRegistry registry) { contextStorageWrappingFunction.accept( contextStorage -> { ActiveSpanTracker tracker = new ActiveSpanTracker(contextStorage, registry); diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java index d68bd14f7..dff0f04af 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java @@ -21,7 +21,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -class SimpleTraceRegistry implements ITraceRegistry { +class SimpleTraceRegistry implements TraceRegistry { private final Set traceIds = Collections.newSetFromMap(new ConcurrentHashMap<>()); public void register(SpanContext spanContext) { diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java index dd893d2cc..0e8669ca0 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java @@ -33,7 +33,7 @@ @AutoService(AutoConfigurationCustomizerProvider.class) public class SnapshotProfilingSdkCustomizer implements AutoConfigurationCustomizerProvider { - private final ITraceRegistry registry; + private final TraceRegistry registry; private final Function samplerProvider; private final SpanTrackingActivator spanTrackingActivator; @@ -61,12 +61,12 @@ private static StagingArea createStagingArea(ConfigProperties properties) { @VisibleForTesting SnapshotProfilingSdkCustomizer( - ITraceRegistry registry, StackTraceSampler sampler, SpanTrackingActivator activator) { + TraceRegistry registry, StackTraceSampler sampler, SpanTrackingActivator activator) { this(registry, properties -> sampler, activator); } private SnapshotProfilingSdkCustomizer( - ITraceRegistry registry, + TraceRegistry registry, Function samplerProvider, SpanTrackingActivator spanTrackingActivator) { this.registry = registry; @@ -94,7 +94,7 @@ public void customize(AutoConfigurationCustomizer autoConfigurationCustomizer) { } private BiFunction - snapshotProfilingSpanProcessor(ITraceRegistry registry) { + snapshotProfilingSpanProcessor(TraceRegistry registry) { return (builder, properties) -> { if (snapshotProfilingEnabled(properties)) { StackTraceSampler sampler = samplerProvider.apply(properties); @@ -141,7 +141,7 @@ private boolean includeTraceContextPropagator(Set configuredPropagators) } private Function> startTrackingActiveSpans( - ITraceRegistry registry) { + TraceRegistry registry) { return properties -> { if (snapshotProfilingEnabled(properties)) { spanTrackingActivator.activate(registry); diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java index d0f74e3a7..3905df64b 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java @@ -36,10 +36,10 @@ * be profiled at a time. */ public class SnapshotProfilingSpanProcessor implements SpanProcessor { - private final ITraceRegistry registry; + private final TraceRegistry registry; private final Supplier sampler; - SnapshotProfilingSpanProcessor(ITraceRegistry registry, Supplier sampler) { + SnapshotProfilingSpanProcessor(TraceRegistry registry, Supplier sampler) { this.registry = registry; this.sampler = sampler; } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java index d4f910f12..87d098d76 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java @@ -17,5 +17,5 @@ package com.splunk.opentelemetry.profiler.snapshot; interface SpanTrackingActivator { - void activate(ITraceRegistry registry); + void activate(TraceRegistry registry); } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ITraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java similarity index 89% rename from profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ITraceRegistry.java rename to profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java index 64927dd4b..187b5d146 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ITraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java @@ -2,7 +2,7 @@ import io.opentelemetry.api.trace.SpanContext; -interface ITraceRegistry { +interface TraceRegistry { void register(SpanContext spanContext); boolean isRegistered(SpanContext spanContext); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java index 3e49fce4a..5d00de036 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/RecordingTraceRegistry.java @@ -23,10 +23,10 @@ import java.util.concurrent.ConcurrentHashMap; /** - * Test only version of {@link ITraceRegistry} that keeps a record of every trace ID registered over + * Test only version of {@link TraceRegistry} that keeps a record of every trace ID registered over * the lifetime of the instance. */ -class RecordingTraceRegistry implements ITraceRegistry { +class RecordingTraceRegistry implements TraceRegistry { private final Map traceIds = new ConcurrentHashMap<>(); @Override diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java index 0aaa8a440..0b6f202a8 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingFeatureFlagTest.java @@ -26,7 +26,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; class SnapshotProfilingFeatureFlagTest { - private final ITraceRegistry registry = new SimpleTraceRegistry(); + private final TraceRegistry registry = new SimpleTraceRegistry(); private final SnapshotProfilingSdkCustomizer customizer = Snapshotting.customizer().with(registry).build(); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java index ff7d2ea6c..8f5480a8b 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java @@ -106,7 +106,7 @@ private Predicate> label(AttributeKey key) { private static class ResetContextStorage implements SpanTrackingActivator, AfterEachCallback { @Override - public void activate(ITraceRegistry registry) { + public void activate(TraceRegistry registry) { ActiveSpanTracker spanTracker = new ActiveSpanTracker(ContextStorage.defaultStorage(), registry); SpanTracker.SUPPLIER.configure(spanTracker); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java index 30e981653..ab01e7295 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java @@ -19,11 +19,11 @@ import java.time.Duration; class SnapshotProfilingSdkCustomizerBuilder { - private ITraceRegistry registry = new SimpleTraceRegistry(); + private TraceRegistry registry = new SimpleTraceRegistry(); private StackTraceSampler sampler = new ObservableStackTraceSampler(); private SpanTrackingActivator spanTrackingActivator = registry -> {}; - SnapshotProfilingSdkCustomizerBuilder with(ITraceRegistry registry) { + SnapshotProfilingSdkCustomizerBuilder with(TraceRegistry registry) { this.registry = registry; return this; } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java index f4b158b47..23bb43a0c 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SpanSamplingTest.java @@ -27,7 +27,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; class SpanSamplingTest { - private final ITraceRegistry registry = new SimpleTraceRegistry(); + private final TraceRegistry registry = new SimpleTraceRegistry(); private final SnapshotProfilingSdkCustomizer customizer = Snapshotting.customizer().with(registry).build(); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java index b135810e9..9244b4f23 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TogglableTraceRegistry.java @@ -20,7 +20,7 @@ import java.util.HashSet; import java.util.Set; -class TogglableTraceRegistry implements ITraceRegistry { +class TogglableTraceRegistry implements TraceRegistry { private final Set traceIds = new HashSet<>(); enum State { diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java index 7b31201aa..9eb60b8d9 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistrationTest.java @@ -26,7 +26,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; class TraceRegistrationTest { - private final ITraceRegistry registry = new SimpleTraceRegistry(); + private final TraceRegistry registry = new SimpleTraceRegistry(); private final SnapshotProfilingSdkCustomizer customizer = Snapshotting.customizer().with(registry).build(); From 7602fe7a6aa425dc228f300b60b0c02d3c9e1bed Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 15:47:19 -0700 Subject: [PATCH 08/35] Add @Override annotations in SimpleTraceRegistry. --- .../opentelemetry/profiler/snapshot/SimpleTraceRegistry.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java index dff0f04af..bc852470c 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java @@ -24,14 +24,17 @@ class SimpleTraceRegistry implements TraceRegistry { private final Set traceIds = Collections.newSetFromMap(new ConcurrentHashMap<>()); + @Override public void register(SpanContext spanContext) { traceIds.add(spanContext.getTraceId()); } + @Override public boolean isRegistered(SpanContext spanContext) { return traceIds.contains(spanContext.getTraceId()); } + @Override public void unregister(SpanContext spanContext) { traceIds.remove(spanContext.getTraceId()); } From 60dc92e7edd2165a22b84754454cfa5132ac5836 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 16:05:58 -0700 Subject: [PATCH 09/35] Add basic TraceRegistry implementation for unregistering stalled traces. --- .../StalledTraceDetectingTraceRegistry.java | 94 +++++++++++++++++++ .../profiler/snapshot/TraceRegistry.java | 16 ++++ ...talledTraceDetectingTraceRegistryTest.java | 72 ++++++++++++++ 3 files changed, 182 insertions(+) create mode 100644 profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java create mode 100644 profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java new file mode 100644 index 000000000..cfa019ec0 --- /dev/null +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java @@ -0,0 +1,94 @@ +/* + * Copyright Splunk Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.splunk.opentelemetry.profiler.snapshot; + +import com.splunk.opentelemetry.profiler.util.HelpfulExecutors; +import io.opentelemetry.api.trace.SpanContext; +import java.time.Duration; +import java.time.Instant; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +class StalledTraceDetectingTraceRegistry implements TraceRegistry, AutoCloseable { + private final ScheduledExecutorService scheduler = + HelpfulExecutors.newSingleThreadedScheduledExecutor("stalled-trace-detector"); + private final Map traceIds = new ConcurrentHashMap<>(); + private final TraceRegistry delegate; + + StalledTraceDetectingTraceRegistry(TraceRegistry delegate) { + this(delegate, Duration.ofMillis(10)); + } + + public StalledTraceDetectingTraceRegistry(TraceRegistry delegate, Duration stalledTimeLimit) { + this.delegate = delegate; + + scheduler.scheduleAtFixedRate( + removeStalledTraces(stalledTimeLimit), + stalledTimeLimit.toMillis() / 2, + stalledTimeLimit.toMillis() / 2, + TimeUnit.MILLISECONDS); + } + + @Override + public void register(SpanContext spanContext) { + delegate.register(spanContext); + traceIds.put(spanContext.getTraceId(), new RegistrationContext(Instant.now(), spanContext)); + } + + @Override + public boolean isRegistered(SpanContext spanContext) { + return delegate.isRegistered(spanContext); + } + + @Override + public void unregister(SpanContext spanContext) { + delegate.unregister(spanContext); + } + + @Override + public void close() throws Exception { + scheduler.shutdown(); + } + + private Runnable removeStalledTraces(Duration stalledTimeLimit) { + return () -> + traceIds + .entrySet() + .iterator() + .forEachRemaining( + entry -> { + Instant now = Instant.now(); + RegistrationContext context = entry.getValue(); + Duration duration = Duration.between(now, context.registrationTime); + if (duration.compareTo(stalledTimeLimit) <= 0) { + unregister(context.spanContext); + } + }); + } + + private static class RegistrationContext { + private final Instant registrationTime; + private final SpanContext spanContext; + + private RegistrationContext(Instant registrationTime, SpanContext spanContext) { + this.registrationTime = registrationTime; + this.spanContext = spanContext; + } + } +} diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java index 187b5d146..e97d10a23 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.splunk.opentelemetry.profiler.snapshot; import io.opentelemetry.api.trace.SpanContext; diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java new file mode 100644 index 000000000..facfe9082 --- /dev/null +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java @@ -0,0 +1,72 @@ +/* + * Copyright Splunk Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.splunk.opentelemetry.profiler.snapshot; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +import java.time.Duration; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +class StalledTraceDetectingTraceRegistryTest { + private final TraceRegistry delegate = new SimpleTraceRegistry(); + private final StalledTraceDetectingTraceRegistry registry = + new StalledTraceDetectingTraceRegistry(delegate); + + @AfterEach + void teardown() throws Exception { + registry.close(); + } + + @Test + void delegateTraceRegistration() { + var spanContext = Snapshotting.spanContext().build(); + registry.register(spanContext); + assertThat(delegate.isRegistered(spanContext)).isTrue(); + } + + @Test + void delegateTraceDeregistration() { + var spanContext = Snapshotting.spanContext().build(); + + registry.register(spanContext); + registry.unregister(spanContext); + + assertThat(delegate.isRegistered(spanContext)).isFalse(); + } + + @Test + void delegateTraceRegistrationLookup() { + var spanContext = Snapshotting.spanContext().build(); + delegate.register(spanContext); + assertThat(registry.isRegistered(spanContext)).isTrue(); + } + + @Test + void removeRegisteredTracesAfterStalledTimeLimitPasses() throws Exception { + var spanContext = Snapshotting.spanContext().build(); + var stalledTimeLimit = Duration.ofMillis(10); + + try (var registry = new StalledTraceDetectingTraceRegistry(delegate, stalledTimeLimit)) { + registry.register(spanContext); + assertThat(registry.isRegistered(spanContext)).isTrue(); + + await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); + } + } +} From a52393ceb3657a9cd1c47d52a8be08db0b626856 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 6 May 2025 16:08:32 -0700 Subject: [PATCH 10/35] Add test verifying that stalled traces are continually detected. --- .../StalledTraceDetectingTraceRegistryTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java index facfe9082..e80cdaa0e 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java @@ -69,4 +69,20 @@ void removeRegisteredTracesAfterStalledTimeLimitPasses() throws Exception { await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); } } + + @Test + void continueRemovingRegisteredTracesAfterStalledTimeLimitPasses() throws Exception { + var spanContext = Snapshotting.spanContext().build(); + var stalledTimeLimit = Duration.ofMillis(10); + + try (var registry = new StalledTraceDetectingTraceRegistry(delegate, stalledTimeLimit)) { + registry.register(spanContext); + assertThat(registry.isRegistered(spanContext)).isTrue(); + await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); + + registry.register(spanContext); + assertThat(registry.isRegistered(spanContext)).isTrue(); + await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); + } + } } From d06cf23e96e1b64a1ad59a9d92f3cc3ff3edea25 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Wed, 7 May 2025 09:45:58 -0700 Subject: [PATCH 11/35] Stop stack trace sampling when stalled trace is automatically unregistered. --- .../StalledTraceDetectingTraceRegistry.java | 13 +++--- ...talledTraceDetectingTraceRegistryTest.java | 43 +++++++++++-------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java index cfa019ec0..59aeb8ad4 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java @@ -24,19 +24,21 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; class StalledTraceDetectingTraceRegistry implements TraceRegistry, AutoCloseable { private final ScheduledExecutorService scheduler = HelpfulExecutors.newSingleThreadedScheduledExecutor("stalled-trace-detector"); private final Map traceIds = new ConcurrentHashMap<>(); private final TraceRegistry delegate; + private final Supplier sampler; - StalledTraceDetectingTraceRegistry(TraceRegistry delegate) { - this(delegate, Duration.ofMillis(10)); - } - - public StalledTraceDetectingTraceRegistry(TraceRegistry delegate, Duration stalledTimeLimit) { + public StalledTraceDetectingTraceRegistry( + TraceRegistry delegate, + Supplier sampler, + Duration stalledTimeLimit) { this.delegate = delegate; + this.sampler = sampler; scheduler.scheduleAtFixedRate( removeStalledTraces(stalledTimeLimit), @@ -78,6 +80,7 @@ private Runnable removeStalledTraces(Duration stalledTimeLimit) { Duration duration = Duration.between(now, context.registrationTime); if (duration.compareTo(stalledTimeLimit) <= 0) { unregister(context.spanContext); + sampler.get().stop(context.spanContext); } }); } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java index e80cdaa0e..ac126207a 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java @@ -24,9 +24,12 @@ import org.junit.jupiter.api.Test; class StalledTraceDetectingTraceRegistryTest { + private static final Duration STALLED_TIME_LIMIT = Duration.ofMillis(10); + private final TraceRegistry delegate = new SimpleTraceRegistry(); + private final ObservableStackTraceSampler sampler = new ObservableStackTraceSampler(); private final StalledTraceDetectingTraceRegistry registry = - new StalledTraceDetectingTraceRegistry(delegate); + new StalledTraceDetectingTraceRegistry(delegate, () -> sampler, STALLED_TIME_LIMIT); @AfterEach void teardown() throws Exception { @@ -58,31 +61,35 @@ void delegateTraceRegistrationLookup() { } @Test - void removeRegisteredTracesAfterStalledTimeLimitPasses() throws Exception { + void removeRegisteredTracesAfterStalledTimeLimitPasses() { + var spanContext = Snapshotting.spanContext().build(); + + registry.register(spanContext); + assertThat(registry.isRegistered(spanContext)).isTrue(); + + await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); + } + + @Test + void continueRemovingRegisteredTracesAfterStalledTimeLimitPasses() { var spanContext = Snapshotting.spanContext().build(); - var stalledTimeLimit = Duration.ofMillis(10); - try (var registry = new StalledTraceDetectingTraceRegistry(delegate, stalledTimeLimit)) { - registry.register(spanContext); - assertThat(registry.isRegistered(spanContext)).isTrue(); + registry.register(spanContext); + assertThat(registry.isRegistered(spanContext)).isTrue(); + await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); - await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); - } + registry.register(spanContext); + assertThat(registry.isRegistered(spanContext)).isTrue(); + await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); } @Test - void continueRemovingRegisteredTracesAfterStalledTimeLimitPasses() throws Exception { + void stopTraceSamplingWhenStalledTraceUnregistered() { var spanContext = Snapshotting.spanContext().build(); - var stalledTimeLimit = Duration.ofMillis(10); - try (var registry = new StalledTraceDetectingTraceRegistry(delegate, stalledTimeLimit)) { - registry.register(spanContext); - assertThat(registry.isRegistered(spanContext)).isTrue(); - await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); + registry.register(spanContext); + sampler.start(spanContext); - registry.register(spanContext); - assertThat(registry.isRegistered(spanContext)).isTrue(); - await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); - } + await().untilAsserted(() -> assertThat(sampler.isBeingSampled(spanContext)).isFalse()); } } From 88e7681b6cc280aeb74c17de8650b9c601aa8b07 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Wed, 7 May 2025 13:40:07 -0700 Subject: [PATCH 12/35] Add configuration property for snapshot profiling stalled trace timeout. --- .../opentelemetry/profiler/Configuration.java | 11 +++++++++++ .../StalledTraceDetectingTraceRegistry.java | 4 +--- .../profiler/ConfigurationTest.java | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/Configuration.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/Configuration.java index 89a302ec0..640f5d99f 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/Configuration.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/Configuration.java @@ -89,6 +89,11 @@ public class Configuration implements AutoConfigurationCustomizerProvider { "splunk.snapshot.profiler.staging.capacity"; private static final int DEFAULT_SNAPSHOT_PROFILER_STAGING_CAPACITY = 2000; + private static final String CONFIG_KEY_SNAPSHOT_PROFILER_STALLED_TRACE_TIMEOUT = + "splunk.snapshot.profiler.stalled.trace.timeout"; + private static final Duration DEFAULT_SNAPSHOT_PROFILER_STALLED_TRACE_TIMEOUT = + Duration.ofMinutes(10); + @Override public void customize(AutoConfigurationCustomizer autoConfiguration) { autoConfiguration.addPropertiesSupplier(this::defaultProperties); @@ -246,4 +251,10 @@ public static int getSnapshotProfilerStagingCapacity(ConfigProperties properties return properties.getInt( CONFIG_KEY_SNAPSHOT_PROFILER_STAGING_CAPACITY, DEFAULT_SNAPSHOT_PROFILER_STAGING_CAPACITY); } + + public static Duration getSnapshotProfilerStalledTraceTimeout(ConfigProperties properties) { + return properties.getDuration( + CONFIG_KEY_SNAPSHOT_PROFILER_STALLED_TRACE_TIMEOUT, + DEFAULT_SNAPSHOT_PROFILER_STALLED_TRACE_TIMEOUT); + } } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java index 59aeb8ad4..68cd36e27 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java @@ -34,9 +34,7 @@ class StalledTraceDetectingTraceRegistry implements TraceRegistry, AutoCloseable private final Supplier sampler; public StalledTraceDetectingTraceRegistry( - TraceRegistry delegate, - Supplier sampler, - Duration stalledTimeLimit) { + TraceRegistry delegate, Supplier sampler, Duration stalledTimeLimit) { this.delegate = delegate; this.sampler = sampler; diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/ConfigurationTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/ConfigurationTest.java index dac5ac8fa..5eb48b417 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/ConfigurationTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/ConfigurationTest.java @@ -237,4 +237,21 @@ void getDefaultSnapshotProfilerStagingCapacity() { var properties = DefaultConfigProperties.create(Collections.emptyMap()); assertEquals(2000, Configuration.getSnapshotProfilerStagingCapacity(properties)); } + + @ParameterizedTest + @ValueSource(strings = {"10s", "10m", "5h"}) + void getConfiguredSnapshotProfilerStalledTraceTimeout(String value) { + var timeout = Duration.parse("PT" + value); + var properties = + DefaultConfigProperties.create( + Map.of("splunk.snapshot.profiler.stalled.trace.timeout", value)); + assertEquals(timeout, Configuration.getSnapshotProfilerStalledTraceTimeout(properties)); + } + + @Test + void getDefaultSnapshotProfilerStalledTraceTimeout() { + var properties = DefaultConfigProperties.create(Collections.emptyMap()); + var tenMinutes = Duration.ofMinutes(10); + assertEquals(tenMinutes, Configuration.getSnapshotProfilerStalledTraceTimeout(properties)); + } } From c818959d4738e3b0ad108d641a906984959eaf35 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Thu, 8 May 2025 14:35:13 -0700 Subject: [PATCH 13/35] Expand the TraceRegistry interface extend AutoCloseable. --- .../opentelemetry/profiler/snapshot/TraceRegistry.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java index e97d10a23..ab3ea2fce 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java @@ -18,10 +18,13 @@ import io.opentelemetry.api.trace.SpanContext; -interface TraceRegistry { +interface TraceRegistry extends AutoCloseable { void register(SpanContext spanContext); boolean isRegistered(SpanContext spanContext); void unregister(SpanContext spanContext); + + @Override + default void close() throws Exception {} } From 4e170f254a97a4f13522e0a2677e467f5e075be1 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Thu, 8 May 2025 14:36:02 -0700 Subject: [PATCH 14/35] Unregister all traces when closed. --- .../snapshot/SimpleTraceRegistry.java | 10 ++++++++ .../snapshot/SimpleTraceRegistryTest.java | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java index bc852470c..f054b5012 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistry.java @@ -23,9 +23,13 @@ class SimpleTraceRegistry implements TraceRegistry { private final Set traceIds = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private volatile boolean closed = false; @Override public void register(SpanContext spanContext) { + if (closed) { + return; + } traceIds.add(spanContext.getTraceId()); } @@ -38,4 +42,10 @@ public boolean isRegistered(SpanContext spanContext) { public void unregister(SpanContext spanContext) { traceIds.remove(spanContext.getTraceId()); } + + @Override + public void close() { + closed = true; + traceIds.clear(); + } } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistryTest.java index 5b65b9708..798bb8bdd 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SimpleTraceRegistryTest.java @@ -16,6 +16,7 @@ package com.splunk.opentelemetry.profiler.snapshot; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -46,4 +47,28 @@ void unregisterTraceForProfiling() { assertFalse(registry.isRegistered(spanContext)); } + + @Test + void unregisterTracesWhenClosed() { + var spanContext1 = Snapshotting.spanContext().build(); + var spanContext2 = Snapshotting.spanContext().build(); + + registry.register(spanContext1); + registry.register(spanContext2); + + registry.close(); + + assertThat(registry.isRegistered(spanContext1)).isFalse(); + assertThat(registry.isRegistered(spanContext2)).isFalse(); + } + + @Test + void doNotRegisterNewTracesWhenClosed() { + var spanContext = Snapshotting.spanContext().build(); + + registry.close(); + registry.register(spanContext); + + assertThat(registry.isRegistered(spanContext)).isFalse(); + } } From 099096e30cf5c36e8fc29b7aec8be425e465e689 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Thu, 8 May 2025 14:38:02 -0700 Subject: [PATCH 15/35] Close encapsulated TraceRegistry delegate when StalledTraceDetectingTraceRegistry is closed. --- .../StalledTraceDetectingTraceRegistry.java | 5 ++-- ...talledTraceDetectingTraceRegistryTest.java | 25 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java index 68cd36e27..90de709f6 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java @@ -26,7 +26,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.Supplier; -class StalledTraceDetectingTraceRegistry implements TraceRegistry, AutoCloseable { +class StalledTraceDetectingTraceRegistry implements TraceRegistry { private final ScheduledExecutorService scheduler = HelpfulExecutors.newSingleThreadedScheduledExecutor("stalled-trace-detector"); private final Map traceIds = new ConcurrentHashMap<>(); @@ -63,7 +63,8 @@ public void unregister(SpanContext spanContext) { @Override public void close() throws Exception { - scheduler.shutdown(); + scheduler.shutdownNow(); + delegate.close(); } private Runnable removeStalledTraces(Duration stalledTimeLimit) { diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java index ac126207a..24131e226 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java @@ -92,4 +92,29 @@ void stopTraceSamplingWhenStalledTraceUnregistered() { await().untilAsserted(() -> assertThat(sampler.isBeingSampled(spanContext)).isFalse()); } + + @Test + void stopUnregisteredTraceSamplingWhenClosed() throws Exception { + var timeout = Duration.ofMillis(100); + var spanContext = Snapshotting.spanContext().build(); + + var delegate = new RecordingTraceRegistry(); + var registry = new StalledTraceDetectingTraceRegistry(delegate, () -> sampler, timeout); + registry.register(spanContext); + registry.close(); + + Thread.sleep(timeout.toMillis() * 2); + + assertThat(registry.isRegistered(spanContext)).isTrue(); + } + + @Test + void alsoCloseDelegateTraceRegistryWhenClosed() throws Exception { + var spanContext = Snapshotting.spanContext().build(); + + registry.register(spanContext); + registry.close(); + + assertThat(delegate.isRegistered(spanContext)).isFalse(); + } } From 43db5f575a8fa5cbb43c14794b80aa008d00e6dc Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Thu, 8 May 2025 14:41:50 -0700 Subject: [PATCH 16/35] Define a noop TraceRegistry and ConfigurableSupplier. --- .../opentelemetry/profiler/snapshot/TraceRegistry.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java index ab3ea2fce..36d873311 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java @@ -19,6 +19,13 @@ import io.opentelemetry.api.trace.SpanContext; interface TraceRegistry extends AutoCloseable { + TraceRegistry NOOP = new TraceRegistry() { + @Override public void register(SpanContext spanContext) {} + @Override public boolean isRegistered(SpanContext spanContext) { return false; } + @Override public void unregister(SpanContext spanContext) {} + }; + ConfigurableSupplier SUPPLIER = new ConfigurableSupplier<>(NOOP); + void register(SpanContext spanContext); boolean isRegistered(SpanContext spanContext); From 4924bb210ebbdbc4de55b70fa276df43795b4a29 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Fri, 23 May 2025 10:57:11 -0700 Subject: [PATCH 17/35] Access TraceRegistry via a ConfigurableSupplier. --- .../profiler/snapshot/ActiveSpanTracker.java | 7 +-- ...ngContextStorageSpanTrackingActivator.java | 3 +- .../SnapshotProfilingSdkCustomizer.java | 27 ++++++++---- .../SnapshotProfilingSpanProcessor.java | 10 ++--- .../snapshot/SpanTrackingActivator.java | 4 +- .../StalledTraceDetectingTraceRegistry.java | 44 +++++++------------ .../profiler/snapshot/TraceRegistry.java | 7 --- .../snapshot/ActiveSpanTrackerTest.java | 2 +- ...ntextStorageSpanTrackingActivatorTest.java | 4 +- .../SnapshotProfilingLogExportingTest.java | 3 +- ...SnapshotProfilingSdkCustomizerBuilder.java | 2 +- 11 files changed, 54 insertions(+), 59 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java index 01ae49601..86fd9179e 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java @@ -23,15 +23,16 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.internal.cache.Cache; import java.util.Optional; +import java.util.function.Supplier; import javax.annotation.Nullable; class ActiveSpanTracker implements ContextStorage, SpanTracker { private final Cache cache = Cache.weak(); private final ContextStorage delegate; - private final TraceRegistry registry; + private final Supplier registry; - ActiveSpanTracker(ContextStorage delegate, TraceRegistry registry) { + ActiveSpanTracker(ContextStorage delegate, Supplier registry) { this.delegate = delegate; this.registry = registry; } @@ -62,7 +63,7 @@ public Scope attach(Context toAttach) { } private boolean doNotTrack(SpanContext spanContext) { - return !spanContext.isSampled() || !registry.isRegistered(spanContext); + return !spanContext.isSampled() || !registry.get().isRegistered(spanContext); } @Nullable diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java index 282454785..198d961c5 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivator.java @@ -19,6 +19,7 @@ import com.google.common.annotations.VisibleForTesting; import io.opentelemetry.context.ContextStorage; import java.util.function.Consumer; +import java.util.function.Supplier; import java.util.function.UnaryOperator; class InterceptingContextStorageSpanTrackingActivator implements SpanTrackingActivator { @@ -35,7 +36,7 @@ class InterceptingContextStorageSpanTrackingActivator implements SpanTrackingAct } @Override - public void activate(TraceRegistry registry) { + public void activate(Supplier registry) { contextStorageWrappingFunction.accept( contextStorage -> { ActiveSpanTracker tracker = new ActiveSpanTracker(contextStorage, registry); diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java index 0e8669ca0..9bf826c7e 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java @@ -30,16 +30,17 @@ import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; +import java.util.function.Supplier; @AutoService(AutoConfigurationCustomizerProvider.class) public class SnapshotProfilingSdkCustomizer implements AutoConfigurationCustomizerProvider { - private final TraceRegistry registry; + private final ConfigurableSupplier registry; private final Function samplerProvider; private final SpanTrackingActivator spanTrackingActivator; public SnapshotProfilingSdkCustomizer() { this( - new SimpleTraceRegistry(), + new ConfigurableSupplier<>(new SimpleTraceRegistry()), stackTraceSamplerProvider(), new InterceptingContextStorageSpanTrackingActivator()); } @@ -60,13 +61,12 @@ private static StagingArea createStagingArea(ConfigProperties properties) { } @VisibleForTesting - SnapshotProfilingSdkCustomizer( - TraceRegistry registry, StackTraceSampler sampler, SpanTrackingActivator activator) { + SnapshotProfilingSdkCustomizer(ConfigurableSupplier registry, StackTraceSampler sampler, SpanTrackingActivator activator) { this(registry, properties -> sampler, activator); } private SnapshotProfilingSdkCustomizer( - TraceRegistry registry, + ConfigurableSupplier registry, Function samplerProvider, SpanTrackingActivator spanTrackingActivator) { this.registry = registry; @@ -78,11 +78,23 @@ private SnapshotProfilingSdkCustomizer( public void customize(AutoConfigurationCustomizer autoConfigurationCustomizer) { autoConfigurationCustomizer .addPropertiesCustomizer(autoConfigureSnapshotVolumePropagator()) + .addPropertiesCustomizer(configureTraceRegistry(registry)) .addTracerProviderCustomizer(snapshotProfilingSpanProcessor(registry)) .addPropertiesCustomizer(startTrackingActiveSpans(registry)) .addTracerProviderCustomizer(addShutdownHook()); } + private Function> configureTraceRegistry(ConfigurableSupplier registry) { + return properties -> { + if (snapshotProfilingEnabled(properties)) { + TraceRegistry current = registry.get(); + TraceRegistry stalledTraceDetector = new StalledTraceDetectingTraceRegistry(current, StackTraceSampler.SUPPLIER, Duration.ofMinutes(10)); + registry.configure(stalledTraceDetector); + } + return Collections.emptyMap(); + }; + } + private BiFunction addShutdownHook() { return (builder, properties) -> { @@ -94,7 +106,7 @@ public void customize(AutoConfigurationCustomizer autoConfigurationCustomizer) { } private BiFunction - snapshotProfilingSpanProcessor(TraceRegistry registry) { + snapshotProfilingSpanProcessor(Supplier registry) { return (builder, properties) -> { if (snapshotProfilingEnabled(properties)) { StackTraceSampler sampler = samplerProvider.apply(properties); @@ -140,8 +152,7 @@ private boolean includeTraceContextPropagator(Set configuredPropagators) return configuredPropagators.isEmpty(); } - private Function> startTrackingActiveSpans( - TraceRegistry registry) { + private Function> startTrackingActiveSpans(Supplier registry) { return properties -> { if (snapshotProfilingEnabled(properties)) { spanTrackingActivator.activate(registry); diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java index 3905df64b..d645b0141 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java @@ -36,10 +36,10 @@ * be profiled at a time. */ public class SnapshotProfilingSpanProcessor implements SpanProcessor { - private final TraceRegistry registry; + private final Supplier registry; private final Supplier sampler; - SnapshotProfilingSpanProcessor(TraceRegistry registry, Supplier sampler) { + SnapshotProfilingSpanProcessor(Supplier registry, Supplier sampler) { this.registry = registry; this.sampler = sampler; } @@ -49,11 +49,11 @@ public void onStart(Context context, ReadWriteSpan span) { if (isEntry(span)) { Volume volume = Volume.from(context); if (volume == Volume.HIGHEST) { - registry.register(span.getSpanContext()); + registry.get().register(span.getSpanContext()); } } - if (isEntry(span) && registry.isRegistered(span.getSpanContext())) { + if (isEntry(span) && registry.get().isRegistered(span.getSpanContext())) { sampler.get().start(span.getSpanContext()); span.setAttribute(SNAPSHOT_PROFILING, true); } @@ -74,7 +74,7 @@ public boolean isStartRequired() { @Override public void onEnd(ReadableSpan span) { if (isEntry(span)) { - registry.unregister(span.getSpanContext()); + registry.get().unregister(span.getSpanContext()); sampler.get().stop(span.getSpanContext()); } } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java index 87d098d76..3dec8d672 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTrackingActivator.java @@ -16,6 +16,8 @@ package com.splunk.opentelemetry.profiler.snapshot; +import java.util.function.Supplier; + interface SpanTrackingActivator { - void activate(TraceRegistry registry); + void activate(Supplier registry); } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java index 90de709f6..be99f780a 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java @@ -19,7 +19,6 @@ import com.splunk.opentelemetry.profiler.util.HelpfulExecutors; import io.opentelemetry.api.trace.SpanContext; import java.time.Duration; -import java.time.Instant; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; @@ -29,17 +28,19 @@ class StalledTraceDetectingTraceRegistry implements TraceRegistry { private final ScheduledExecutorService scheduler = HelpfulExecutors.newSingleThreadedScheduledExecutor("stalled-trace-detector"); - private final Map traceIds = new ConcurrentHashMap<>(); + private final Map traceIds = new ConcurrentHashMap<>(); private final TraceRegistry delegate; private final Supplier sampler; + private final Duration stalledTimeLimit; public StalledTraceDetectingTraceRegistry( TraceRegistry delegate, Supplier sampler, Duration stalledTimeLimit) { this.delegate = delegate; this.sampler = sampler; + this.stalledTimeLimit = stalledTimeLimit; scheduler.scheduleAtFixedRate( - removeStalledTraces(stalledTimeLimit), + removeStalledTraces(), stalledTimeLimit.toMillis() / 2, stalledTimeLimit.toMillis() / 2, TimeUnit.MILLISECONDS); @@ -48,7 +49,7 @@ public StalledTraceDetectingTraceRegistry( @Override public void register(SpanContext spanContext) { delegate.register(spanContext); - traceIds.put(spanContext.getTraceId(), new RegistrationContext(Instant.now(), spanContext)); + traceIds.put(spanContext, System.nanoTime() + stalledTimeLimit.toNanos()); } @Override @@ -59,6 +60,7 @@ public boolean isRegistered(SpanContext spanContext) { @Override public void unregister(SpanContext spanContext) { delegate.unregister(spanContext); + traceIds.remove(spanContext); } @Override @@ -67,30 +69,14 @@ public void close() throws Exception { delegate.close(); } - private Runnable removeStalledTraces(Duration stalledTimeLimit) { - return () -> - traceIds - .entrySet() - .iterator() - .forEachRemaining( - entry -> { - Instant now = Instant.now(); - RegistrationContext context = entry.getValue(); - Duration duration = Duration.between(now, context.registrationTime); - if (duration.compareTo(stalledTimeLimit) <= 0) { - unregister(context.spanContext); - sampler.get().stop(context.spanContext); - } - }); - } - - private static class RegistrationContext { - private final Instant registrationTime; - private final SpanContext spanContext; - - private RegistrationContext(Instant registrationTime, SpanContext spanContext) { - this.registrationTime = registrationTime; - this.spanContext = spanContext; - } + private Runnable removeStalledTraces() { + return () -> traceIds.entrySet().stream() + .filter(entry -> entry.getValue() <= System.nanoTime()) + .map(Map.Entry::getKey) + .iterator() + .forEachRemaining(spanContext -> { + unregister(spanContext); + sampler.get().stop(spanContext); + }); } } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java index 36d873311..ab3ea2fce 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java @@ -19,13 +19,6 @@ import io.opentelemetry.api.trace.SpanContext; interface TraceRegistry extends AutoCloseable { - TraceRegistry NOOP = new TraceRegistry() { - @Override public void register(SpanContext spanContext) {} - @Override public boolean isRegistered(SpanContext spanContext) { return false; } - @Override public void unregister(SpanContext spanContext) {} - }; - ConfigurableSupplier SUPPLIER = new ConfigurableSupplier<>(NOOP); - void register(SpanContext spanContext); boolean isRegistered(SpanContext spanContext); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java index dc498418f..cd3e8f26c 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java @@ -40,7 +40,7 @@ class ActiveSpanTrackerTest { private final ContextStorage storage = ContextStorage.get(); private final TogglableTraceRegistry registry = new TogglableTraceRegistry(); - private final ActiveSpanTracker spanTracker = new ActiveSpanTracker(storage, registry); + private final ActiveSpanTracker spanTracker = new ActiveSpanTracker(storage, () -> registry); @Test void currentContextComesFromOpenTelemetryContextStorage() { diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivatorTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivatorTest.java index d09e30cdc..5a1b44954 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivatorTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InterceptingContextStorageSpanTrackingActivatorTest.java @@ -30,13 +30,13 @@ class InterceptingContextStorageSpanTrackingActivatorTest { @Test void interceptContextStorage() { - activator.activate(new SimpleTraceRegistry()); + activator.activate(SimpleTraceRegistry::new); assertInstanceOf(ActiveSpanTracker.class, delegate.storage); } @Test void activateSpanTracker() { - activator.activate(new SimpleTraceRegistry()); + activator.activate(SimpleTraceRegistry::new); assertInstanceOf(ActiveSpanTracker.class, SpanTracker.SUPPLIER.get()); } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java index 8f5480a8b..2b696c10f 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingLogExportingTest.java @@ -37,6 +37,7 @@ import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.AfterEach; @@ -106,7 +107,7 @@ private Predicate> label(AttributeKey key) { private static class ResetContextStorage implements SpanTrackingActivator, AfterEachCallback { @Override - public void activate(TraceRegistry registry) { + public void activate(Supplier registry) { ActiveSpanTracker spanTracker = new ActiveSpanTracker(ContextStorage.defaultStorage(), registry); SpanTracker.SUPPLIER.configure(spanTracker); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java index ab01e7295..06ab5b5aa 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java @@ -54,6 +54,6 @@ SnapshotProfilingSdkCustomizerBuilder with(SpanTrackingActivator spanTrackingAct } SnapshotProfilingSdkCustomizer build() { - return new SnapshotProfilingSdkCustomizer(registry, sampler, spanTrackingActivator); + return new SnapshotProfilingSdkCustomizer(new ConfigurableSupplier<>(registry), sampler, spanTrackingActivator); } } From b6e8b7687c81644ee2ec6379ae7a021be6084eef Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Fri, 23 May 2025 11:49:31 -0700 Subject: [PATCH 18/35] Use a daemon thread rather than a ScheduledExecutorService for detecting stalled traces. --- .../StalledTraceDetectingTraceRegistry.java | 131 ++++++++++++++---- 1 file changed, 103 insertions(+), 28 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java index be99f780a..498ab78c6 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java @@ -16,40 +16,32 @@ package com.splunk.opentelemetry.profiler.snapshot; -import com.splunk.opentelemetry.profiler.util.HelpfulExecutors; import io.opentelemetry.api.trace.SpanContext; import java.time.Duration; +import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; class StalledTraceDetectingTraceRegistry implements TraceRegistry { - private final ScheduledExecutorService scheduler = - HelpfulExecutors.newSingleThreadedScheduledExecutor("stalled-trace-detector"); - private final Map traceIds = new ConcurrentHashMap<>(); private final TraceRegistry delegate; - private final Supplier sampler; - private final Duration stalledTimeLimit; + private final StalledTraceDetector stalledTraceDetector; public StalledTraceDetectingTraceRegistry( TraceRegistry delegate, Supplier sampler, Duration stalledTimeLimit) { this.delegate = delegate; - this.sampler = sampler; - this.stalledTimeLimit = stalledTimeLimit; - - scheduler.scheduleAtFixedRate( - removeStalledTraces(), - stalledTimeLimit.toMillis() / 2, - stalledTimeLimit.toMillis() / 2, - TimeUnit.MILLISECONDS); + + stalledTraceDetector = new StalledTraceDetector(delegate, sampler, stalledTimeLimit); + stalledTraceDetector.setName("stalled-trace-detector"); + stalledTraceDetector.setDaemon(true); + stalledTraceDetector.start(); } @Override public void register(SpanContext spanContext) { delegate.register(spanContext); - traceIds.put(spanContext, System.nanoTime() + stalledTimeLimit.toNanos()); + stalledTraceDetector.register(spanContext); } @Override @@ -60,23 +52,106 @@ public boolean isRegistered(SpanContext spanContext) { @Override public void unregister(SpanContext spanContext) { delegate.unregister(spanContext); - traceIds.remove(spanContext); + stalledTraceDetector.unregister(spanContext); } @Override public void close() throws Exception { - scheduler.shutdownNow(); delegate.close(); + stalledTraceDetector.shutdown(); } - private Runnable removeStalledTraces() { - return () -> traceIds.entrySet().stream() - .filter(entry -> entry.getValue() <= System.nanoTime()) - .map(Map.Entry::getKey) - .iterator() - .forEachRemaining(spanContext -> { - unregister(spanContext); - sampler.get().stop(spanContext); - }); + private static class StalledTraceDetector extends Thread { + private final Map traceIds = new HashMap<>(); + private final LinkedBlockingQueue queue = new LinkedBlockingQueue<>(); + private final TraceRegistry registry; + private final Supplier sampler; + private final Duration timeout; + + private boolean shutdown = false; + private long nextStallCheck; + + StalledTraceDetector(TraceRegistry registry, Supplier sampler, Duration timeout) { + this.registry = registry; + this.sampler = sampler; + this.timeout = timeout; + updateNextExportTime(); + } + + public void register(SpanContext spanContext) { + queue.add(Command.register(spanContext)); + } + + public void unregister(SpanContext spanContext) { + queue.add(Command.unregister(spanContext)); + } + + public void shutdown() { + shutdown = true; + queue.add(Command.SHUTDOWN); + } + + @Override + public void run() { + while(!shutdown) { + try { + Command command = queue.poll(nextStallCheck - System.nanoTime(), TimeUnit.NANOSECONDS); + if (command != null) { + switch (command.action) { + case REGISTER: + traceIds.put(command.spanContext, System.nanoTime() + timeout.toNanos()); + break; + case UNREGISTER: + traceIds.remove(command.spanContext); + break; + case SHUTDOWN: + traceIds.clear(); + return; + } + } + removeStalledTraces(); + updateNextExportTime(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + + private void removeStalledTraces() { + traceIds.entrySet().stream() + .filter(entry -> entry.getValue() <= System.nanoTime()) + .map(Map.Entry::getKey) + .iterator() + .forEachRemaining(spanContext -> { + registry.unregister(spanContext); + sampler.get().stop(spanContext); + }); + } + + private void updateNextExportTime() { + nextStallCheck = System.nanoTime() + timeout.toNanos(); + } + + private static class Command { + public static final Command SHUTDOWN = new Command(Action.SHUTDOWN, null); + + public static Command register(SpanContext spanContext) { + return new Command(Action.REGISTER, spanContext); + } + + public static Command unregister(SpanContext spanContext) { + return new Command(Action.UNREGISTER, spanContext); + } + + private final Action action; + private final SpanContext spanContext; + + private Command(Action action, SpanContext spanContext) { + this.action = action; + this.spanContext = spanContext; + } + } + + private enum Action { REGISTER, UNREGISTER, SHUTDOWN } } } From a534305f0554318a894d79514ee8b7e711a17a02 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Fri, 23 May 2025 11:51:20 -0700 Subject: [PATCH 19/35] Apply spotless code formatting. --- .../SnapshotProfilingSdkCustomizer.java | 17 ++++++++++++---- .../SnapshotProfilingSpanProcessor.java | 3 ++- .../StalledTraceDetectingTraceRegistry.java | 20 ++++++++++++------- ...SnapshotProfilingSdkCustomizerBuilder.java | 3 ++- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java index 9bf826c7e..910ac732c 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java @@ -61,7 +61,10 @@ private static StagingArea createStagingArea(ConfigProperties properties) { } @VisibleForTesting - SnapshotProfilingSdkCustomizer(ConfigurableSupplier registry, StackTraceSampler sampler, SpanTrackingActivator activator) { + SnapshotProfilingSdkCustomizer( + ConfigurableSupplier registry, + StackTraceSampler sampler, + SpanTrackingActivator activator) { this(registry, properties -> sampler, activator); } @@ -84,11 +87,16 @@ public void customize(AutoConfigurationCustomizer autoConfigurationCustomizer) { .addTracerProviderCustomizer(addShutdownHook()); } - private Function> configureTraceRegistry(ConfigurableSupplier registry) { + private Function> configureTraceRegistry( + ConfigurableSupplier registry) { return properties -> { if (snapshotProfilingEnabled(properties)) { TraceRegistry current = registry.get(); - TraceRegistry stalledTraceDetector = new StalledTraceDetectingTraceRegistry(current, StackTraceSampler.SUPPLIER, Duration.ofMinutes(10)); + TraceRegistry stalledTraceDetector = + new StalledTraceDetectingTraceRegistry( + current, + StackTraceSampler.SUPPLIER, + Configuration.getSnapshotProfilerStalledTraceTimeout(properties)); registry.configure(stalledTraceDetector); } return Collections.emptyMap(); @@ -152,7 +160,8 @@ private boolean includeTraceContextPropagator(Set configuredPropagators) return configuredPropagators.isEmpty(); } - private Function> startTrackingActiveSpans(Supplier registry) { + private Function> startTrackingActiveSpans( + Supplier registry) { return properties -> { if (snapshotProfilingEnabled(properties)) { spanTrackingActivator.activate(registry); diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java index d645b0141..4661d9cc7 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java @@ -39,7 +39,8 @@ public class SnapshotProfilingSpanProcessor implements SpanProcessor { private final Supplier registry; private final Supplier sampler; - SnapshotProfilingSpanProcessor(Supplier registry, Supplier sampler) { + SnapshotProfilingSpanProcessor( + Supplier registry, Supplier sampler) { this.registry = registry; this.sampler = sampler; } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java index 498ab78c6..b1660bb30 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java @@ -71,7 +71,8 @@ private static class StalledTraceDetector extends Thread { private boolean shutdown = false; private long nextStallCheck; - StalledTraceDetector(TraceRegistry registry, Supplier sampler, Duration timeout) { + StalledTraceDetector( + TraceRegistry registry, Supplier sampler, Duration timeout) { this.registry = registry; this.sampler = sampler; this.timeout = timeout; @@ -93,7 +94,7 @@ public void shutdown() { @Override public void run() { - while(!shutdown) { + while (!shutdown) { try { Command command = queue.poll(nextStallCheck - System.nanoTime(), TimeUnit.NANOSECONDS); if (command != null) { @@ -122,10 +123,11 @@ private void removeStalledTraces() { .filter(entry -> entry.getValue() <= System.nanoTime()) .map(Map.Entry::getKey) .iterator() - .forEachRemaining(spanContext -> { - registry.unregister(spanContext); - sampler.get().stop(spanContext); - }); + .forEachRemaining( + spanContext -> { + registry.unregister(spanContext); + sampler.get().stop(spanContext); + }); } private void updateNextExportTime() { @@ -152,6 +154,10 @@ private Command(Action action, SpanContext spanContext) { } } - private enum Action { REGISTER, UNREGISTER, SHUTDOWN } + private enum Action { + REGISTER, + UNREGISTER, + SHUTDOWN + } } } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java index 06ab5b5aa..64382375d 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizerBuilder.java @@ -54,6 +54,7 @@ SnapshotProfilingSdkCustomizerBuilder with(SpanTrackingActivator spanTrackingAct } SnapshotProfilingSdkCustomizer build() { - return new SnapshotProfilingSdkCustomizer(new ConfigurableSupplier<>(registry), sampler, spanTrackingActivator); + return new SnapshotProfilingSdkCustomizer( + new ConfigurableSupplier<>(registry), sampler, spanTrackingActivator); } } From 8858d7a8694a3c3e74ba78fd4d76f4b06b66ac32 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Fri, 23 May 2025 15:37:15 -0700 Subject: [PATCH 20/35] If if/else rather than switch. --- .../StalledTraceDetectingTraceRegistry.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java index b1660bb30..4a87f5b8c 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java @@ -17,6 +17,7 @@ package com.splunk.opentelemetry.profiler.snapshot; import io.opentelemetry.api.trace.SpanContext; + import java.time.Duration; import java.util.HashMap; import java.util.Map; @@ -98,16 +99,13 @@ public void run() { try { Command command = queue.poll(nextStallCheck - System.nanoTime(), TimeUnit.NANOSECONDS); if (command != null) { - switch (command.action) { - case REGISTER: - traceIds.put(command.spanContext, System.nanoTime() + timeout.toNanos()); - break; - case UNREGISTER: - traceIds.remove(command.spanContext); - break; - case SHUTDOWN: - traceIds.clear(); - return; + if (command.action == Action.SHUTDOWN) { + traceIds.clear(); + return; + } else if(command.action == Action.REGISTER) { + traceIds.put(command.spanContext, System.nanoTime() + timeout.toNanos()); + } else if (command.action == Action.UNREGISTER) { + traceIds.remove(command.spanContext); } } removeStalledTraces(); From 1054b7583ed4c79f7937b92627b157d93ff3dd60 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Fri, 23 May 2025 15:56:07 -0700 Subject: [PATCH 21/35] Close TraceRegistry when OpenTelemetry SDK shutdown. --- .../profiler/snapshot/SdkShutdownHook.java | 24 ++++++++++++++--- .../SnapshotProfilingSdkCustomizer.java | 11 +++++--- .../StalledTraceDetectingTraceRegistry.java | 5 ++-- .../profiler/snapshot/TraceRegistry.java | 5 ++-- .../snapshot/SdkShutdownHookTest.java | 26 +++++++++++++++++-- 5 files changed, 58 insertions(+), 13 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SdkShutdownHook.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SdkShutdownHook.java index 2b05b0646..058702728 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SdkShutdownHook.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SdkShutdownHook.java @@ -24,14 +24,32 @@ import java.io.Closeable; import java.util.ArrayList; import java.util.List; +import java.util.function.Supplier; class SdkShutdownHook implements SpanProcessor { + private final Supplier registry; + private final Supplier sampler; + private final Supplier stagingArea; + private final Supplier exporter; + + SdkShutdownHook( + Supplier registry, + Supplier sampler, + Supplier stagingArea, + Supplier exporter) { + this.registry = registry; + this.sampler = sampler; + this.stagingArea = stagingArea; + this.exporter = exporter; + } + @Override public CompletableResultCode shutdown() { List results = new ArrayList<>(); - results.add(close(StackTraceSampler.SUPPLIER.get())); - results.add(close(StagingArea.SUPPLIER.get())); - results.add(close(StackTraceExporter.SUPPLIER.get())); + results.add(close(registry.get())); + results.add(close(sampler.get())); + results.add(close(stagingArea.get())); + results.add(close(exporter.get())); return CompletableResultCode.ofAll(results); } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java index 910ac732c..69b1a012e 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java @@ -84,7 +84,7 @@ public void customize(AutoConfigurationCustomizer autoConfigurationCustomizer) { .addPropertiesCustomizer(configureTraceRegistry(registry)) .addTracerProviderCustomizer(snapshotProfilingSpanProcessor(registry)) .addPropertiesCustomizer(startTrackingActiveSpans(registry)) - .addTracerProviderCustomizer(addShutdownHook()); + .addTracerProviderCustomizer(addShutdownHook(registry)); } private Function> configureTraceRegistry( @@ -104,10 +104,15 @@ private Function> configureTraceRegistry( } private BiFunction - addShutdownHook() { + addShutdownHook(Supplier registry) { return (builder, properties) -> { if (snapshotProfilingEnabled(properties)) { - builder.addSpanProcessor(new SdkShutdownHook()); + builder.addSpanProcessor( + new SdkShutdownHook( + registry, + StackTraceSampler.SUPPLIER, + StagingArea.SUPPLIER, + StackTraceExporter.SUPPLIER)); } return builder; }; diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java index 4a87f5b8c..bac3506fc 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java @@ -17,7 +17,6 @@ package com.splunk.opentelemetry.profiler.snapshot; import io.opentelemetry.api.trace.SpanContext; - import java.time.Duration; import java.util.HashMap; import java.util.Map; @@ -57,7 +56,7 @@ public void unregister(SpanContext spanContext) { } @Override - public void close() throws Exception { + public void close() { delegate.close(); stalledTraceDetector.shutdown(); } @@ -102,7 +101,7 @@ public void run() { if (command.action == Action.SHUTDOWN) { traceIds.clear(); return; - } else if(command.action == Action.REGISTER) { + } else if (command.action == Action.REGISTER) { traceIds.put(command.spanContext, System.nanoTime() + timeout.toNanos()); } else if (command.action == Action.UNREGISTER) { traceIds.remove(command.spanContext); diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java index ab3ea2fce..989bac523 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/TraceRegistry.java @@ -17,8 +17,9 @@ package com.splunk.opentelemetry.profiler.snapshot; import io.opentelemetry.api.trace.SpanContext; +import java.io.Closeable; -interface TraceRegistry extends AutoCloseable { +interface TraceRegistry extends Closeable { void register(SpanContext spanContext); boolean isRegistered(SpanContext spanContext); @@ -26,5 +27,5 @@ interface TraceRegistry extends AutoCloseable { void unregister(SpanContext spanContext); @Override - default void close() throws Exception {} + default void close() {} } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SdkShutdownHookTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SdkShutdownHookTest.java index 552c85500..cb711eb81 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SdkShutdownHookTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/SdkShutdownHookTest.java @@ -24,7 +24,18 @@ class SdkShutdownHookTest { private final ClosingObserver observer = new ClosingObserver(); - private final SdkShutdownHook shutdownHook = new SdkShutdownHook(); + private final ConfigurableSupplier registry = + new ConfigurableSupplier<>(new SimpleTraceRegistry()); + private final SdkShutdownHook shutdownHook = + new SdkShutdownHook( + registry, StackTraceSampler.SUPPLIER, StagingArea.SUPPLIER, StackTraceExporter.SUPPLIER); + + @Test + void shutdownTraceRegistrySampling() { + registry.configure(observer); + shutdownHook.shutdown(); + assertThat(observer.isClosed).isTrue(); + } @Test void shutdownStackTraceSampling() { @@ -60,7 +71,7 @@ void shutdownStackTraceExporting() { } private static class ClosingObserver - implements StackTraceSampler, StagingArea, StackTraceExporter { + implements TraceRegistry, StackTraceSampler, StagingArea, StackTraceExporter { private boolean isClosed = false; @Override @@ -68,6 +79,17 @@ public void close() { isClosed = true; } + @Override + public void register(SpanContext spanContext) {} + + @Override + public boolean isRegistered(SpanContext spanContext) { + return false; + } + + @Override + public void unregister(SpanContext spanContext) {} + @Override public void start(SpanContext spanContext) {} From af1beabc1ab272395a5f4c759e0bf09435ce51c4 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 27 May 2025 14:43:08 -0700 Subject: [PATCH 22/35] Detect orphaned traces using WeakReferences and a ReferenceQueue rather than waiting for some pre-determined timeout. --- .../opentelemetry/profiler/Configuration.java | 11 -- .../OrphanedTraceDetectingTraceRegistry.java | 110 ++++++++++++ .../SnapshotProfilingSdkCustomizer.java | 9 +- .../StalledTraceDetectingTraceRegistry.java | 160 ------------------ .../profiler/ConfigurationTest.java | 17 -- ...hanedTraceDetectingTraceRegistryTest.java} | 75 +++----- 6 files changed, 137 insertions(+), 245 deletions(-) create mode 100644 profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java delete mode 100644 profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java rename profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/{StalledTraceDetectingTraceRegistryTest.java => OrphanedTraceDetectingTraceRegistryTest.java} (51%) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/Configuration.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/Configuration.java index 640f5d99f..89a302ec0 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/Configuration.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/Configuration.java @@ -89,11 +89,6 @@ public class Configuration implements AutoConfigurationCustomizerProvider { "splunk.snapshot.profiler.staging.capacity"; private static final int DEFAULT_SNAPSHOT_PROFILER_STAGING_CAPACITY = 2000; - private static final String CONFIG_KEY_SNAPSHOT_PROFILER_STALLED_TRACE_TIMEOUT = - "splunk.snapshot.profiler.stalled.trace.timeout"; - private static final Duration DEFAULT_SNAPSHOT_PROFILER_STALLED_TRACE_TIMEOUT = - Duration.ofMinutes(10); - @Override public void customize(AutoConfigurationCustomizer autoConfiguration) { autoConfiguration.addPropertiesSupplier(this::defaultProperties); @@ -251,10 +246,4 @@ public static int getSnapshotProfilerStagingCapacity(ConfigProperties properties return properties.getInt( CONFIG_KEY_SNAPSHOT_PROFILER_STAGING_CAPACITY, DEFAULT_SNAPSHOT_PROFILER_STAGING_CAPACITY); } - - public static Duration getSnapshotProfilerStalledTraceTimeout(ConfigProperties properties) { - return properties.getDuration( - CONFIG_KEY_SNAPSHOT_PROFILER_STALLED_TRACE_TIMEOUT, - DEFAULT_SNAPSHOT_PROFILER_STALLED_TRACE_TIMEOUT); - } } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java new file mode 100644 index 000000000..3243e9f31 --- /dev/null +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java @@ -0,0 +1,110 @@ +/* + * Copyright Splunk Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.splunk.opentelemetry.profiler.snapshot; + +import io.opentelemetry.api.trace.SpanContext; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.util.Collections; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; + +class OrphanedTraceDetectingTraceRegistry implements TraceRegistry { + private final Set traces = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final ReferenceQueue referenceQueue = new ReferenceQueue<>(); + + private final TraceRegistry delegate; + private final Supplier sampler; + private final Thread thread; + + OrphanedTraceDetectingTraceRegistry(TraceRegistry delegate, Supplier sampler) { + this.delegate = delegate; + this.sampler = sampler; + + thread = new Thread(this::unregisterOrphanedTraces); + thread.setName("orphaned-trace-detector"); + thread.setDaemon(true); + thread.start(); + } + + @Override + public void register(SpanContext spanContext) { + delegate.register(spanContext); + traces.add(new WeakSpanContext(spanContext, referenceQueue)); + } + + @Override + public boolean isRegistered(SpanContext spanContext) { + return delegate.isRegistered(spanContext); + } + + @Override + public void unregister(SpanContext spanContext) { + delegate.unregister(spanContext); + traces.remove(new WeakSpanContext(spanContext, referenceQueue)); + } + + public void unregisterOrphanedTraces() { + while (!Thread.interrupted()) { + try { + Object reference = referenceQueue.remove(); + if (reference != null) { + WeakSpanContext weakContext = (WeakSpanContext) reference; + traces.remove(weakContext); + delegate.unregister(weakContext.spanContext); + sampler.get().stop(weakContext.spanContext); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + + @Override + public void close() { + delegate.close(); + thread.interrupt(); + } + + private static class WeakSpanContext extends WeakReference { + private final SpanContext spanContext; + + public WeakSpanContext(SpanContext referent, ReferenceQueue q) { + super(referent, q); + this.spanContext = + SpanContext.create( + referent.getTraceId(), + referent.getSpanId(), + referent.getTraceFlags(), + referent.getTraceState()); + } + + @Override + public int hashCode() { + return Objects.hashCode(spanContext); + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) return false; + WeakSpanContext that = (WeakSpanContext) o; + return Objects.equals(spanContext, that.spanContext); + } + } +} diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java index 69b1a012e..8922c24a5 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSdkCustomizer.java @@ -92,12 +92,9 @@ private Function> configureTraceRegistry( return properties -> { if (snapshotProfilingEnabled(properties)) { TraceRegistry current = registry.get(); - TraceRegistry stalledTraceDetector = - new StalledTraceDetectingTraceRegistry( - current, - StackTraceSampler.SUPPLIER, - Configuration.getSnapshotProfilerStalledTraceTimeout(properties)); - registry.configure(stalledTraceDetector); + TraceRegistry orphanedTraceDetector = + new OrphanedTraceDetectingTraceRegistry(current, StackTraceSampler.SUPPLIER); + registry.configure(orphanedTraceDetector); } return Collections.emptyMap(); }; diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java deleted file mode 100644 index bac3506fc..000000000 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistry.java +++ /dev/null @@ -1,160 +0,0 @@ -/* - * Copyright Splunk Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.splunk.opentelemetry.profiler.snapshot; - -import io.opentelemetry.api.trace.SpanContext; -import java.time.Duration; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.TimeUnit; -import java.util.function.Supplier; - -class StalledTraceDetectingTraceRegistry implements TraceRegistry { - private final TraceRegistry delegate; - private final StalledTraceDetector stalledTraceDetector; - - public StalledTraceDetectingTraceRegistry( - TraceRegistry delegate, Supplier sampler, Duration stalledTimeLimit) { - this.delegate = delegate; - - stalledTraceDetector = new StalledTraceDetector(delegate, sampler, stalledTimeLimit); - stalledTraceDetector.setName("stalled-trace-detector"); - stalledTraceDetector.setDaemon(true); - stalledTraceDetector.start(); - } - - @Override - public void register(SpanContext spanContext) { - delegate.register(spanContext); - stalledTraceDetector.register(spanContext); - } - - @Override - public boolean isRegistered(SpanContext spanContext) { - return delegate.isRegistered(spanContext); - } - - @Override - public void unregister(SpanContext spanContext) { - delegate.unregister(spanContext); - stalledTraceDetector.unregister(spanContext); - } - - @Override - public void close() { - delegate.close(); - stalledTraceDetector.shutdown(); - } - - private static class StalledTraceDetector extends Thread { - private final Map traceIds = new HashMap<>(); - private final LinkedBlockingQueue queue = new LinkedBlockingQueue<>(); - private final TraceRegistry registry; - private final Supplier sampler; - private final Duration timeout; - - private boolean shutdown = false; - private long nextStallCheck; - - StalledTraceDetector( - TraceRegistry registry, Supplier sampler, Duration timeout) { - this.registry = registry; - this.sampler = sampler; - this.timeout = timeout; - updateNextExportTime(); - } - - public void register(SpanContext spanContext) { - queue.add(Command.register(spanContext)); - } - - public void unregister(SpanContext spanContext) { - queue.add(Command.unregister(spanContext)); - } - - public void shutdown() { - shutdown = true; - queue.add(Command.SHUTDOWN); - } - - @Override - public void run() { - while (!shutdown) { - try { - Command command = queue.poll(nextStallCheck - System.nanoTime(), TimeUnit.NANOSECONDS); - if (command != null) { - if (command.action == Action.SHUTDOWN) { - traceIds.clear(); - return; - } else if (command.action == Action.REGISTER) { - traceIds.put(command.spanContext, System.nanoTime() + timeout.toNanos()); - } else if (command.action == Action.UNREGISTER) { - traceIds.remove(command.spanContext); - } - } - removeStalledTraces(); - updateNextExportTime(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - } - - private void removeStalledTraces() { - traceIds.entrySet().stream() - .filter(entry -> entry.getValue() <= System.nanoTime()) - .map(Map.Entry::getKey) - .iterator() - .forEachRemaining( - spanContext -> { - registry.unregister(spanContext); - sampler.get().stop(spanContext); - }); - } - - private void updateNextExportTime() { - nextStallCheck = System.nanoTime() + timeout.toNanos(); - } - - private static class Command { - public static final Command SHUTDOWN = new Command(Action.SHUTDOWN, null); - - public static Command register(SpanContext spanContext) { - return new Command(Action.REGISTER, spanContext); - } - - public static Command unregister(SpanContext spanContext) { - return new Command(Action.UNREGISTER, spanContext); - } - - private final Action action; - private final SpanContext spanContext; - - private Command(Action action, SpanContext spanContext) { - this.action = action; - this.spanContext = spanContext; - } - } - - private enum Action { - REGISTER, - UNREGISTER, - SHUTDOWN - } - } -} diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/ConfigurationTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/ConfigurationTest.java index 5eb48b417..dac5ac8fa 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/ConfigurationTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/ConfigurationTest.java @@ -237,21 +237,4 @@ void getDefaultSnapshotProfilerStagingCapacity() { var properties = DefaultConfigProperties.create(Collections.emptyMap()); assertEquals(2000, Configuration.getSnapshotProfilerStagingCapacity(properties)); } - - @ParameterizedTest - @ValueSource(strings = {"10s", "10m", "5h"}) - void getConfiguredSnapshotProfilerStalledTraceTimeout(String value) { - var timeout = Duration.parse("PT" + value); - var properties = - DefaultConfigProperties.create( - Map.of("splunk.snapshot.profiler.stalled.trace.timeout", value)); - assertEquals(timeout, Configuration.getSnapshotProfilerStalledTraceTimeout(properties)); - } - - @Test - void getDefaultSnapshotProfilerStalledTraceTimeout() { - var properties = DefaultConfigProperties.create(Collections.emptyMap()); - var tenMinutes = Duration.ofMinutes(10); - assertEquals(tenMinutes, Configuration.getSnapshotProfilerStalledTraceTimeout(properties)); - } } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java similarity index 51% rename from profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java rename to profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java index 24131e226..3f74427a2 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/StalledTraceDetectingTraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java @@ -18,21 +18,23 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; +import static org.junit.jupiter.api.Assertions.assertFalse; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.instrumentation.test.utils.GcUtils; +import java.lang.ref.WeakReference; import java.time.Duration; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; -class StalledTraceDetectingTraceRegistryTest { - private static final Duration STALLED_TIME_LIMIT = Duration.ofMillis(10); - +class OrphanedTraceDetectingTraceRegistryTest { private final TraceRegistry delegate = new SimpleTraceRegistry(); private final ObservableStackTraceSampler sampler = new ObservableStackTraceSampler(); - private final StalledTraceDetectingTraceRegistry registry = - new StalledTraceDetectingTraceRegistry(delegate, () -> sampler, STALLED_TIME_LIMIT); + private final OrphanedTraceDetectingTraceRegistry registry = + new OrphanedTraceDetectingTraceRegistry(delegate, () -> sampler); @AfterEach - void teardown() throws Exception { + void teardown() { registry.close(); } @@ -61,60 +63,31 @@ void delegateTraceRegistrationLookup() { } @Test - void removeRegisteredTracesAfterStalledTimeLimitPasses() { - var spanContext = Snapshotting.spanContext().build(); - - registry.register(spanContext); - assertThat(registry.isRegistered(spanContext)).isTrue(); - - await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); - } - - @Test - void continueRemovingRegisteredTracesAfterStalledTimeLimitPasses() { + void closeDelegateTraceRegistryWhenClosed() { var spanContext = Snapshotting.spanContext().build(); - registry.register(spanContext); - assertThat(registry.isRegistered(spanContext)).isTrue(); - await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); - - registry.register(spanContext); - assertThat(registry.isRegistered(spanContext)).isTrue(); - await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContext)).isFalse()); - } - - @Test - void stopTraceSamplingWhenStalledTraceUnregistered() { - var spanContext = Snapshotting.spanContext().build(); - - registry.register(spanContext); - sampler.start(spanContext); - - await().untilAsserted(() -> assertThat(sampler.isBeingSampled(spanContext)).isFalse()); - } - - @Test - void stopUnregisteredTraceSamplingWhenClosed() throws Exception { - var timeout = Duration.ofMillis(100); - var spanContext = Snapshotting.spanContext().build(); - - var delegate = new RecordingTraceRegistry(); - var registry = new StalledTraceDetectingTraceRegistry(delegate, () -> sampler, timeout); registry.register(spanContext); registry.close(); - Thread.sleep(timeout.toMillis() * 2); - - assertThat(registry.isRegistered(spanContext)).isTrue(); + assertThat(delegate.isRegistered(spanContext)).isFalse(); } @Test - void alsoCloseDelegateTraceRegistryWhenClosed() throws Exception { + void unregisterOrphanedTraces() throws Exception { var spanContext = Snapshotting.spanContext().build(); - registry.register(spanContext); - registry.close(); - - assertThat(delegate.isRegistered(spanContext)).isFalse(); + var spanContextCopy = + SpanContext.create( + spanContext.getTraceId(), + spanContext.getSpanId(), + spanContext.getTraceFlags(), + spanContext.getTraceState()); + + var spanContextReference = new WeakReference<>(spanContext); + spanContext = null; + GcUtils.awaitGc(spanContextReference, Duration.ofSeconds(10)); + + await().untilAsserted(() -> assertFalse(registry.isRegistered(spanContextCopy))); + await().untilAsserted(() -> assertFalse(delegate.isRegistered(spanContextCopy))); } } From 221b5d612f28016ca25293325bc49a08f498a68d Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Tue, 27 May 2025 16:09:14 -0700 Subject: [PATCH 23/35] Introduce the TraceProfilingContext in ScheduledExecutorStackTraceSampler to remove storing direct refrences to SpanContext instances. --- .../ScheduledExecutorStackTraceSampler.java | 49 ++++++++++++++----- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java index 626ac4e6c..8bc0a1cb6 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java @@ -23,6 +23,7 @@ import java.lang.management.ThreadMXBean; import java.time.Duration; import java.time.Instant; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ScheduledExecutorService; @@ -59,7 +60,8 @@ public void start(SpanContext spanContext) { } samplers.computeIfAbsent( - spanContext.getTraceId(), id -> new ThreadSampler(spanContext, samplingPeriod)); + spanContext.getTraceId(), + id -> new ThreadSampler(TraceProfilingContext.from(spanContext), samplingPeriod)); } @Override @@ -67,7 +69,8 @@ public void stop(SpanContext spanContext) { samplers.computeIfPresent( spanContext.getTraceId(), (traceId, sampler) -> { - if (spanContext.equals(sampler.getSpanContext())) { + TraceProfilingContext context = TraceProfilingContext.from(spanContext); + if (context.equals(sampler.context)) { sampler.shutdown(); waitForShutdown(sampler); return null; @@ -98,17 +101,15 @@ private void waitForShutdown(ThreadSampler sampler) { private class ThreadSampler { private final ScheduledExecutorService scheduler; - private final SpanContext spanContext; + private final TraceProfilingContext context; private final StackTraceGatherer gatherer; - ThreadSampler(SpanContext spanContext, Duration samplingPeriod) { - this.spanContext = spanContext; - gatherer = - new StackTraceGatherer( - spanContext.getTraceId(), Thread.currentThread(), System.nanoTime()); + ThreadSampler(TraceProfilingContext context, Duration samplingPeriod) { + this.context = context; + gatherer = new StackTraceGatherer(context.traceId, Thread.currentThread(), System.nanoTime()); scheduler = HelpfulExecutors.newSingleThreadedScheduledExecutor( - "stack-trace-sampler-" + spanContext.getTraceId()); + "stack-trace-sampler-" + context.traceId); scheduler.scheduleAtFixedRate( gatherer, SCHEDULER_INITIAL_DELAY, samplingPeriod.toMillis(), TimeUnit.MILLISECONDS); } @@ -125,10 +126,6 @@ void shutdownNow() { boolean awaitTermination(Duration timeout) throws InterruptedException { return scheduler.awaitTermination(timeout.toMillis(), TimeUnit.MILLISECONDS); } - - SpanContext getSpanContext() { - return spanContext; - } } private class StackTraceGatherer implements Runnable { @@ -172,4 +169,30 @@ private Supplier samplerErrorMessage(String traceId, long threadId) { + threadId; } } + + private static class TraceProfilingContext { + static TraceProfilingContext from(SpanContext spanContext) { + return new TraceProfilingContext(spanContext.getTraceId(), spanContext.getSpanId()); + } + + private final String traceId; + private final String spanId; + + TraceProfilingContext(String traceId, String spanId) { + this.traceId = traceId; + this.spanId = spanId; + } + + @Override + public int hashCode() { + return Objects.hash(traceId, spanId); + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) return false; + TraceProfilingContext that = (TraceProfilingContext) o; + return Objects.equals(traceId, that.traceId) && Objects.equals(spanId, that.spanId); + } + } } From 0a5e8902bb016099a50c99cf4e12f9cf9beeedc4 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Wed, 28 May 2025 12:08:03 -0700 Subject: [PATCH 24/35] Rename class. --- .../ScheduledExecutorStackTraceSampler.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java index 8bc0a1cb6..6bec4c187 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java @@ -61,7 +61,7 @@ public void start(SpanContext spanContext) { samplers.computeIfAbsent( spanContext.getTraceId(), - id -> new ThreadSampler(TraceProfilingContext.from(spanContext), samplingPeriod)); + id -> new ThreadSampler(ProfilingSpanContext.from(spanContext), samplingPeriod)); } @Override @@ -69,7 +69,7 @@ public void stop(SpanContext spanContext) { samplers.computeIfPresent( spanContext.getTraceId(), (traceId, sampler) -> { - TraceProfilingContext context = TraceProfilingContext.from(spanContext); + ProfilingSpanContext context = ProfilingSpanContext.from(spanContext); if (context.equals(sampler.context)) { sampler.shutdown(); waitForShutdown(sampler); @@ -101,10 +101,10 @@ private void waitForShutdown(ThreadSampler sampler) { private class ThreadSampler { private final ScheduledExecutorService scheduler; - private final TraceProfilingContext context; + private final ProfilingSpanContext context; private final StackTraceGatherer gatherer; - ThreadSampler(TraceProfilingContext context, Duration samplingPeriod) { + ThreadSampler(ProfilingSpanContext context, Duration samplingPeriod) { this.context = context; gatherer = new StackTraceGatherer(context.traceId, Thread.currentThread(), System.nanoTime()); scheduler = @@ -170,15 +170,15 @@ private Supplier samplerErrorMessage(String traceId, long threadId) { } } - private static class TraceProfilingContext { - static TraceProfilingContext from(SpanContext spanContext) { - return new TraceProfilingContext(spanContext.getTraceId(), spanContext.getSpanId()); + private static class ProfilingSpanContext { + static ProfilingSpanContext from(SpanContext spanContext) { + return new ProfilingSpanContext(spanContext.getTraceId(), spanContext.getSpanId()); } private final String traceId; private final String spanId; - TraceProfilingContext(String traceId, String spanId) { + ProfilingSpanContext(String traceId, String spanId) { this.traceId = traceId; this.spanId = spanId; } @@ -191,7 +191,7 @@ public int hashCode() { @Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; - TraceProfilingContext that = (TraceProfilingContext) o; + ProfilingSpanContext that = (ProfilingSpanContext) o; return Objects.equals(traceId, that.traceId) && Objects.equals(spanId, that.spanId); } } From e80563548da802826df3152bb3451c34d36fb5f5 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Wed, 28 May 2025 12:18:17 -0700 Subject: [PATCH 25/35] Extract inner class to first class citizen. --- .../snapshot/ProfilingSpanContext.java | 41 +++++++++++++ .../ScheduledExecutorStackTraceSampler.java | 31 +--------- .../snapshot/ProfilingSpanContextTest.java | 58 +++++++++++++++++++ 3 files changed, 101 insertions(+), 29 deletions(-) create mode 100644 profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java create mode 100644 profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java new file mode 100644 index 000000000..ac557bc03 --- /dev/null +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java @@ -0,0 +1,41 @@ +package com.splunk.opentelemetry.profiler.snapshot; + +import io.opentelemetry.api.trace.SpanContext; + +import java.util.Objects; + +class ProfilingSpanContext { + static ProfilingSpanContext from(SpanContext spanContext) { + return new ProfilingSpanContext(spanContext.getTraceId(), spanContext.getSpanId()); + } + + private final String traceId; + private final String spanId; + + private ProfilingSpanContext(String traceId, String spanId) { + this.traceId = traceId; + this.spanId = spanId; + } + + public String getTraceId() { + return traceId; + } + + public String getSpanId() { + return spanId; + } + + @Override + public int hashCode() { + return Objects.hash(traceId, spanId); + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + ProfilingSpanContext that = (ProfilingSpanContext) o; + return Objects.equals(traceId, that.traceId) && Objects.equals(spanId, that.spanId); + } +} diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java index 6bec4c187..2bf573689 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java @@ -23,7 +23,6 @@ import java.lang.management.ThreadMXBean; import java.time.Duration; import java.time.Instant; -import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ScheduledExecutorService; @@ -106,10 +105,10 @@ private class ThreadSampler { ThreadSampler(ProfilingSpanContext context, Duration samplingPeriod) { this.context = context; - gatherer = new StackTraceGatherer(context.traceId, Thread.currentThread(), System.nanoTime()); + gatherer = new StackTraceGatherer(context.getTraceId(), Thread.currentThread(), System.nanoTime()); scheduler = HelpfulExecutors.newSingleThreadedScheduledExecutor( - "stack-trace-sampler-" + context.traceId); + "stack-trace-sampler-" + context.getSpanId()); scheduler.scheduleAtFixedRate( gatherer, SCHEDULER_INITIAL_DELAY, samplingPeriod.toMillis(), TimeUnit.MILLISECONDS); } @@ -169,30 +168,4 @@ private Supplier samplerErrorMessage(String traceId, long threadId) { + threadId; } } - - private static class ProfilingSpanContext { - static ProfilingSpanContext from(SpanContext spanContext) { - return new ProfilingSpanContext(spanContext.getTraceId(), spanContext.getSpanId()); - } - - private final String traceId; - private final String spanId; - - ProfilingSpanContext(String traceId, String spanId) { - this.traceId = traceId; - this.spanId = spanId; - } - - @Override - public int hashCode() { - return Objects.hash(traceId, spanId); - } - - @Override - public boolean equals(Object o) { - if (o == null || getClass() != o.getClass()) return false; - ProfilingSpanContext that = (ProfilingSpanContext) o; - return Objects.equals(traceId, that.traceId) && Objects.equals(spanId, that.spanId); - } - } } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java new file mode 100644 index 000000000..c8acc32a2 --- /dev/null +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java @@ -0,0 +1,58 @@ +package com.splunk.opentelemetry.profiler.snapshot; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import org.junit.jupiter.api.Test; + +class ProfilingSpanContextTest { + @Test + void createFromOpenTelemetrySpanContext() { + var spanContext = Snapshotting.spanContext().build(); + var profilingSpanContext = ProfilingSpanContext.from(spanContext); + + assertEquals(spanContext.getTraceId(), profilingSpanContext.getTraceId()); + assertEquals(spanContext.getSpanId(), profilingSpanContext.getSpanId()); + } + + @Test + void equals() { + var spanContext = Snapshotting.spanContext().build(); + + var one = ProfilingSpanContext.from(spanContext); + var two = ProfilingSpanContext.from(spanContext); + + assertThat(one.equals(two)).isTrue(); + assertThat(two.equals(one)).isTrue(); + } + + @Test + void notEquals() { + var one = ProfilingSpanContext.from(Snapshotting.spanContext().build()); + var two = ProfilingSpanContext.from(Snapshotting.spanContext().build()); + + assertThat(one.equals(two)).isFalse(); + assertThat(one.equals(new Object())).isFalse(); + assertThat(two.equals(one)).isFalse(); + assertThat(two.equals(new Object())).isFalse(); + } + + @Test + void hashCodeEquals() { + var spanContext = Snapshotting.spanContext().build(); + + var one = ProfilingSpanContext.from(spanContext); + var two = ProfilingSpanContext.from(spanContext); + + assertEquals(one.hashCode(), two.hashCode()); + } + + @Test + void hasCodeNotEquals() { + var one = ProfilingSpanContext.from(Snapshotting.spanContext().build()); + var two = ProfilingSpanContext.from(Snapshotting.spanContext().build()); + + assertNotEquals(one.hashCode(), two.hashCode()); + } +} From a9bce31329a2d5fede4c37b0e4c7168b8f3c0e07 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Wed, 28 May 2025 12:23:52 -0700 Subject: [PATCH 26/35] Use ProfilingSpanContext reference within ScheduledExecutorStackTraceSampler rather than the component parts where it makes sense. --- .../ScheduledExecutorStackTraceSampler.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java index 2bf573689..af8500cea 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java @@ -105,7 +105,7 @@ private class ThreadSampler { ThreadSampler(ProfilingSpanContext context, Duration samplingPeriod) { this.context = context; - gatherer = new StackTraceGatherer(context.getTraceId(), Thread.currentThread(), System.nanoTime()); + gatherer = new StackTraceGatherer(context, Thread.currentThread(), System.nanoTime()); scheduler = HelpfulExecutors.newSingleThreadedScheduledExecutor( "stack-trace-sampler-" + context.getSpanId()); @@ -128,12 +128,12 @@ boolean awaitTermination(Duration timeout) throws InterruptedException { } private class StackTraceGatherer implements Runnable { - private final String traceId; + private final ProfilingSpanContext context; private final Thread thread; private volatile long timestampNanos; - StackTraceGatherer(String traceId, Thread thread, long timestampNanos) { - this.traceId = traceId; + StackTraceGatherer(ProfilingSpanContext context, Thread thread, long timestampNanos) { + this.context = context; this.thread = thread; this.timestampNanos = timestampNanos; } @@ -147,10 +147,10 @@ public void run() { Duration samplingPeriod = Duration.ofNanos(currentSampleTimestamp - previousTimestampNanos); String spanId = retrieveActiveSpan(thread).getSpanId(); StackTrace stackTrace = - StackTrace.from(Instant.now(), samplingPeriod, threadInfo, traceId, spanId); + StackTrace.from(Instant.now(), samplingPeriod, threadInfo, context.getTraceId(), spanId); stagingArea.get().stage(stackTrace); } catch (Exception e) { - logger.log(Level.SEVERE, e, samplerErrorMessage(traceId, thread.getId())); + logger.log(Level.SEVERE, e, samplerErrorMessage(context, thread.getId())); } finally { timestampNanos = currentSampleTimestamp; } @@ -160,10 +160,10 @@ private SpanContext retrieveActiveSpan(Thread thread) { return spanTracker.get().getActiveSpan(thread).orElse(SpanContext.getInvalid()); } - private Supplier samplerErrorMessage(String traceId, long threadId) { + private Supplier samplerErrorMessage(ProfilingSpanContext context, long threadId) { return () -> "Exception thrown attempting to stage callstacks for trace ID ' " - + traceId + + context.getTraceId() + "' on profiled thread " + threadId; } From c373d110d20453aca0a34f40d1b213837cfa37c2 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Wed, 28 May 2025 13:08:32 -0700 Subject: [PATCH 27/35] Use ProfilingSpanContext in ActiveSpanTracker. --- .../profiler/snapshot/ActiveSpanTracker.java | 15 ++++---- .../ScheduledExecutorStackTraceSampler.java | 4 +-- .../profiler/snapshot/SpanTracker.java | 3 +- .../snapshot/ActiveSpanTrackerTest.java | 36 ++++++++++++------- .../snapshot/InMemorySpanTracker.java | 7 ++-- 5 files changed, 39 insertions(+), 26 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java index 86fd9179e..7f8c199bf 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTracker.java @@ -22,12 +22,13 @@ import io.opentelemetry.context.ContextStorage; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.internal.cache.Cache; +import java.util.Objects; import java.util.Optional; import java.util.function.Supplier; import javax.annotation.Nullable; class ActiveSpanTracker implements ContextStorage, SpanTracker { - private final Cache cache = Cache.weak(); + private final Cache cache = Cache.weak(); private final ContextStorage delegate; private final Supplier registry; @@ -40,14 +41,15 @@ class ActiveSpanTracker implements ContextStorage, SpanTracker { @Override public Scope attach(Context toAttach) { Scope scope = delegate.attach(toAttach); - SpanContext newSpanContext = Span.fromContext(toAttach).getSpanContext(); - if (doNotTrack(newSpanContext)) { + SpanContext spanContext = Span.fromContext(toAttach).getSpanContext(); + if (doNotTrack(spanContext)) { return scope; } Thread thread = Thread.currentThread(); - SpanContext oldSpanContext = cache.get(thread); - if (oldSpanContext == newSpanContext) { + ProfilingSpanContext oldSpanContext = cache.get(thread); + ProfilingSpanContext newSpanContext = ProfilingSpanContext.from(spanContext); + if (Objects.equals(oldSpanContext, newSpanContext)) { return scope; } @@ -72,7 +74,8 @@ public Context current() { return delegate.current(); } - public Optional getActiveSpan(Thread thread) { + @Override + public Optional getActiveSpan(Thread thread) { return Optional.ofNullable(cache.get(thread)); } } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java index af8500cea..9e9e9a2c5 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java @@ -156,8 +156,8 @@ public void run() { } } - private SpanContext retrieveActiveSpan(Thread thread) { - return spanTracker.get().getActiveSpan(thread).orElse(SpanContext.getInvalid()); + private ProfilingSpanContext retrieveActiveSpan(Thread thread) { + return spanTracker.get().getActiveSpan(thread).orElse(ProfilingSpanContext.from(SpanContext.getInvalid())); } private Supplier samplerErrorMessage(ProfilingSpanContext context, long threadId) { diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTracker.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTracker.java index 6c5f370d5..2dcab12f1 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTracker.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SpanTracker.java @@ -16,12 +16,11 @@ package com.splunk.opentelemetry.profiler.snapshot; -import io.opentelemetry.api.trace.SpanContext; import java.util.Optional; interface SpanTracker { SpanTracker NOOP = thread -> Optional.empty(); ConfigurableSupplier SUPPLIER = new ConfigurableSupplier<>(SpanTracker.NOOP); - Optional getActiveSpan(Thread thread); + Optional getActiveSpan(Thread thread); } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java index cd3e8f26c..7a2d65f9f 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java @@ -63,7 +63,8 @@ void trackActiveSpanWhenNewContextAttached() { registry.register(spanContext); try (var ignored = spanTracker.attach(context)) { - assertEquals(Optional.of(spanContext), spanTracker.getActiveSpan(Thread.currentThread())); + var profilingSpanContext = ProfilingSpanContext.from(spanContext); + assertEquals(Optional.of(profilingSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); } } @@ -91,7 +92,8 @@ void trackActiveSpanAcrossMultipleContextChanges() { var spanContext = span.getSpanContext(); context = context.with(span); try (var ignoredScope2 = spanTracker.attach(context)) { - assertEquals(Optional.of(spanContext), spanTracker.getActiveSpan(Thread.currentThread())); + var profilingSpanContext = ProfilingSpanContext.from(spanContext); + assertEquals(Optional.of(profilingSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); } } } @@ -110,7 +112,8 @@ void restoreActiveSpanToPreviousSpanAfterScopeClosing() { scope.close(); var rootSpanContext = root.getSpanContext(); - assertEquals(Optional.of(rootSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); + var profilingSpanContext = ProfilingSpanContext.from(rootSpanContext); + assertEquals(Optional.of(profilingSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); } } @@ -128,10 +131,12 @@ void trackActiveSpanForMultipleTraces() throws Exception { var f2 = executor.submit(attach(span2, releaseLatch)); releaseLatch.countDown(); - try (var scope1 = f1.get(); - var scope2 = f2.get()) { - assertEquals(Optional.of(span1.getSpanContext()), spanTracker.getActiveSpan(scope1.thread)); - assertEquals(Optional.of(span2.getSpanContext()), spanTracker.getActiveSpan(scope2.thread)); + try (var scope1 = f1.get(); var scope2 = f2.get()) { + var profilingSpanContext1 = ProfilingSpanContext.from(span1.getSpanContext()); + assertEquals(Optional.of(profilingSpanContext1), spanTracker.getActiveSpan(scope1.thread)); + + var profilingSpanContext2 = ProfilingSpanContext.from(span2.getSpanContext()); + assertEquals(Optional.of(profilingSpanContext2), spanTracker.getActiveSpan(scope2.thread)); } finally { executor.shutdown(); } @@ -148,8 +153,11 @@ void trackMultipleActiveSpansForSameTraceFromDifferentThreads() throws Exception var executor = Executors.newFixedThreadPool(2); try (var scope1 = executor.submit(attach(span1)).get(); var scope2 = executor.submit(attach(span2)).get()) { - assertEquals(Optional.of(span1.getSpanContext()), spanTracker.getActiveSpan(scope1.thread)); - assertEquals(Optional.of(span2.getSpanContext()), spanTracker.getActiveSpan(scope2.thread)); + var profilingSpanContext1 = ProfilingSpanContext.from(span1.getSpanContext()); + assertEquals(Optional.of(profilingSpanContext1), spanTracker.getActiveSpan(scope1.thread)); + + var profilingSpanContext2 = ProfilingSpanContext.from(span2.getSpanContext()); + assertEquals(Optional.of(profilingSpanContext2), spanTracker.getActiveSpan(scope2.thread)); } finally { executor.shutdown(); } @@ -192,10 +200,12 @@ void activeSpanForThreadIsUnchangedWhenTraceStartsSpanInAnotherThread() throws E registry.register(child.getSpanContext()); var executor = Executors.newSingleThreadExecutor(); - try (var scope1 = attach(root).call(); - var scope2 = executor.submit(attach(child)).get()) { - assertEquals(Optional.of(root.getSpanContext()), spanTracker.getActiveSpan(scope1.thread)); - assertEquals(Optional.of(child.getSpanContext()), spanTracker.getActiveSpan(scope2.thread)); + try (var scope1 = attach(root).call(); var scope2 = executor.submit(attach(child)).get()) { + var rootProfilingSpanContext = ProfilingSpanContext.from(root.getSpanContext()); + assertEquals(Optional.of(rootProfilingSpanContext), spanTracker.getActiveSpan(scope1.thread)); + + var childProfilingSpanContext = ProfilingSpanContext.from(child.getSpanContext()); + assertEquals(Optional.of(childProfilingSpanContext), spanTracker.getActiveSpan(scope2.thread)); } finally { executor.shutdown(); } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InMemorySpanTracker.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InMemorySpanTracker.java index be1a78a95..8c7978251 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InMemorySpanTracker.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/InMemorySpanTracker.java @@ -22,14 +22,15 @@ import java.util.Optional; class InMemorySpanTracker implements SpanTracker { - private final Map activeSpans = new HashMap<>(); + private final Map activeSpans = new HashMap<>(); void store(long threadId, SpanContext spanContext) { - activeSpans.put(threadId, spanContext); + ProfilingSpanContext context = ProfilingSpanContext.from(spanContext); + activeSpans.put(threadId, context); } @Override - public Optional getActiveSpan(Thread thread) { + public Optional getActiveSpan(Thread thread) { return Optional.ofNullable(activeSpans.get(thread.getId())); } } From d462bdc272844caa52a2108f344f5c4a15198b42 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Wed, 28 May 2025 13:16:32 -0700 Subject: [PATCH 28/35] Define an invalid ProfilingSpanContext. --- .../profiler/snapshot/ProfilingSpanContext.java | 2 ++ .../snapshot/ScheduledExecutorStackTraceSampler.java | 2 +- .../profiler/snapshot/ProfilingSpanContextTest.java | 9 +++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java index ac557bc03..b4294131c 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java @@ -5,6 +5,8 @@ import java.util.Objects; class ProfilingSpanContext { + static final ProfilingSpanContext INVALID = from(SpanContext.getInvalid()); + static ProfilingSpanContext from(SpanContext spanContext) { return new ProfilingSpanContext(spanContext.getTraceId(), spanContext.getSpanId()); } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java index 9e9e9a2c5..69c7c803f 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java @@ -157,7 +157,7 @@ public void run() { } private ProfilingSpanContext retrieveActiveSpan(Thread thread) { - return spanTracker.get().getActiveSpan(thread).orElse(ProfilingSpanContext.from(SpanContext.getInvalid())); + return spanTracker.get().getActiveSpan(thread).orElse(ProfilingSpanContext.INVALID); } private Supplier samplerErrorMessage(ProfilingSpanContext context, long threadId) { diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java index c8acc32a2..2f1ef38e9 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java @@ -4,6 +4,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import io.opentelemetry.api.trace.SpanId; +import io.opentelemetry.api.trace.TraceId; import org.junit.jupiter.api.Test; class ProfilingSpanContextTest { @@ -55,4 +57,11 @@ void hasCodeNotEquals() { assertNotEquals(one.hashCode(), two.hashCode()); } + + @Test + void invalidProfilingSpanContext() { + var invalid = ProfilingSpanContext.INVALID; + assertEquals(TraceId.getInvalid(), invalid.getTraceId()); + assertEquals(SpanId.getInvalid(), invalid.getSpanId()); + } } From 204e9abca433d5102a41476fb227a7aba78d23f2 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Wed, 28 May 2025 14:13:30 -0700 Subject: [PATCH 29/35] Add test requiring that stack trace sampling be stopped when an orphaned trace is found. --- ...phanedTraceDetectingTraceRegistryTest.java | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java index 3f74427a2..bee9de69d 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java @@ -18,7 +18,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; -import static org.junit.jupiter.api.Assertions.assertFalse; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.instrumentation.test.utils.GcUtils; @@ -87,7 +86,25 @@ void unregisterOrphanedTraces() throws Exception { spanContext = null; GcUtils.awaitGc(spanContextReference, Duration.ofSeconds(10)); - await().untilAsserted(() -> assertFalse(registry.isRegistered(spanContextCopy))); - await().untilAsserted(() -> assertFalse(delegate.isRegistered(spanContextCopy))); + await().untilAsserted(() -> assertThat(delegate.isRegistered(spanContextCopy)).isFalse()); + await().untilAsserted(() -> assertThat(registry.isRegistered(spanContextCopy)).isFalse()); + } + + @Test + void stopSamplingForOrphanedTraces() throws Exception { + var spanContext = Snapshotting.spanContext().build(); + registry.register(spanContext); + var spanContextCopy = + SpanContext.create( + spanContext.getTraceId(), + spanContext.getSpanId(), + spanContext.getTraceFlags(), + spanContext.getTraceState()); + + var spanContextReference = new WeakReference<>(spanContext); + spanContext = null; + GcUtils.awaitGc(spanContextReference, Duration.ofSeconds(10)); + + await().untilAsserted(() -> assertThat(sampler.isBeingSampled(spanContextCopy)).isFalse()); } } From c8fac10f29912abb4620470d43843bf66940288a Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Thu, 29 May 2025 13:53:19 -0700 Subject: [PATCH 30/35] Apply spotless code formatting. --- .../snapshot/ProfilingSpanContext.java | 17 ++++++++++++++++- .../ScheduledExecutorStackTraceSampler.java | 3 ++- .../snapshot/ActiveSpanTrackerTest.java | 18 ++++++++++++------ .../snapshot/ProfilingSpanContextTest.java | 16 ++++++++++++++++ 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java index b4294131c..a63412504 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContext.java @@ -1,7 +1,22 @@ +/* + * Copyright Splunk Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.splunk.opentelemetry.profiler.snapshot; import io.opentelemetry.api.trace.SpanContext; - import java.util.Objects; class ProfilingSpanContext { diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java index 69c7c803f..e8b8071be 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/ScheduledExecutorStackTraceSampler.java @@ -147,7 +147,8 @@ public void run() { Duration samplingPeriod = Duration.ofNanos(currentSampleTimestamp - previousTimestampNanos); String spanId = retrieveActiveSpan(thread).getSpanId(); StackTrace stackTrace = - StackTrace.from(Instant.now(), samplingPeriod, threadInfo, context.getTraceId(), spanId); + StackTrace.from( + Instant.now(), samplingPeriod, threadInfo, context.getTraceId(), spanId); stagingArea.get().stage(stackTrace); } catch (Exception e) { logger.log(Level.SEVERE, e, samplerErrorMessage(context, thread.getId())); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java index 7a2d65f9f..ffe0bc72b 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ActiveSpanTrackerTest.java @@ -64,7 +64,8 @@ void trackActiveSpanWhenNewContextAttached() { try (var ignored = spanTracker.attach(context)) { var profilingSpanContext = ProfilingSpanContext.from(spanContext); - assertEquals(Optional.of(profilingSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); + assertEquals( + Optional.of(profilingSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); } } @@ -93,7 +94,8 @@ void trackActiveSpanAcrossMultipleContextChanges() { context = context.with(span); try (var ignoredScope2 = spanTracker.attach(context)) { var profilingSpanContext = ProfilingSpanContext.from(spanContext); - assertEquals(Optional.of(profilingSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); + assertEquals( + Optional.of(profilingSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); } } } @@ -113,7 +115,8 @@ void restoreActiveSpanToPreviousSpanAfterScopeClosing() { var rootSpanContext = root.getSpanContext(); var profilingSpanContext = ProfilingSpanContext.from(rootSpanContext); - assertEquals(Optional.of(profilingSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); + assertEquals( + Optional.of(profilingSpanContext), spanTracker.getActiveSpan(Thread.currentThread())); } } @@ -131,7 +134,8 @@ void trackActiveSpanForMultipleTraces() throws Exception { var f2 = executor.submit(attach(span2, releaseLatch)); releaseLatch.countDown(); - try (var scope1 = f1.get(); var scope2 = f2.get()) { + try (var scope1 = f1.get(); + var scope2 = f2.get()) { var profilingSpanContext1 = ProfilingSpanContext.from(span1.getSpanContext()); assertEquals(Optional.of(profilingSpanContext1), spanTracker.getActiveSpan(scope1.thread)); @@ -200,12 +204,14 @@ void activeSpanForThreadIsUnchangedWhenTraceStartsSpanInAnotherThread() throws E registry.register(child.getSpanContext()); var executor = Executors.newSingleThreadExecutor(); - try (var scope1 = attach(root).call(); var scope2 = executor.submit(attach(child)).get()) { + try (var scope1 = attach(root).call(); + var scope2 = executor.submit(attach(child)).get()) { var rootProfilingSpanContext = ProfilingSpanContext.from(root.getSpanContext()); assertEquals(Optional.of(rootProfilingSpanContext), spanTracker.getActiveSpan(scope1.thread)); var childProfilingSpanContext = ProfilingSpanContext.from(child.getSpanContext()); - assertEquals(Optional.of(childProfilingSpanContext), spanTracker.getActiveSpan(scope2.thread)); + assertEquals( + Optional.of(childProfilingSpanContext), spanTracker.getActiveSpan(scope2.thread)); } finally { executor.shutdown(); } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java index 2f1ef38e9..f9fce7acb 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/ProfilingSpanContextTest.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.splunk.opentelemetry.profiler.snapshot; import static org.assertj.core.api.Assertions.assertThat; From 9b229d790fbec3512c17f9293451095113865d9e Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Thu, 29 May 2025 14:54:25 -0700 Subject: [PATCH 31/35] Update javadoc comment. --- .../profiler/snapshot/SnapshotProfilingSpanProcessor.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java index 4661d9cc7..0026f2d2d 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSpanProcessor.java @@ -68,9 +68,12 @@ public boolean isStartRequired() { /** * Relying solely on the OpenTelemetry instrumentation to correctly notify this SpanProcessor when * a span has ended opens up the possibility of a memory leak in the event a bug is encountered - * within the instrumentation layer that prevents a span from being ended. + * within the instrumentation layer that prevents a span from being ended. To protect against this + * {@link OrphanedTraceDetectingTraceRegistry}, a specialized {@link TraceRegistry}, is installed + * to search for garbage collected {@link io.opentelemetry.api.trace.SpanContext} instances and + * automatically unregister traces and stop sampling. * - *

Will follow up with a more robust solution to this potential problem. + * @see OrphanedTraceDetectingTraceRegistry */ @Override public void onEnd(ReadableSpan span) { From 0cc5af7b081e45ba89d74f182723e98ddd885a68 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Fri, 30 May 2025 10:51:29 -0700 Subject: [PATCH 32/35] Avoid creating weak references to a SpanContext when a trace is being unregistered. --- .../OrphanedTraceDetectingTraceRegistry.java | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java index 3243e9f31..3099270cc 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java @@ -26,7 +26,7 @@ import java.util.function.Supplier; class OrphanedTraceDetectingTraceRegistry implements TraceRegistry { - private final Set traces = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final Set traces = Collections.newSetFromMap(new ConcurrentHashMap<>()); private final ReferenceQueue referenceQueue = new ReferenceQueue<>(); private final TraceRegistry delegate; @@ -57,7 +57,7 @@ public boolean isRegistered(SpanContext spanContext) { @Override public void unregister(SpanContext spanContext) { delegate.unregister(spanContext); - traces.remove(new WeakSpanContext(spanContext, referenceQueue)); + traces.remove(new LookupKey(spanContext)); } public void unregisterOrphanedTraces() { @@ -65,10 +65,10 @@ public void unregisterOrphanedTraces() { try { Object reference = referenceQueue.remove(); if (reference != null) { - WeakSpanContext weakContext = (WeakSpanContext) reference; - traces.remove(weakContext); - delegate.unregister(weakContext.spanContext); - sampler.get().stop(weakContext.spanContext); + Key key = (Key) reference; + traces.remove(key); + delegate.unregister(key.getSpanContext()); + sampler.get().stop(key.getSpanContext()); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); @@ -82,7 +82,24 @@ public void close() { thread.interrupt(); } - private static class WeakSpanContext extends WeakReference { + interface Key { + SpanContext getSpanContext(); + } + + private static class LookupKey implements Key { + private final SpanContext spanContext; + + private LookupKey(SpanContext spanContext) { + this.spanContext = spanContext; + } + + @Override + public SpanContext getSpanContext() { + return spanContext; + } + } + + private static class WeakSpanContext extends WeakReference implements Key { private final SpanContext spanContext; public WeakSpanContext(SpanContext referent, ReferenceQueue q) { @@ -95,6 +112,11 @@ public WeakSpanContext(SpanContext referent, ReferenceQueue q) { referent.getTraceState()); } + @Override + public SpanContext getSpanContext() { + return spanContext; + } + @Override public int hashCode() { return Objects.hashCode(spanContext); From 8d4b2ada60f57c2d46c694349a6f097c86be05d4 Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Fri, 6 Jun 2025 11:24:28 -0700 Subject: [PATCH 33/35] Add missing equals method to correctly remove entries from the set in OrphanedTraceDetectingTraceRegistry. --- .../OrphanedTraceDetectingTraceRegistry.java | 48 +++++++++++++++---- ...phanedTraceDetectingTraceRegistryTest.java | 28 +++++++++-- 2 files changed, 63 insertions(+), 13 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java index 3099270cc..75a461d43 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java @@ -16,7 +16,9 @@ package com.splunk.opentelemetry.profiler.snapshot; +import com.google.common.annotations.VisibleForTesting; import io.opentelemetry.api.trace.SpanContext; + import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; import java.util.Collections; @@ -60,6 +62,14 @@ public void unregister(SpanContext spanContext) { traces.remove(new LookupKey(spanContext)); } + /** + * Test-specific method to inspect the weak references. DO NOT USE IN PRODUCTION CODE! + */ + @VisibleForTesting + boolean isRegisteredInternal(SpanContext spanContext) { + return traces.contains(new LookupKey(spanContext)); + } + public void unregisterOrphanedTraces() { while (!Thread.interrupted()) { try { @@ -82,14 +92,14 @@ public void close() { thread.interrupt(); } - interface Key { + public interface Key { SpanContext getSpanContext(); } - private static class LookupKey implements Key { + public static class LookupKey implements Key { private final SpanContext spanContext; - private LookupKey(SpanContext spanContext) { + public LookupKey(SpanContext spanContext) { this.spanContext = spanContext; } @@ -97,9 +107,26 @@ private LookupKey(SpanContext spanContext) { public SpanContext getSpanContext() { return spanContext; } + + @Override + public int hashCode() { + return spanContext.hashCode(); + } + + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (!(o instanceof Key)) { + return false; + } + Key other = (Key) o; + return Objects.equals(spanContext, other.getSpanContext()); + } } - private static class WeakSpanContext extends WeakReference implements Key { + public static class WeakSpanContext extends WeakReference implements Key { private final SpanContext spanContext; public WeakSpanContext(SpanContext referent, ReferenceQueue q) { @@ -119,14 +146,19 @@ public SpanContext getSpanContext() { @Override public int hashCode() { - return Objects.hashCode(spanContext); + return spanContext.hashCode(); } @Override public boolean equals(Object o) { - if (o == null || getClass() != o.getClass()) return false; - WeakSpanContext that = (WeakSpanContext) o; - return Objects.equals(spanContext, that.spanContext); + if (o == this) { + return true; + } + if (!(o instanceof Key)) { + return false; + } + Key other = (Key) o; + return Objects.equals(spanContext, other.getSpanContext()); } } } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java index bee9de69d..55bf83b9e 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java @@ -16,16 +16,17 @@ package com.splunk.opentelemetry.profiler.snapshot; -import static org.assertj.core.api.Assertions.assertThat; -import static org.awaitility.Awaitility.await; - import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.instrumentation.test.utils.GcUtils; -import java.lang.ref.WeakReference; -import java.time.Duration; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import java.lang.ref.WeakReference; +import java.time.Duration; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + class OrphanedTraceDetectingTraceRegistryTest { private final TraceRegistry delegate = new SimpleTraceRegistry(); private final ObservableStackTraceSampler sampler = new ObservableStackTraceSampler(); @@ -37,6 +38,13 @@ void teardown() { registry.close(); } + @Test + void trackRegisteredSpanContext() { + var spanContext = Snapshotting.spanContext().build(); + registry.register(spanContext); + assertThat(registry.isRegisteredInternal(spanContext)).isTrue(); + } + @Test void delegateTraceRegistration() { var spanContext = Snapshotting.spanContext().build(); @@ -44,6 +52,16 @@ void delegateTraceRegistration() { assertThat(delegate.isRegistered(spanContext)).isTrue(); } + @Test + void stopTrackingUnregisteredSpanContext() { + var spanContext = Snapshotting.spanContext().build(); + + registry.register(spanContext); + registry.unregister(spanContext); + + assertThat(registry.isRegisteredInternal(spanContext)).isFalse(); + } + @Test void delegateTraceDeregistration() { var spanContext = Snapshotting.spanContext().build(); From 87db84adb2afb5ba671efa006b6bb7681ab3761b Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Fri, 6 Jun 2025 11:44:19 -0700 Subject: [PATCH 34/35] Verify that sampling starts and is later stopped when an orphaned span context is found. --- .../snapshot/OrphanedTraceDetectingTraceRegistryTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java index 55bf83b9e..4805795c9 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java @@ -112,12 +112,14 @@ void unregisterOrphanedTraces() throws Exception { void stopSamplingForOrphanedTraces() throws Exception { var spanContext = Snapshotting.spanContext().build(); registry.register(spanContext); + sampler.start(spanContext); var spanContextCopy = SpanContext.create( spanContext.getTraceId(), spanContext.getSpanId(), spanContext.getTraceFlags(), spanContext.getTraceState()); + await().untilAsserted(() -> assertThat(sampler.isBeingSampled(spanContextCopy)).isTrue()); var spanContextReference = new WeakReference<>(spanContext); spanContext = null; From 77679ffb223fe0cc4d7bda370e7e5236354fcd6f Mon Sep 17 00:00:00 2001 From: thomasduncan Date: Fri, 6 Jun 2025 11:52:05 -0700 Subject: [PATCH 35/35] Apply spotless code formatting. --- .../snapshot/OrphanedTraceDetectingTraceRegistry.java | 9 +++------ .../OrphanedTraceDetectingTraceRegistryTest.java | 11 +++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java index 75a461d43..90ce61fe7 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistry.java @@ -18,7 +18,6 @@ import com.google.common.annotations.VisibleForTesting; import io.opentelemetry.api.trace.SpanContext; - import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; import java.util.Collections; @@ -62,9 +61,7 @@ public void unregister(SpanContext spanContext) { traces.remove(new LookupKey(spanContext)); } - /** - * Test-specific method to inspect the weak references. DO NOT USE IN PRODUCTION CODE! - */ + /** Test-specific method to inspect the weak references. DO NOT USE IN PRODUCTION CODE! */ @VisibleForTesting boolean isRegisteredInternal(SpanContext spanContext) { return traces.contains(new LookupKey(spanContext)); @@ -122,7 +119,7 @@ public boolean equals(Object o) { return false; } Key other = (Key) o; - return Objects.equals(spanContext, other.getSpanContext()); + return Objects.equals(spanContext, other.getSpanContext()); } } @@ -158,7 +155,7 @@ public boolean equals(Object o) { return false; } Key other = (Key) o; - return Objects.equals(spanContext, other.getSpanContext()); + return Objects.equals(spanContext, other.getSpanContext()); } } } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java index 4805795c9..0f63e42b9 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/snapshot/OrphanedTraceDetectingTraceRegistryTest.java @@ -16,16 +16,15 @@ package com.splunk.opentelemetry.profiler.snapshot; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.instrumentation.test.utils.GcUtils; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Test; - import java.lang.ref.WeakReference; import java.time.Duration; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.awaitility.Awaitility.await; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; class OrphanedTraceDetectingTraceRegistryTest { private final TraceRegistry delegate = new SimpleTraceRegistry();