diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/ParentBasedSamplerBuilder.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/ParentBasedSamplerBuilder.java index 24cb7ea4860..1738646a0be 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/ParentBasedSamplerBuilder.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/ParentBasedSamplerBuilder.java @@ -5,11 +5,14 @@ package io.opentelemetry.sdk.trace.samplers; +import java.util.logging.Logger; import javax.annotation.Nullable; /** A builder for creating ParentBased sampler instances. */ public final class ParentBasedSamplerBuilder { + private static final Logger logger = Logger.getLogger(ParentBasedSamplerBuilder.class.getName()); + private final Sampler root; @Nullable private Sampler remoteParentSampled; @Nullable private Sampler remoteParentNotSampled; @@ -27,6 +30,12 @@ public final class ParentBasedSamplerBuilder { * @return this Builder */ public ParentBasedSamplerBuilder setRemoteParentSampled(Sampler remoteParentSampled) { + if (remoteParentSampled instanceof TraceIdRatioBasedSampler) { + logger.warning( + "TraceIdRatioBasedSampler is being used as a child sampler (remoteParentSampled). " + + "This configuration is discouraged per the OpenTelemetry specification " + + "and may lead to unexpected sampling behavior."); + } this.remoteParentSampled = remoteParentSampled; return this; } @@ -38,6 +47,12 @@ public ParentBasedSamplerBuilder setRemoteParentSampled(Sampler remoteParentSamp * @return this Builder */ public ParentBasedSamplerBuilder setRemoteParentNotSampled(Sampler remoteParentNotSampled) { + if (remoteParentNotSampled instanceof TraceIdRatioBasedSampler) { + logger.warning( + "TraceIdRatioBasedSampler is being used as a child sampler (remoteParentNotSampled). " + + "This configuration is discouraged per the OpenTelemetry specification " + + "and may lead to unexpected sampling behavior."); + } this.remoteParentNotSampled = remoteParentNotSampled; return this; } @@ -49,6 +64,12 @@ public ParentBasedSamplerBuilder setRemoteParentNotSampled(Sampler remoteParentN * @return this Builder */ public ParentBasedSamplerBuilder setLocalParentSampled(Sampler localParentSampled) { + if (localParentSampled instanceof TraceIdRatioBasedSampler) { + logger.warning( + "TraceIdRatioBasedSampler is being used as a child sampler (localParentSampled). " + + "This configuration is discouraged per the OpenTelemetry specification " + + "and may lead to unexpected sampling behavior."); + } this.localParentSampled = localParentSampled; return this; } @@ -60,6 +81,12 @@ public ParentBasedSamplerBuilder setLocalParentSampled(Sampler localParentSample * @return this Builder */ public ParentBasedSamplerBuilder setLocalParentNotSampled(Sampler localParentNotSampled) { + if (localParentNotSampled instanceof TraceIdRatioBasedSampler) { + logger.warning( + "TraceIdRatioBasedSampler is being used as a child sampler (localParentNotSampled). " + + "This configuration is discouraged per the OpenTelemetry specification " + + "and may lead to unexpected sampling behavior."); + } this.localParentNotSampled = localParentNotSampled; return this; } diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/samplers/ParentBasedSamplerBuilderTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/samplers/ParentBasedSamplerBuilderTest.java new file mode 100644 index 00000000000..97972ebd24d --- /dev/null +++ b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/samplers/ParentBasedSamplerBuilderTest.java @@ -0,0 +1,86 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.trace.samplers; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; +import org.junit.jupiter.api.Test; + +class ParentBasedSamplerBuilderTest { + + @Test + void emitsWarningWhenTraceIdRatioBasedUsedAsChildSampler() { + Logger logger = Logger.getLogger(ParentBasedSamplerBuilder.class.getName()); + TestLogHandler handler = new TestLogHandler(); + logger.addHandler(handler); + + try { + Sampler ratioSampler = Sampler.traceIdRatioBased(0.5); + ParentBasedSamplerBuilder builder = Sampler.parentBasedBuilder(Sampler.alwaysOn()); + + builder.setRemoteParentSampled(ratioSampler); + builder.setRemoteParentSampled(ratioSampler); + builder.build(); + + assertTrue(handler.warnings.stream().anyMatch(msg -> msg.contains("remoteParentSampled"))); + } finally { + logger.removeHandler(handler); + } + } + + @Test + void emitsWarningForAllChildSamplerSetters() { + Logger logger = Logger.getLogger(ParentBasedSamplerBuilder.class.getName()); + TestLogHandler handler = new TestLogHandler(); + logger.addHandler(handler); + + try { + Sampler ratioSampler = Sampler.traceIdRatioBased(0.5); + ParentBasedSamplerBuilder builder = Sampler.parentBasedBuilder(Sampler.alwaysOn()); + + builder.setRemoteParentNotSampled(ratioSampler); + builder.setRemoteParentNotSampled(ratioSampler); + + builder.setLocalParentSampled(ratioSampler); + builder.setLocalParentSampled(ratioSampler); + + builder.setLocalParentNotSampled(ratioSampler); + builder.setLocalParentNotSampled(ratioSampler); + + builder.build(); + + assertTrue(handler.warnings.stream().anyMatch(msg -> msg.contains("remoteParentNotSampled"))); + assertTrue(handler.warnings.stream().anyMatch(msg -> msg.contains("localParentSampled"))); + assertTrue(handler.warnings.stream().anyMatch(msg -> msg.contains("localParentNotSampled"))); + } finally { + logger.removeHandler(handler); + } + } + + static final class TestLogHandler extends Handler { + + final List warnings = new ArrayList<>(); + + @Override + public void publish(LogRecord record) { + if (Level.WARNING.equals(record.getLevel())) { + warnings.add(record.getMessage()); + } + } + + @Override + public void flush() {} + + @Override + public void close() {} + } +}