From 2539208a0c6a0a0966de3700b54d4e1f386e9869 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 10:54:53 +0200 Subject: [PATCH 01/12] refactor to use ExtendedSpanProcessor.onEnding --- .../stacktrace/StackTraceSpanProcessor.java | 44 ++--- .../AbstractSimpleChainingSpanProcessor.java | 139 -------------- .../stacktrace/internal/MutableSpan.java | 153 ---------------- .../stacktrace/internal/MutableSpanData.java | 94 ---------- .../StackTraceSpanProcessorTest.java | 16 +- ...stractSimpleChainingSpanProcessorTest.java | 109 ----------- .../stacktrace/internal/MutableSpanTest.java | 173 ------------------ .../stacktrace/internal/TestUtils.java | 21 --- 8 files changed, 35 insertions(+), 714 deletions(-) delete mode 100644 span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/AbstractSimpleChainingSpanProcessor.java delete mode 100644 span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpan.java delete mode 100644 span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpanData.java delete mode 100644 span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/AbstractSimpleChainingSpanProcessorTest.java delete mode 100644 span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpanTest.java delete mode 100644 span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/TestUtils.java diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java index 1714cc917..9e5dc67db 100644 --- a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java +++ b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java @@ -6,11 +6,11 @@ package io.opentelemetry.contrib.stacktrace; import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.contrib.stacktrace.internal.AbstractSimpleChainingSpanProcessor; -import io.opentelemetry.contrib.stacktrace.internal.MutableSpan; +import io.opentelemetry.context.Context; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.trace.ReadWriteSpan; import io.opentelemetry.sdk.trace.ReadableSpan; -import io.opentelemetry.sdk.trace.SpanProcessor; +import io.opentelemetry.sdk.trace.internal.ExtendedSpanProcessor; import java.io.PrintWriter; import java.io.StringWriter; import java.time.Duration; @@ -18,7 +18,7 @@ import java.util.logging.Level; import java.util.logging.Logger; -public class StackTraceSpanProcessor extends AbstractSimpleChainingSpanProcessor { +public class StackTraceSpanProcessor implements ExtendedSpanProcessor { private static final String CONFIG_MIN_DURATION = "otel.java.experimental.span-stacktrace.min.duration"; @@ -35,13 +35,11 @@ public class StackTraceSpanProcessor extends AbstractSimpleChainingSpanProcessor private final Predicate filterPredicate; /** - * @param next next span processor to invoke * @param minSpanDurationNanos minimum span duration in ns for stacktrace capture * @param filterPredicate extra filter function to exclude spans if needed */ public StackTraceSpanProcessor( - SpanProcessor next, long minSpanDurationNanos, Predicate filterPredicate) { - super(next); + long minSpanDurationNanos, Predicate filterPredicate) { this.minSpanDurationNanos = minSpanDurationNanos; this.filterPredicate = filterPredicate; if (minSpanDurationNanos < 0) { @@ -55,47 +53,51 @@ public StackTraceSpanProcessor( } /** - * @param next next span processor to invoke * @param config configuration * @param filterPredicate extra filter function to exclude spans if needed */ - public StackTraceSpanProcessor( - SpanProcessor next, ConfigProperties config, Predicate filterPredicate) { + public StackTraceSpanProcessor(ConfigProperties config, Predicate filterPredicate) { this( - next, config.getDuration(CONFIG_MIN_DURATION, CONFIG_MIN_DURATION_DEFAULT).toNanos(), filterPredicate); } @Override - protected boolean requiresStart() { + public boolean isStartRequired() { return false; } @Override - protected boolean requiresEnd() { + public void onStart(Context context, ReadWriteSpan readWriteSpan) {} + + @Override + public boolean isOnEndingRequired() { return true; } @Override - protected ReadableSpan doOnEnd(ReadableSpan span) { + public void onEnding(ReadWriteSpan span) { if (minSpanDurationNanos < 0 || span.getLatencyNanos() < minSpanDurationNanos) { - return span; + return; } if (span.getAttribute(SPAN_STACKTRACE) != null) { // Span already has a stacktrace, do not override - return span; + return; } if (!filterPredicate.test(span)) { - return span; + return; } - MutableSpan mutableSpan = MutableSpan.makeMutable(span); + span.setAttribute(SPAN_STACKTRACE, generateSpanEndStacktrace()); + } - String stacktrace = generateSpanEndStacktrace(); - mutableSpan.setAttribute(SPAN_STACKTRACE, stacktrace); - return mutableSpan; + @Override + public boolean isEndRequired() { + return false; } + @Override + public void onEnd(ReadableSpan readableSpan) {} + private static String generateSpanEndStacktrace() { Throwable exception = new Throwable(); StringWriter stringWriter = new StringWriter(); diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/AbstractSimpleChainingSpanProcessor.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/AbstractSimpleChainingSpanProcessor.java deleted file mode 100644 index c0b4b5e72..000000000 --- a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/AbstractSimpleChainingSpanProcessor.java +++ /dev/null @@ -1,139 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.stacktrace.internal; - -import com.google.errorprone.annotations.CanIgnoreReturnValue; -import io.opentelemetry.context.Context; -import io.opentelemetry.sdk.common.CompletableResultCode; -import io.opentelemetry.sdk.trace.ReadWriteSpan; -import io.opentelemetry.sdk.trace.ReadableSpan; -import io.opentelemetry.sdk.trace.SpanProcessor; -import java.util.Arrays; - -/** - * A @{@link SpanProcessor} which in addition to all standard operations is capable of modifying and - * optionally filtering spans in the end-callback. - * - *

This is done by chaining processors and registering only the first processor with the SDK. - * Mutations can be performed in {@link #doOnEnd(ReadableSpan)} by wrapping the span in a {@link - * MutableSpan} - */ -public abstract class AbstractSimpleChainingSpanProcessor implements SpanProcessor { - - protected final SpanProcessor next; - private final boolean nextRequiresStart; - private final boolean nextRequiresEnd; - - /** - * @param next the next processor to be invoked after the one being constructed. - */ - public AbstractSimpleChainingSpanProcessor(SpanProcessor next) { - this.next = next; - nextRequiresStart = next.isStartRequired(); - nextRequiresEnd = next.isEndRequired(); - } - - /** - * Equivalent of {@link SpanProcessor#onStart(Context, ReadWriteSpan)}. The onStart callback of - * the next processor must not be invoked from this method, this is already handled by the - * implementation of {@link #onStart(Context, ReadWriteSpan)}. - */ - protected void doOnStart(Context context, ReadWriteSpan readWriteSpan) {} - - /** - * Equivalent of {@link SpanProcessor#onEnd(ReadableSpan)}}. - * - *

If this method returns null, the provided span will be dropped and not passed to the next - * processor. If anything non-null is returned, the returned instance is passed to the next - * processor. - * - *

So in order to mutate the span, simply use {@link MutableSpan#makeMutable(ReadableSpan)} on - * the provided argument and return the {@link MutableSpan} from this method. - */ - @CanIgnoreReturnValue - protected ReadableSpan doOnEnd(ReadableSpan readableSpan) { - return readableSpan; - } - - /** - * Indicates if span processor needs to be called on span start - * - * @return true, if this implementation would like {@link #doOnStart(Context, ReadWriteSpan)} to - * be invoked. - */ - protected boolean requiresStart() { - return true; - } - - /** - * Indicates if span processor needs to be called on span end - * - * @return true, if this implementation would like {@link #doOnEnd(ReadableSpan)} to be invoked. - */ - protected boolean requiresEnd() { - return true; - } - - protected CompletableResultCode doForceFlush() { - return CompletableResultCode.ofSuccess(); - } - - protected CompletableResultCode doShutdown() { - return CompletableResultCode.ofSuccess(); - } - - @Override - public final void onStart(Context context, ReadWriteSpan readWriteSpan) { - try { - if (requiresStart()) { - doOnStart(context, readWriteSpan); - } - } finally { - if (nextRequiresStart) { - next.onStart(context, readWriteSpan); - } - } - } - - @Override - public final void onEnd(ReadableSpan readableSpan) { - ReadableSpan mappedTo = readableSpan; - try { - if (requiresEnd()) { - mappedTo = doOnEnd(readableSpan); - } - } finally { - if (mappedTo != null && nextRequiresEnd) { - next.onEnd(mappedTo); - } - } - } - - @Override - public final boolean isStartRequired() { - return requiresStart() || nextRequiresStart; - } - - @Override - public final boolean isEndRequired() { - return requiresEnd() || nextRequiresEnd; - } - - @Override - public final CompletableResultCode shutdown() { - return CompletableResultCode.ofAll(Arrays.asList(doShutdown(), next.shutdown())); - } - - @Override - public final CompletableResultCode forceFlush() { - return CompletableResultCode.ofAll(Arrays.asList(doForceFlush(), next.forceFlush())); - } - - @Override - public final void close() { - SpanProcessor.super.close(); - } -} diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpan.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpan.java deleted file mode 100644 index 781281605..000000000 --- a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpan.java +++ /dev/null @@ -1,153 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.stacktrace.internal; - -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.trace.SpanContext; -import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.sdk.common.InstrumentationScopeInfo; -import io.opentelemetry.sdk.trace.ReadableSpan; -import io.opentelemetry.sdk.trace.data.SpanData; -import javax.annotation.Nullable; - -/** - * A wrapper around an ended {@link ReadableSpan}, which allows mutation. This is done by wrapping - * the {@link SpanData} of the provided span and returning a mutated wrapper when {@link - * #toSpanData()} is called. - * - *

This class is not thread-safe.Note that after {@link #toSpanData()} has been called, no more - * mutation are allowed. This guarantees that the returned SpanData is safe to use across threads. - */ -public class MutableSpan implements ReadableSpan { - - private final ReadableSpan delegate; - - @Nullable private MutableSpanData mutableSpanData = null; - @Nullable private SpanData cachedDelegateSpanData = null; - - private boolean frozen; - - private MutableSpan(ReadableSpan delegate) { - if (!delegate.hasEnded()) { - throw new IllegalArgumentException("The provided span has not ended yet!"); - } - this.delegate = delegate; - } - - /** - * If the provided span is already mutable, it is casted and returned. Otherwise, it is wrapped in - * a new MutableSpan instance and returned. - * - * @param span the span to make mutable - */ - public static MutableSpan makeMutable(ReadableSpan span) { - if (span instanceof MutableSpan && !((MutableSpan) span).frozen) { - return (MutableSpan) span; - } else { - return new MutableSpan(span); - } - } - - public ReadableSpan getOriginalSpan() { - return delegate; - } - - private SpanData getDelegateSpanData() { - if (cachedDelegateSpanData == null) { - cachedDelegateSpanData = delegate.toSpanData(); - } - return cachedDelegateSpanData; - } - - @Override - public SpanData toSpanData() { - frozen = true; - if (mutableSpanData != null) { - return mutableSpanData; - } - return getDelegateSpanData(); - } - - private MutableSpanData mutate() { - if (frozen) { - throw new IllegalStateException( - "toSpanData() has already been called on this span, it is no longer mutable!"); - } - if (mutableSpanData == null) { - mutableSpanData = new MutableSpanData(getDelegateSpanData()); - } - return mutableSpanData; - } - - @Nullable - @Override - public T getAttribute(AttributeKey key) { - if (mutableSpanData != null) { - return mutableSpanData.getAttribute(key); - } else { - return delegate.getAttribute(key); - } - } - - public void removeAttribute(AttributeKey key) { - mutate().setAttribute(key, null); - } - - public void setAttribute(AttributeKey key, @Nullable T value) { - mutate().setAttribute(key, value); - } - - @Override - public String getName() { - if (mutableSpanData != null) { - return mutableSpanData.getName(); - } - return delegate.getName(); - } - - public void setName(String name) { - if (name == null) { - throw new IllegalArgumentException("name must not be null"); - } - mutate().setName(name); - } - - @Override - public SpanContext getSpanContext() { - return delegate.getSpanContext(); - } - - @Override - public SpanContext getParentSpanContext() { - return delegate.getParentSpanContext(); - } - - @Override - @Deprecated - public io.opentelemetry.sdk.common.InstrumentationLibraryInfo getInstrumentationLibraryInfo() { - return delegate.getInstrumentationLibraryInfo(); - } - - @Override - public InstrumentationScopeInfo getInstrumentationScopeInfo() { - return delegate.getInstrumentationScopeInfo(); - } - - @Override - public boolean hasEnded() { - return delegate.hasEnded(); - } - - @Override - public long getLatencyNanos() { - return delegate.getLatencyNanos(); - } - - @Override - public SpanKind getKind() { - return delegate.getKind(); - } -} diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpanData.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpanData.java deleted file mode 100644 index debfae325..000000000 --- a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpanData.java +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.stacktrace.internal; - -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.common.AttributesBuilder; -import io.opentelemetry.sdk.trace.data.DelegatingSpanData; -import io.opentelemetry.sdk.trace.data.SpanData; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; -import javax.annotation.Nullable; - -public class MutableSpanData extends DelegatingSpanData { - - @Nullable private Map, Object> attributeOverrides = null; - - @Nullable private Attributes cachedMutatedAttributes = null; - - @Nullable private String nameOverride = null; - - protected MutableSpanData(SpanData delegate) { - super(delegate); - } - - public void setAttribute(AttributeKey key, @Nullable T value) { - if (attributeOverrides != null - && attributeOverrides.containsKey(key) - && Objects.equals(attributeOverrides.get(key), value)) { - return; - } - T originalValue = super.getAttributes().get(key); - if (Objects.equals(originalValue, value)) { - if (attributeOverrides != null) { - cachedMutatedAttributes = null; - attributeOverrides.remove(key); - } - return; - } - if (attributeOverrides == null) { - attributeOverrides = new HashMap<>(); - } - cachedMutatedAttributes = null; - attributeOverrides.put(key, value); - } - - @Override - @SuppressWarnings({"rawtypes", "unchecked"}) - public Attributes getAttributes() { - - Attributes original = super.getAttributes(); - if (attributeOverrides == null || attributeOverrides.isEmpty()) { - return original; - } - if (cachedMutatedAttributes == null) { - AttributesBuilder attributesBuilder = Attributes.builder().putAll(original); - for (AttributeKey overrideKey : attributeOverrides.keySet()) { - Object value = attributeOverrides.get(overrideKey); - if (value == null) { - attributesBuilder.remove(overrideKey); - } else { - attributesBuilder.put(overrideKey, value); - } - } - cachedMutatedAttributes = attributesBuilder.build(); - } - return cachedMutatedAttributes; - } - - @SuppressWarnings("unchecked") - @Nullable - public T getAttribute(AttributeKey key) { - if (attributeOverrides != null && attributeOverrides.containsKey(key)) { - return (T) attributeOverrides.get(key); - } - return super.getAttributes().get(key); - } - - public void setName(String name) { - nameOverride = name; - } - - @Override - public String getName() { - if (nameOverride != null) { - return nameOverride; - } - return super.getName(); - } -} diff --git a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java index 0ddffec9e..f8b6cbe4a 100644 --- a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java +++ b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java @@ -11,11 +11,11 @@ import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Scope; -import io.opentelemetry.contrib.stacktrace.internal.TestUtils; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter; import io.opentelemetry.sdk.trace.ReadableSpan; +import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.sdk.trace.SpanProcessor; import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor; @@ -111,11 +111,19 @@ private static void checkSpan( configMap.put("otel.java.experimental.span-stacktrace.min.duration", configString); } - try (SpanProcessor processor = + StackTraceSpanProcessor processor = new StackTraceSpanProcessor( - exportProcessor, DefaultConfigProperties.createFromMap(configMap), filterPredicate)) { + DefaultConfigProperties.createFromMap(configMap), filterPredicate); + + try (OpenTelemetrySdk sdk = + OpenTelemetrySdk.builder() + .setTracerProvider( + SdkTracerProvider.builder() + .addSpanProcessor(processor) + .addSpanProcessor(exportProcessor) + .build()) + .build()) { - OpenTelemetrySdk sdk = TestUtils.sdkWith(processor); Tracer tracer = sdk.getTracer("test"); Instant start = Instant.now(); diff --git a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/AbstractSimpleChainingSpanProcessorTest.java b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/AbstractSimpleChainingSpanProcessorTest.java deleted file mode 100644 index b82857a4f..000000000 --- a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/AbstractSimpleChainingSpanProcessorTest.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.stacktrace.internal; - -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; - -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.trace.Tracer; -import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter; -import io.opentelemetry.sdk.trace.ReadableSpan; -import io.opentelemetry.sdk.trace.SpanProcessor; -import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -public class AbstractSimpleChainingSpanProcessorTest { - - private InMemorySpanExporter spans; - private SpanProcessor exportProcessor; - - @BeforeEach - public void setup() { - spans = InMemorySpanExporter.create(); - exportProcessor = SimpleSpanProcessor.create(spans); - } - - @Test - public void testSpanDropping() { - SpanProcessor processor = - new AbstractSimpleChainingSpanProcessor(exportProcessor) { - @Override - protected ReadableSpan doOnEnd(ReadableSpan readableSpan) { - if (readableSpan.getName().startsWith("dropMe")) { - return null; - } else { - return readableSpan; - } - } - }; - try (OpenTelemetrySdk sdk = TestUtils.sdkWith(processor)) { - Tracer tracer = sdk.getTracer("dummy-tracer"); - - tracer.spanBuilder("dropMe1").startSpan().end(); - tracer.spanBuilder("sendMe").startSpan().end(); - tracer.spanBuilder("dropMe2").startSpan().end(); - - assertThat(spans.getFinishedSpanItems()) - .hasSize(1) - .anySatisfy(span -> assertThat(span).hasName("sendMe")); - } - } - - @Test - public void testAttributeUpdate() { - - AttributeKey keepMeKey = AttributeKey.stringKey("keepMe"); - AttributeKey updateMeKey = AttributeKey.stringKey("updateMe"); - AttributeKey addMeKey = AttributeKey.stringKey("addMe"); - AttributeKey removeMeKey = AttributeKey.stringKey("removeMe"); - - SpanProcessor second = - new AbstractSimpleChainingSpanProcessor(exportProcessor) { - @Override - protected ReadableSpan doOnEnd(ReadableSpan readableSpan) { - MutableSpan span = MutableSpan.makeMutable(readableSpan); - span.setAttribute(addMeKey, "added"); - return span; - } - }; - SpanProcessor first = - new AbstractSimpleChainingSpanProcessor(second) { - @Override - protected ReadableSpan doOnEnd(ReadableSpan readableSpan) { - MutableSpan span = MutableSpan.makeMutable(readableSpan); - span.setAttribute(updateMeKey, "updated"); - span.removeAttribute(removeMeKey); - return span; - } - }; - try (OpenTelemetrySdk sdk = TestUtils.sdkWith(first)) { - Tracer tracer = sdk.getTracer("dummy-tracer"); - - tracer - .spanBuilder("dropMe1") - .startSpan() - .setAttribute(keepMeKey, "keep-me-original") - .setAttribute(removeMeKey, "remove-me-original") - .setAttribute(updateMeKey, "foo") - .end(); - - assertThat(spans.getFinishedSpanItems()) - .hasSize(1) - .anySatisfy( - span -> { - Attributes attribs = span.getAttributes(); - assertThat(attribs) - .hasSize(3) - .containsEntry(keepMeKey, "keep-me-original") - .containsEntry(updateMeKey, "updated") - .containsEntry(addMeKey, "added"); - }); - } - } -} diff --git a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpanTest.java b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpanTest.java deleted file mode 100644 index 6c81baf62..000000000 --- a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/MutableSpanTest.java +++ /dev/null @@ -1,173 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.stacktrace.internal; - -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.trace.SpanBuilder; -import io.opentelemetry.context.Context; -import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.trace.ReadWriteSpan; -import io.opentelemetry.sdk.trace.ReadableSpan; -import io.opentelemetry.sdk.trace.SpanProcessor; -import io.opentelemetry.sdk.trace.data.SpanData; -import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Consumer; -import org.junit.jupiter.api.Test; - -public class MutableSpanTest { - - @Test - public void noSpanDataCopyWithoutMutation() { - ReadableSpan original = createSpan("foo", builder -> {}); - - MutableSpan mutable = MutableSpan.makeMutable(original); - SpanData first = mutable.toSpanData(); - SpanData second = mutable.toSpanData(); - - assertThat(first.getClass().getName()) - .isEqualTo("io.opentelemetry.sdk.trace.AutoValue_SpanWrapper"); - assertThat(first).isSameAs(second); - } - - @Test - public void freezeAfterMutation() { - ReadableSpan original = createSpan("foo", builder -> {}); - - MutableSpan mutable1 = MutableSpan.makeMutable(original); - mutable1.setName("updated"); - mutable1.toSpanData(); - - assertThatThrownBy(() -> mutable1.setName("should not be allowed")) - .isInstanceOf(IllegalStateException.class); - - // it should be okay to wrap again and then mutate - MutableSpan mutable2 = MutableSpan.makeMutable(mutable1); - mutable2.setName("updated again"); - - assertThat(mutable1.toSpanData()).hasName("updated"); - assertThat(mutable2.toSpanData()).hasName("updated again"); - - assertThat(mutable1.getOriginalSpan()).isSameAs(original); - assertThat(mutable2.getOriginalSpan()).isSameAs(mutable1); - } - - @Test - public void testAttributesMutations() { - AttributeKey keep = AttributeKey.stringKey("keep-me"); - AttributeKey update = AttributeKey.stringKey("update-me"); - AttributeKey remove = AttributeKey.stringKey("remove-me"); - AttributeKey add = AttributeKey.stringKey("add-me"); - - ReadableSpan original = - createSpan( - "foo", - builder -> { - builder.setAttribute(keep, "keep-original"); - builder.setAttribute(update, "update-original"); - builder.setAttribute(remove, "remove-original"); - }); - - MutableSpan mutable = MutableSpan.makeMutable(original); - - assertThat(mutable.getAttribute(keep)).isEqualTo("keep-original"); - assertThat(mutable.getAttribute(update)).isEqualTo("update-original"); - assertThat(mutable.getAttribute(remove)).isEqualTo("remove-original"); - - mutable.setAttribute(add, "added"); - mutable.removeAttribute(remove); - mutable.setAttribute(update, "updated"); - - assertThat(mutable.getAttribute(keep)).isEqualTo("keep-original"); - assertThat(mutable.getAttribute(update)).isEqualTo("updated"); - assertThat(mutable.getAttribute(remove)).isNull(); - assertThat(mutable.getAttribute(add)).isEqualTo("added"); - - // check again after the MutableSpan has been frozen due ot the toSpanData() call - assertThat(mutable.toSpanData().getAttributes()) - .hasSize(3) - .containsEntry(keep, "keep-original") - .containsEntry(update, "updated") - .containsEntry(add, "added"); - - assertThat(mutable.getAttribute(keep)).isEqualTo("keep-original"); - assertThat(mutable.getAttribute(update)).isEqualTo("updated"); - assertThat(mutable.getAttribute(remove)).isNull(); - assertThat(mutable.getAttribute(add)).isEqualTo("added"); - - // Ensure attributes are cached - assertThat(mutable.toSpanData().getAttributes()).isSameAs(mutable.toSpanData().getAttributes()); - } - - @Test - public void testAttributesReusedIfNotMutated() { - AttributeKey key = AttributeKey.stringKey("first-key"); - AttributeKey cancelledKey = AttributeKey.stringKey("second-key"); - - ReadableSpan original = - createSpan( - "foo", - builder -> { - builder.setAttribute(key, "original"); - }); - - MutableSpan mutable1 = MutableSpan.makeMutable(original); - mutable1.setAttribute(key, "updated"); - mutable1.setAttribute(cancelledKey, "removed later"); - mutable1.setAttribute(key, "original"); - mutable1.removeAttribute(cancelledKey); - - SpanData mutatedSpanData = mutable1.toSpanData(); - - assertThat(mutatedSpanData.getAttributes()).isSameAs(original.toSpanData().getAttributes()); - } - - @Test - public void noDoubleWrapping() { - ReadableSpan original = createSpan("foo", builder -> {}); - - MutableSpan mutable = MutableSpan.makeMutable(original); - assertThat(MutableSpan.makeMutable(mutable)).isSameAs(mutable); - - mutable.setName("updated"); - assertThat(MutableSpan.makeMutable(mutable)).isSameAs(mutable); - } - - private static ReadableSpan createSpan(String name, Consumer spanCustomizer) { - - AtomicReference resultSpan = new AtomicReference<>(); - SpanProcessor collecting = - new SpanProcessor() { - @Override - public void onStart(Context parentContext, ReadWriteSpan span) {} - - @Override - public boolean isStartRequired() { - return false; - } - - @Override - public void onEnd(ReadableSpan span) { - resultSpan.set(span); - } - - @Override - public boolean isEndRequired() { - return true; - } - }; - - try (OpenTelemetrySdk sdk = TestUtils.sdkWith(collecting)) { - - SpanBuilder builder = sdk.getTracer("my-tracer").spanBuilder(name); - spanCustomizer.accept(builder); - builder.startSpan().end(); - return resultSpan.get(); - } - } -} diff --git a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/TestUtils.java b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/TestUtils.java deleted file mode 100644 index bc695e788..000000000 --- a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/internal/TestUtils.java +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.stacktrace.internal; - -import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.trace.SdkTracerProvider; -import io.opentelemetry.sdk.trace.SpanProcessor; - -public class TestUtils { - - private TestUtils() {} - - public static OpenTelemetrySdk sdkWith(SpanProcessor processor) { - return OpenTelemetrySdk.builder() - .setTracerProvider(SdkTracerProvider.builder().addSpanProcessor(processor).build()) - .build(); - } -} From 01edb03d20a91b849f1e8b4a2db99d097ed20735 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 14:43:45 +0200 Subject: [PATCH 02/12] add autoconfig --- span-stacktrace/build.gradle.kts | 6 + .../stacktrace/StackTraceAutoConfig.java | 109 ++++++++++++++++++ .../stacktrace/StackTraceSpanProcessor.java | 12 -- .../stacktrace/StackTraceAutoConfigTest.java | 88 ++++++++++++++ 4 files changed, 203 insertions(+), 12 deletions(-) create mode 100644 span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfig.java create mode 100644 span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfigTest.java diff --git a/span-stacktrace/build.gradle.kts b/span-stacktrace/build.gradle.kts index 50901b6e4..ff7937cd8 100644 --- a/span-stacktrace/build.gradle.kts +++ b/span-stacktrace/build.gradle.kts @@ -7,6 +7,9 @@ description = "OpenTelemetry Java span stacktrace capture module" otelJava.moduleName.set("io.opentelemetry.contrib.stacktrace") dependencies { + annotationProcessor("com.google.auto.service:auto-service") + compileOnly("com.google.auto.service:auto-service-annotations") + api("io.opentelemetry:opentelemetry-sdk") testImplementation("io.opentelemetry:opentelemetry-sdk-testing") @@ -16,4 +19,7 @@ dependencies { testImplementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi") testImplementation("io.opentelemetry.semconv:opentelemetry-semconv-incubating") + + testAnnotationProcessor("com.google.auto.service:auto-service") + testCompileOnly("com.google.auto.service:auto-service-annotations") } diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfig.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfig.java new file mode 100644 index 000000000..7fc2cd304 --- /dev/null +++ b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfig.java @@ -0,0 +1,109 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.stacktrace; + +import com.google.auto.service.AutoService; +import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizer; +import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.trace.ReadableSpan; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.time.Duration; +import java.util.function.Predicate; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.Nullable; + +@AutoService(AutoConfigurationCustomizerProvider.class) +public class StackTraceAutoConfig implements AutoConfigurationCustomizerProvider { + + private static final Logger log = Logger.getLogger(StackTraceAutoConfig.class.getName()); + + private static final String CONFIG_MIN_DURATION = + "otel.java.experimental.span-stacktrace.min.duration"; + private static final Duration CONFIG_MIN_DURATION_DEFAULT = Duration.ofMillis(5); + + private static final String CONFIG_FILTER = "otel.java.experimental.span-stacktrace.filter"; + + @Override + public void customize(AutoConfigurationCustomizer config) { + config.addTracerProviderCustomizer( + (providerBuilder, properties) -> { + long minDuration = getMinDuration(properties); + Predicate filter = getFilterPredicate(properties); + providerBuilder.addSpanProcessor(new StackTraceSpanProcessor(minDuration, filter)); + return providerBuilder; + }); + } + + // package-private for testing + static long getMinDuration(ConfigProperties properties) { + long minDuration = + properties.getDuration(CONFIG_MIN_DURATION, CONFIG_MIN_DURATION_DEFAULT).toNanos(); + if (minDuration < 0) { + log.fine("Stack traces capture is disabled"); + } else { + log.log( + Level.FINE, + "Stack traces will be added to spans with a minimum duration of {0} nanos", + minDuration); + } + return minDuration; + } + + // package private for testing + static Predicate getFilterPredicate(ConfigProperties properties) { + String filterClass = properties.getString(CONFIG_FILTER); + Predicate filter = null; + if (filterClass != null) { + Class filterType = getFilterType(filterClass); + if (filterType != null) { + filter = getFilterInstance(filterType); + } + } + + if (filter == null) { + // if value is set, lack of filtering is likely an error and must be reported + Level disabledLogLevel = filterClass != null ? Level.SEVERE : Level.FINE; + log.log(disabledLogLevel, "Span stacktrace filtering disabled"); + return span -> true; + } else { + log.fine("Span stacktrace filtering enabled with: " + filterClass); + return filter; + } + } + + @Nullable + private static Class getFilterType(String filterClass) { + try { + Class filterType = Class.forName(filterClass); + if (!Predicate.class.isAssignableFrom(filterType)) { + log.severe("Filter must be a subclass of java.util.function.Predicate"); + return null; + } + return filterType; + } catch (ClassNotFoundException e) { + log.severe("Unable to load filter class: " + filterClass); + return null; + } + } + + @Nullable + @SuppressWarnings("unchecked") + private static Predicate getFilterInstance(Class filterType) { + try { + Constructor constructor = filterType.getConstructor(); + return (Predicate) constructor.newInstance(); + } catch (NoSuchMethodException + | InstantiationException + | IllegalAccessException + | InvocationTargetException e) { + log.severe("Unable to create filter instance with no-arg constructor: " + filterType); + return null; + } + } +} diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java index 9e5dc67db..1ab660f22 100644 --- a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java +++ b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java @@ -15,8 +15,6 @@ import java.io.StringWriter; import java.time.Duration; import java.util.function.Predicate; -import java.util.logging.Level; -import java.util.logging.Logger; public class StackTraceSpanProcessor implements ExtendedSpanProcessor { @@ -28,8 +26,6 @@ public class StackTraceSpanProcessor implements ExtendedSpanProcessor { private static final AttributeKey SPAN_STACKTRACE = AttributeKey.stringKey("code.stacktrace"); - private static final Logger logger = Logger.getLogger(StackTraceSpanProcessor.class.getName()); - private final long minSpanDurationNanos; private final Predicate filterPredicate; @@ -42,14 +38,6 @@ public StackTraceSpanProcessor( long minSpanDurationNanos, Predicate filterPredicate) { this.minSpanDurationNanos = minSpanDurationNanos; this.filterPredicate = filterPredicate; - if (minSpanDurationNanos < 0) { - logger.log(Level.FINE, "Stack traces capture is disabled"); - } else { - logger.log( - Level.FINE, - "Stack traces will be added to spans with a minimum duration of {0} nanos", - minSpanDurationNanos); - } } /** diff --git a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfigTest.java b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfigTest.java new file mode 100644 index 000000000..4b8f999f4 --- /dev/null +++ b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfigTest.java @@ -0,0 +1,88 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.stacktrace; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; +import io.opentelemetry.sdk.trace.ReadableSpan; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Predicate; +import org.junit.jupiter.api.Test; + +public class StackTraceAutoConfigTest { + + @Test + void defaultConfig() { + DefaultConfigProperties config = DefaultConfigProperties.createFromMap(Collections.emptyMap()); + assertThat(StackTraceAutoConfig.getMinDuration(config)).isEqualTo(5000000L); + Predicate filterPredicate = StackTraceAutoConfig.getFilterPredicate(config); + assertThat(filterPredicate).isNotNull(); + } + + @Test + void minDurationValue() { + Map configMap = new HashMap<>(); + configMap.put("otel.java.experimental.span-stacktrace.min.duration", "42ms"); + DefaultConfigProperties config = DefaultConfigProperties.createFromMap(configMap); + assertThat(StackTraceAutoConfig.getMinDuration(config)).isEqualTo(42000000L); + } + + @Test + void negativeMinDuration() { + Map configMap = new HashMap<>(); + configMap.put("otel.java.experimental.span-stacktrace.min.duration", "-1"); + DefaultConfigProperties config = DefaultConfigProperties.createFromMap(configMap); + assertThat(StackTraceAutoConfig.getMinDuration(config)).isNegative(); + } + + @Test + void customFilter() { + Map configMap = new HashMap<>(); + configMap.put("otel.java.experimental.span-stacktrace.filter", MyFilter.class.getName()); + DefaultConfigProperties config = DefaultConfigProperties.createFromMap(configMap); + Predicate filterPredicate = StackTraceAutoConfig.getFilterPredicate(config); + assertThat(filterPredicate).isInstanceOf(MyFilter.class); + + // default does not filter, so any negative value means we use the test filter + assertThat(filterPredicate.test(null)).isFalse(); + } + + public static class MyFilter implements Predicate { + @Override + public boolean test(ReadableSpan readableSpan) { + return false; + } + } + + @Test + void brokenFilter_classVisibility() { + testBrokenFilter(BrokenFilter.class.getName()); + } + + @Test + void brokenFilter_type() { + testBrokenFilter(Object.class.getName()); + } + + @Test + void brokenFilter_missingType() { + testBrokenFilter("missing.class.name"); + } + + private static void testBrokenFilter(String filterName) { + Map configMap = new HashMap<>(); + configMap.put("otel.java.experimental.span-stacktrace.filter", filterName); + DefaultConfigProperties config = DefaultConfigProperties.createFromMap(configMap); + Predicate filterPredicate = StackTraceAutoConfig.getFilterPredicate(config); + assertThat(filterPredicate).isNotNull(); + assertThat(filterPredicate.test(null)).isTrue(); + } + + private static class BrokenFilter extends MyFilter {} +} From e1e04234e695594cbaf7825fae5e2917af0161d6 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 14:43:54 +0200 Subject: [PATCH 03/12] update readme doc --- span-stacktrace/README.md | 54 +++++++++------------------------------ 1 file changed, 12 insertions(+), 42 deletions(-) diff --git a/span-stacktrace/README.md b/span-stacktrace/README.md index fcd9f6554..f66979469 100644 --- a/span-stacktrace/README.md +++ b/span-stacktrace/README.md @@ -6,51 +6,21 @@ This module provides a `SpanProcessor` that captures the [`code.stacktrace`](htt Capturing the stack trace is an expensive operation and does not provide any value on short-lived spans. As a consequence it should only be used when the span duration is known, thus on span end. -However, the current SDK API does not allow to modify span attributes on span end, so we have to -introduce other components to make it work as expected. +## Usage and configuration -## Usage +This extension supports autoconfiguration. -This extension does not support autoconfiguration because it needs to wrap the `SimpleSpanExporter` -or `BatchingSpanProcessor` that invokes the `SpanExporter`. +`otel.java.experimental.span-stacktrace.min.duration`: + - allows to configure the minimal duration for which spans have a stacktrace captured + - defaults to 5ms + - a value of zero will include all spans + - a negative value will disable the feature -As a consequence you have to use [Manual SDK setup](#manual-sdk-setup) -section below to configure it. - -### Manual SDK setup - -Here is an example registration of `StackTraceSpanProcessor` to capture stack trace for all -the spans that have a duration >= 1 ms. The spans that have an `ignorespan` string attribute -will be ignored. - -```java -InMemorySpanExporter spansExporter = InMemorySpanExporter.create(); -SpanProcessor exportProcessor = SimpleSpanProcessor.create(spansExporter); - -Map configMap = new HashMap<>(); -configMap.put("otel.java.experimental.span-stacktrace.min.duration", "1ms"); -ConfigProperties config = DefaultConfigProperties.createFromMap(configMap); - -Predicate filterPredicate = readableSpan -> { - if(readableSpan.getAttribute(AttributeKey.stringKey("ignorespan")) != null){ - return false; - } - return true; -}; -SdkTracerProvider tracerProvider = SdkTracerProvider.builder() - .addSpanProcessor(new StackTraceSpanProcessor(exportProcessor, config, filterPredicate)) - .build(); - -OpenTelemetrySdk sdk = OpenTelemetrySdk.builder().setTracerProvider(tracerProvider).build(); -``` - -### Configuration - -The `otel.java.experimental.span-stacktrace.min.duration` configuration option (defaults to 5ms) allows configuring -the minimal duration for which spans should have a stacktrace captured. - -Setting `otel.java.experimental.span-stacktrace.min.duration` to zero will include all spans, and using a negative -value will disable the feature. +`otel.java.experimental.span-stacktrace.filter`: +- allows to filter spans to be excluded from stacktrace capture +- defaults to include all spans. +- value is the class name of a class implementing `java.util.function.Predicate` +- filter class must be publicly accessible and provide a no-arg constructor ## Component owners From 2b0f80cda580606dabc526fd1d4474537e0bd436 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:03:56 +0200 Subject: [PATCH 04/12] remove duplicated code --- .../stacktrace/StackTraceSpanProcessor.java | 16 ---------------- .../StackTraceSpanProcessorTest.java | 19 ++++++++++--------- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java index 1ab660f22..a35ef2e0e 100644 --- a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java +++ b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java @@ -7,21 +7,15 @@ import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.context.Context; -import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.trace.ReadWriteSpan; import io.opentelemetry.sdk.trace.ReadableSpan; import io.opentelemetry.sdk.trace.internal.ExtendedSpanProcessor; import java.io.PrintWriter; import java.io.StringWriter; -import java.time.Duration; import java.util.function.Predicate; public class StackTraceSpanProcessor implements ExtendedSpanProcessor { - private static final String CONFIG_MIN_DURATION = - "otel.java.experimental.span-stacktrace.min.duration"; - private static final Duration CONFIG_MIN_DURATION_DEFAULT = Duration.ofMillis(5); - // inlined incubating attribute to prevent direct dependency on incubating semconv private static final AttributeKey SPAN_STACKTRACE = AttributeKey.stringKey("code.stacktrace"); @@ -40,16 +34,6 @@ public StackTraceSpanProcessor( this.filterPredicate = filterPredicate; } - /** - * @param config configuration - * @param filterPredicate extra filter function to exclude spans if needed - */ - public StackTraceSpanProcessor(ConfigProperties config, Predicate filterPredicate) { - this( - config.getDuration(CONFIG_MIN_DURATION, CONFIG_MIN_DURATION_DEFAULT).toNanos(), - filterPredicate); - } - @Override public boolean isStartRequired() { return false; diff --git a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java index f8b6cbe4a..eec407f4b 100644 --- a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java +++ b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java @@ -73,10 +73,10 @@ void spanWithExistingStackTrace() { } private static void checkSpanWithStackTrace( - Predicate filterPredicate, String configString, long spanDurationNanos) { + Predicate filterPredicate, String minDurationString, long spanDurationNanos) { checkSpan( filterPredicate, - configString, + minDurationString, spanDurationNanos, Function.identity(), (stackTrace) -> @@ -86,10 +86,10 @@ private static void checkSpanWithStackTrace( } private static void checkSpanWithoutStackTrace( - Predicate filterPredicate, String configString, long spanDurationNanos) { + Predicate filterPredicate, String minDurationString, long spanDurationNanos) { checkSpan( filterPredicate, - configString, + minDurationString, spanDurationNanos, Function.identity(), (stackTrace) -> assertThat(stackTrace).describedAs("no stack trace expected").isNull()); @@ -97,7 +97,7 @@ private static void checkSpanWithoutStackTrace( private static void checkSpan( Predicate filterPredicate, - String configString, + String minDurationString, long spanDurationNanos, Function customizeSpanBuilder, Consumer stackTraceCheck) { @@ -107,13 +107,14 @@ private static void checkSpan( SpanProcessor exportProcessor = SimpleSpanProcessor.create(spansExporter); Map configMap = new HashMap<>(); - if (configString != null) { - configMap.put("otel.java.experimental.span-stacktrace.min.duration", configString); + if (minDurationString != null) { + configMap.put("otel.java.experimental.span-stacktrace.min.duration", minDurationString); } + long minDuration = StackTraceAutoConfig.getMinDuration( + DefaultConfigProperties.createFromMap(configMap)); StackTraceSpanProcessor processor = - new StackTraceSpanProcessor( - DefaultConfigProperties.createFromMap(configMap), filterPredicate); + new StackTraceSpanProcessor(minDuration, filterPredicate); try (OpenTelemetrySdk sdk = OpenTelemetrySdk.builder() From 9e11955b54a33cc544b28083b65bdf716cf097e2 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:11:34 +0200 Subject: [PATCH 05/12] lint before push you should --- .../contrib/stacktrace/StackTraceSpanProcessorTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java index eec407f4b..5eb2c6d61 100644 --- a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java +++ b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java @@ -110,11 +110,10 @@ private static void checkSpan( if (minDurationString != null) { configMap.put("otel.java.experimental.span-stacktrace.min.duration", minDurationString); } - long minDuration = StackTraceAutoConfig.getMinDuration( - DefaultConfigProperties.createFromMap(configMap)); + long minDuration = + StackTraceAutoConfig.getMinDuration(DefaultConfigProperties.createFromMap(configMap)); - StackTraceSpanProcessor processor = - new StackTraceSpanProcessor(minDuration, filterPredicate); + StackTraceSpanProcessor processor = new StackTraceSpanProcessor(minDuration, filterPredicate); try (OpenTelemetrySdk sdk = OpenTelemetrySdk.builder() From 4ac2c28a640c97a0a1192514477f0183a9ca65f6 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:25:13 +0200 Subject: [PATCH 06/12] fix markdown --- span-stacktrace/README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/span-stacktrace/README.md b/span-stacktrace/README.md index f66979469..e3edffca0 100644 --- a/span-stacktrace/README.md +++ b/span-stacktrace/README.md @@ -10,13 +10,15 @@ As a consequence it should only be used when the span duration is known, thus on This extension supports autoconfiguration. -`otel.java.experimental.span-stacktrace.min.duration`: +`otel.java.experimental.span-stacktrace.min.duration` + - allows to configure the minimal duration for which spans have a stacktrace captured - defaults to 5ms - a value of zero will include all spans - a negative value will disable the feature -`otel.java.experimental.span-stacktrace.filter`: +`otel.java.experimental.span-stacktrace.filter` + - allows to filter spans to be excluded from stacktrace capture - defaults to include all spans. - value is the class name of a class implementing `java.util.function.Predicate` From 7a9f2f41b406ef5aeea8a3036ae20f8b76fb2507 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:32:42 +0200 Subject: [PATCH 07/12] the final markdown ? --- span-stacktrace/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/span-stacktrace/README.md b/span-stacktrace/README.md index e3edffca0..45bec3f79 100644 --- a/span-stacktrace/README.md +++ b/span-stacktrace/README.md @@ -19,10 +19,10 @@ This extension supports autoconfiguration. `otel.java.experimental.span-stacktrace.filter` -- allows to filter spans to be excluded from stacktrace capture -- defaults to include all spans. -- value is the class name of a class implementing `java.util.function.Predicate` -- filter class must be publicly accessible and provide a no-arg constructor + - allows to filter spans to be excluded from stacktrace capture + - defaults to include all spans. + - value is the class name of a class implementing `java.util.function.Predicate` + - filter class must be publicly accessible and provide a no-arg constructor ## Component owners From c75298ac58e76ccc3ec78051c4df3c69e656a879 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:49:33 +0200 Subject: [PATCH 08/12] markdown again --- span-stacktrace/README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/span-stacktrace/README.md b/span-stacktrace/README.md index 45bec3f79..235ae609a 100644 --- a/span-stacktrace/README.md +++ b/span-stacktrace/README.md @@ -12,17 +12,17 @@ This extension supports autoconfiguration. `otel.java.experimental.span-stacktrace.min.duration` - - allows to configure the minimal duration for which spans have a stacktrace captured - - defaults to 5ms - - a value of zero will include all spans - - a negative value will disable the feature +- allows to configure the minimal duration for which spans have a stacktrace captured +- defaults to 5ms +- a value of zero will include all spans +- a negative value will disable the feature `otel.java.experimental.span-stacktrace.filter` - - allows to filter spans to be excluded from stacktrace capture - - defaults to include all spans. - - value is the class name of a class implementing `java.util.function.Predicate` - - filter class must be publicly accessible and provide a no-arg constructor +- allows to filter spans to be excluded from stacktrace capture +- defaults to include all spans. +- value is the class name of a class implementing `java.util.function.Predicate` +- filter class must be publicly accessible and provide a no-arg constructor ## Component owners From d1f984ac2abb5be2028858313b0e6bc28eeed22f Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 17:21:50 +0200 Subject: [PATCH 09/12] skip extra processor when disabled --- .../contrib/stacktrace/StackTraceAutoConfig.java | 6 ++++-- .../contrib/stacktrace/StackTraceSpanProcessor.java | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfig.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfig.java index 7fc2cd304..2315d2a10 100644 --- a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfig.java +++ b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceAutoConfig.java @@ -34,8 +34,10 @@ public void customize(AutoConfigurationCustomizer config) { config.addTracerProviderCustomizer( (providerBuilder, properties) -> { long minDuration = getMinDuration(properties); - Predicate filter = getFilterPredicate(properties); - providerBuilder.addSpanProcessor(new StackTraceSpanProcessor(minDuration, filter)); + if (minDuration >= 0) { + Predicate filter = getFilterPredicate(properties); + providerBuilder.addSpanProcessor(new StackTraceSpanProcessor(minDuration, filter)); + } return providerBuilder; }); } diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java index a35ef2e0e..541e97e02 100644 --- a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java +++ b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java @@ -30,6 +30,10 @@ public class StackTraceSpanProcessor implements ExtendedSpanProcessor { */ public StackTraceSpanProcessor( long minSpanDurationNanos, Predicate filterPredicate) { + if (minSpanDurationNanos < 0) { + throw new IllegalArgumentException("minimal span duration must be positive or zero"); + } + this.minSpanDurationNanos = minSpanDurationNanos; this.filterPredicate = filterPredicate; } From 3072467832520b0ed37831ea6758b6c999a27e1c Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 18:10:16 +0200 Subject: [PATCH 10/12] test autoconfig end-to-end for better coverage --- span-stacktrace/build.gradle.kts | 2 + .../StackTraceSpanProcessorTest.java | 95 +++++++++++-------- 2 files changed, 59 insertions(+), 38 deletions(-) diff --git a/span-stacktrace/build.gradle.kts b/span-stacktrace/build.gradle.kts index ff7937cd8..b8316af62 100644 --- a/span-stacktrace/build.gradle.kts +++ b/span-stacktrace/build.gradle.kts @@ -22,4 +22,6 @@ dependencies { testAnnotationProcessor("com.google.auto.service:auto-service") testCompileOnly("com.google.auto.service:auto-service-annotations") + + testImplementation("io.opentelemetry:opentelemetry-exporter-logging") } diff --git a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java index 5eb2c6d61..96385b8c5 100644 --- a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java +++ b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java @@ -12,13 +12,12 @@ import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Scope; import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; +import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; +import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder; import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter; import io.opentelemetry.sdk.trace.ReadableSpan; -import io.opentelemetry.sdk.trace.SdkTracerProvider; -import io.opentelemetry.sdk.trace.SpanProcessor; import io.opentelemetry.sdk.trace.data.SpanData; -import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor; +import io.opentelemetry.sdk.trace.export.SpanExporter; import io.opentelemetry.semconv.incubating.CodeIncubatingAttributes; import java.time.Duration; import java.time.Instant; @@ -39,43 +38,57 @@ private static long msToNs(int ms) { @Test void durationAndFiltering() { // on duration threshold - checkSpanWithStackTrace(span -> true, "1ms", msToNs(1)); + checkSpanWithStackTrace("1ms", msToNs(1)); // over duration threshold - checkSpanWithStackTrace(span -> true, "1ms", msToNs(2)); + checkSpanWithStackTrace("1ms", msToNs(2)); // under duration threshold - checkSpanWithoutStackTrace(span -> true, "2ms", msToNs(1)); + checkSpanWithoutStackTrace(YesPredicate.class, "2ms", msToNs(1)); // filtering out span - checkSpanWithoutStackTrace(span -> false, "1ms", 20); + checkSpanWithoutStackTrace(NoPredicate.class, "1ms", 20); + } + + public static class YesPredicate implements Predicate { + + @Override + public boolean test(ReadableSpan readableSpan) { + return true; + } + } + + public static class NoPredicate implements Predicate { + @Override + public boolean test(ReadableSpan readableSpan) { + return false; + } } @Test void defaultConfig() { long expectedDefault = msToNs(5); - checkSpanWithStackTrace(span -> true, null, expectedDefault); - checkSpanWithStackTrace(span -> true, null, expectedDefault + 1); - checkSpanWithoutStackTrace(span -> true, null, expectedDefault - 1); + checkSpanWithStackTrace(null, expectedDefault); + checkSpanWithStackTrace(null, expectedDefault + 1); + checkSpanWithoutStackTrace(YesPredicate.class, null, expectedDefault - 1); } @Test void disabledConfig() { - checkSpanWithoutStackTrace(span -> true, "-1", 5); + checkSpanWithoutStackTrace(YesPredicate.class, "-1", 5); } @Test void spanWithExistingStackTrace() { checkSpan( - span -> true, + YesPredicate.class, "1ms", Duration.ofMillis(1).toNanos(), sb -> sb.setAttribute(CodeIncubatingAttributes.CODE_STACKTRACE, "hello"), stacktrace -> assertThat(stacktrace).isEqualTo("hello")); } - private static void checkSpanWithStackTrace( - Predicate filterPredicate, String minDurationString, long spanDurationNanos) { + private static void checkSpanWithStackTrace(String minDurationString, long spanDurationNanos) { checkSpan( - filterPredicate, + YesPredicate.class, minDurationString, spanDurationNanos, Function.identity(), @@ -86,9 +99,10 @@ private static void checkSpanWithStackTrace( } private static void checkSpanWithoutStackTrace( - Predicate filterPredicate, String minDurationString, long spanDurationNanos) { + Class> predicateClass, String minDurationString, + long spanDurationNanos) { checkSpan( - filterPredicate, + predicateClass, minDurationString, spanDurationNanos, Function.identity(), @@ -96,33 +110,38 @@ private static void checkSpanWithoutStackTrace( } private static void checkSpan( - Predicate filterPredicate, + Class> predicateClass, String minDurationString, long spanDurationNanos, Function customizeSpanBuilder, Consumer stackTraceCheck) { - // they must be re-created as they are shutdown when the span processor is closed + // must be re-created on every test as exporter is shut down on span processor close InMemorySpanExporter spansExporter = InMemorySpanExporter.create(); - SpanProcessor exportProcessor = SimpleSpanProcessor.create(spansExporter); - Map configMap = new HashMap<>(); - if (minDurationString != null) { - configMap.put("otel.java.experimental.span-stacktrace.min.duration", minDurationString); - } - long minDuration = - StackTraceAutoConfig.getMinDuration(DefaultConfigProperties.createFromMap(configMap)); - - StackTraceSpanProcessor processor = new StackTraceSpanProcessor(minDuration, filterPredicate); - - try (OpenTelemetrySdk sdk = - OpenTelemetrySdk.builder() - .setTracerProvider( - SdkTracerProvider.builder() - .addSpanProcessor(processor) - .addSpanProcessor(exportProcessor) - .build()) - .build()) { + AutoConfiguredOpenTelemetrySdkBuilder sdkBuilder = AutoConfiguredOpenTelemetrySdk.builder(); + sdkBuilder.addPropertiesSupplier(() -> { + Map configMap = new HashMap<>(); + + configMap.put("otel.metrics.exporter", "none"); + configMap.put("otel.traces.exporter", "logging"); + configMap.put("otel.logs.exporter", "none"); + + if (minDurationString != null) { + configMap.put("otel.java.experimental.span-stacktrace.min.duration", minDurationString); + } + if (predicateClass != null) { + configMap.put("otel.java.experimental.span-stacktrace.filter", predicateClass.getName()); + } + return configMap; + }); + // duplicate export to our in-memory span exporter + sdkBuilder.addSpanExporterCustomizer( + (exporter, config) -> SpanExporter.composite(exporter, spansExporter)); + + new StackTraceAutoConfig().customize(sdkBuilder); + + try (OpenTelemetrySdk sdk = sdkBuilder.build().getOpenTelemetrySdk()) { Tracer tracer = sdk.getTracer("test"); From 60451c0463c9d3be564927078431c33dff6f537a Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 18:18:37 +0200 Subject: [PATCH 11/12] increase coverage + reformat --- .../stacktrace/StackTraceSpanProcessor.java | 2 +- .../StackTraceSpanProcessorTest.java | 44 ++++++++++++------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java index 541e97e02..62fb9ff34 100644 --- a/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java +++ b/span-stacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessor.java @@ -53,7 +53,7 @@ public boolean isOnEndingRequired() { @Override public void onEnding(ReadWriteSpan span) { - if (minSpanDurationNanos < 0 || span.getLatencyNanos() < minSpanDurationNanos) { + if (span.getLatencyNanos() < minSpanDurationNanos) { return; } if (span.getAttribute(SPAN_STACKTRACE) != null) { diff --git a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java index 96385b8c5..3d6d4686d 100644 --- a/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java +++ b/span-stacktrace/src/test/java/io/opentelemetry/contrib/stacktrace/StackTraceSpanProcessorTest.java @@ -6,6 +6,7 @@ package io.opentelemetry.contrib.stacktrace; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanBuilder; @@ -35,6 +36,12 @@ private static long msToNs(int ms) { return Duration.ofMillis(ms).toNanos(); } + @Test + void tryInvalidMinDuration() { + assertThatCode(() -> new StackTraceSpanProcessor(-1, null)) + .isInstanceOf(IllegalArgumentException.class); + } + @Test void durationAndFiltering() { // on duration threshold @@ -45,7 +52,7 @@ void durationAndFiltering() { checkSpanWithoutStackTrace(YesPredicate.class, "2ms", msToNs(1)); // filtering out span - checkSpanWithoutStackTrace(NoPredicate.class, "1ms", 20); + checkSpanWithoutStackTrace(NoPredicate.class, "1ms", msToNs(20)); } public static class YesPredicate implements Predicate { @@ -99,7 +106,8 @@ private static void checkSpanWithStackTrace(String minDurationString, long spanD } private static void checkSpanWithoutStackTrace( - Class> predicateClass, String minDurationString, + Class> predicateClass, + String minDurationString, long spanDurationNanos) { checkSpan( predicateClass, @@ -120,21 +128,23 @@ private static void checkSpan( InMemorySpanExporter spansExporter = InMemorySpanExporter.create(); AutoConfiguredOpenTelemetrySdkBuilder sdkBuilder = AutoConfiguredOpenTelemetrySdk.builder(); - sdkBuilder.addPropertiesSupplier(() -> { - Map configMap = new HashMap<>(); - - configMap.put("otel.metrics.exporter", "none"); - configMap.put("otel.traces.exporter", "logging"); - configMap.put("otel.logs.exporter", "none"); - - if (minDurationString != null) { - configMap.put("otel.java.experimental.span-stacktrace.min.duration", minDurationString); - } - if (predicateClass != null) { - configMap.put("otel.java.experimental.span-stacktrace.filter", predicateClass.getName()); - } - return configMap; - }); + sdkBuilder.addPropertiesSupplier( + () -> { + Map configMap = new HashMap<>(); + + configMap.put("otel.metrics.exporter", "none"); + configMap.put("otel.traces.exporter", "logging"); + configMap.put("otel.logs.exporter", "none"); + + if (minDurationString != null) { + configMap.put("otel.java.experimental.span-stacktrace.min.duration", minDurationString); + } + if (predicateClass != null) { + configMap.put( + "otel.java.experimental.span-stacktrace.filter", predicateClass.getName()); + } + return configMap; + }); // duplicate export to our in-memory span exporter sdkBuilder.addSpanExporterCustomizer( (exporter, config) -> SpanExporter.composite(exporter, spansExporter)); From 8745a7fac46cb7c7ed42854209f7144bf4df04ee Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 16 Oct 2024 21:02:37 +0200 Subject: [PATCH 12/12] elaborate a bit on autoconfiguration --- span-stacktrace/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/span-stacktrace/README.md b/span-stacktrace/README.md index 235ae609a..92eef3bbe 100644 --- a/span-stacktrace/README.md +++ b/span-stacktrace/README.md @@ -8,7 +8,8 @@ As a consequence it should only be used when the span duration is known, thus on ## Usage and configuration -This extension supports autoconfiguration. +This extension supports autoconfiguration, so it will be automatically enabled by OpenTelemetry +SDK when included in the application runtime dependencies. `otel.java.experimental.span-stacktrace.min.duration`