From 34268fa5cabfb142caa57283f0ea96a3ee20ece7 Mon Sep 17 00:00:00 2001 From: Otmar Ertl Date: Tue, 16 Apr 2024 21:09:19 +0200 Subject: [PATCH] * assume least significant bits of trace ID are random by default * remove th-field if not sampled --- .../ConsistentAlwaysOffSampler.java | 8 +- .../ConsistentAlwaysOnSampler.java | 8 +- .../ConsistentComposedAndSampler.java | 6 +- .../ConsistentComposedOrSampler.java | 6 +- .../ConsistentFixedThresholdSampler.java | 4 +- .../ConsistentParentBasedSampler.java | 5 +- .../ConsistentRateLimitingSampler.java | 3 - .../consistent56/ConsistentSampler.java | 104 +++--------------- .../consistent56/ConsistentSamplingUtil.java | 2 +- .../ConsistentFixedThresholdSamplerTest.java | 13 +-- .../ConsistentRateLimitingSamplerTest.java | 66 ++++++----- .../consistent56/ConsistentSamplerTest.java | 30 ++--- .../sampler/consistent56/TestUtil.java | 28 +++++ 13 files changed, 114 insertions(+), 169 deletions(-) create mode 100644 consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/TestUtil.java diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSampler.java index 405c1c019..ba4a0869e 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSampler.java @@ -10,8 +10,12 @@ @Immutable final class ConsistentAlwaysOffSampler extends ConsistentSampler { - ConsistentAlwaysOffSampler(RandomValueGenerator randomValueGenerator) { - super(randomValueGenerator); + private static final ConsistentAlwaysOffSampler INSTANCE = new ConsistentAlwaysOffSampler(); + + private ConsistentAlwaysOffSampler() {} + + static ConsistentAlwaysOffSampler getInstance() { + return INSTANCE; } @Override diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSampler.java index 9f9a79e44..bae1c4b27 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSampler.java @@ -12,8 +12,12 @@ @Immutable final class ConsistentAlwaysOnSampler extends ConsistentSampler { - ConsistentAlwaysOnSampler(RandomValueGenerator randomValueGenerator) { - super(randomValueGenerator); + private static final ConsistentAlwaysOnSampler INSTANCE = new ConsistentAlwaysOnSampler(); + + private ConsistentAlwaysOnSampler() {} + + static ConsistentAlwaysOnSampler getInstance() { + return INSTANCE; } @Override diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentComposedAndSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentComposedAndSampler.java index f9210692f..40df6c895 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentComposedAndSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentComposedAndSampler.java @@ -21,11 +21,7 @@ final class ConsistentComposedAndSampler extends ConsistentSampler { private final ConsistentSampler sampler2; private final String description; - ConsistentComposedAndSampler( - ConsistentSampler sampler1, - ConsistentSampler sampler2, - RandomValueGenerator randomValueGenerator) { - super(randomValueGenerator); + ConsistentComposedAndSampler(ConsistentSampler sampler1, ConsistentSampler sampler2) { this.sampler1 = requireNonNull(sampler1); this.sampler2 = requireNonNull(sampler2); this.description = diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentComposedOrSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentComposedOrSampler.java index 40c473793..b701b5622 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentComposedOrSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentComposedOrSampler.java @@ -21,11 +21,7 @@ final class ConsistentComposedOrSampler extends ConsistentSampler { private final ConsistentSampler sampler2; private final String description; - ConsistentComposedOrSampler( - ConsistentSampler sampler1, - ConsistentSampler sampler2, - RandomValueGenerator randomValueGenerator) { - super(randomValueGenerator); + ConsistentComposedOrSampler(ConsistentSampler sampler1, ConsistentSampler sampler2) { this.sampler1 = requireNonNull(sampler1); this.sampler2 = requireNonNull(sampler2); this.description = diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSampler.java index dd160f387..d2e2fc426 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSampler.java @@ -13,9 +13,7 @@ public class ConsistentFixedThresholdSampler extends ConsistentSampler { private final long threshold; private final String description; - protected ConsistentFixedThresholdSampler( - long threshold, RandomValueGenerator randomValueGenerator) { - super(randomValueGenerator); + protected ConsistentFixedThresholdSampler(long threshold) { checkThreshold(threshold); this.threshold = threshold; diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentParentBasedSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentParentBasedSampler.java index 43c8a0027..bb3f3836a 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentParentBasedSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentParentBasedSampler.java @@ -26,11 +26,8 @@ final class ConsistentParentBasedSampler extends ConsistentSampler { * thread-safe random generator. * * @param rootSampler the root sampler - * @param randomValueGenerator the function to use for generating the r-value */ - ConsistentParentBasedSampler( - ConsistentSampler rootSampler, RandomValueGenerator randomValueGenerator) { - super(randomValueGenerator); + ConsistentParentBasedSampler(ConsistentSampler rootSampler) { this.rootSampler = requireNonNull(rootSampler); this.description = "ConsistentParentBasedSampler{rootSampler=" + rootSampler.getDescription() + '}'; diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSampler.java index b04221df1..d7622effa 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSampler.java @@ -96,15 +96,12 @@ public State(double effectiveWindowCount, double effectiveWindowNanos, long last * @param targetSpansPerSecondLimit the desired spans per second limit * @param adaptationTimeSeconds the typical time to adapt to a new load (time constant used for * exponential smoothing) - * @param randomValueGenerator the function to use for generating the r-value * @param nanoTimeSupplier a supplier for the current nano time */ ConsistentRateLimitingSampler( double targetSpansPerSecondLimit, double adaptationTimeSeconds, - RandomValueGenerator randomValueGenerator, LongSupplier nanoTimeSupplier) { - super(randomValueGenerator); if (targetSpansPerSecondLimit < 0.0) { throw new IllegalArgumentException("Limit for sampled spans per second must be nonnegative!"); diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java index e8beec0c7..618728b5c 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java @@ -8,7 +8,6 @@ import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getInvalidRandomValue; import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getInvalidThreshold; import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.isValidThreshold; -import static java.util.Objects.requireNonNull; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.Span; @@ -32,17 +31,7 @@ public abstract class ConsistentSampler implements Sampler { * @return a sampler */ public static ConsistentSampler alwaysOn() { - return alwaysOn(RandomValueGenerators.getDefault()); - } - - /** - * Returns a {@link ConsistentSampler} that samples all spans. - * - * @param randomValueGenerator the function to use for generating the random value - * @return a sampler - */ - public static ConsistentSampler alwaysOn(RandomValueGenerator randomValueGenerator) { - return new ConsistentAlwaysOnSampler(randomValueGenerator); + return ConsistentAlwaysOnSampler.getInstance(); } /** @@ -51,17 +40,7 @@ public static ConsistentSampler alwaysOn(RandomValueGenerator randomValueGenerat * @return a sampler */ public static ConsistentSampler alwaysOff() { - return alwaysOff(RandomValueGenerators.getDefault()); - } - - /** - * Returns a {@link ConsistentSampler} that does not sample any span. - * - * @param randomValueGenerator the function to use for generating the random value - * @return a sampler - */ - public static ConsistentSampler alwaysOff(RandomValueGenerator randomValueGenerator) { - return new ConsistentAlwaysOffSampler(randomValueGenerator); + return ConsistentAlwaysOffSampler.getInstance(); } /** @@ -71,20 +50,8 @@ public static ConsistentSampler alwaysOff(RandomValueGenerator randomValueGenera * @return a sampler */ public static ConsistentSampler probabilityBased(double samplingProbability) { - return probabilityBased(samplingProbability, RandomValueGenerators.getDefault()); - } - - /** - * Returns a {@link ConsistentSampler} that samples each span with a fixed probability. - * - * @param samplingProbability the sampling probability - * @param randomValueGenerator the function to use for generating the r-value - * @return a sampler - */ - public static ConsistentSampler probabilityBased( - double samplingProbability, RandomValueGenerator randomValueGenerator) { long threshold = ConsistentSamplingUtil.calculateThreshold(samplingProbability); - return new ConsistentFixedThresholdSampler(threshold, randomValueGenerator); + return new ConsistentFixedThresholdSampler(threshold); } /** @@ -94,19 +61,7 @@ public static ConsistentSampler probabilityBased( * @param rootSampler the root sampler */ public static ConsistentSampler parentBased(ConsistentSampler rootSampler) { - return parentBased(rootSampler, RandomValueGenerators.getDefault()); - } - - /** - * Returns a new {@link ConsistentSampler} that respects the sampling decision of the parent span - * or falls-back to the given sampler if it is a root span. - * - * @param rootSampler the root sampler - * @param randomValueGenerator the function to use for generating the random value - */ - public static ConsistentSampler parentBased( - ConsistentSampler rootSampler, RandomValueGenerator randomValueGenerator) { - return new ConsistentParentBasedSampler(rootSampler, randomValueGenerator); + return new ConsistentParentBasedSampler(rootSampler); } /** @@ -119,25 +74,7 @@ public static ConsistentSampler parentBased( */ public static ConsistentSampler rateLimited( double targetSpansPerSecondLimit, double adaptationTimeSeconds) { - return rateLimited( - targetSpansPerSecondLimit, adaptationTimeSeconds, RandomValueGenerators.getDefault()); - } - - /** - * Returns a new {@link ConsistentSampler} that attempts to adjust the sampling probability - * dynamically to meet the target span rate. - * - * @param targetSpansPerSecondLimit the desired spans per second limit - * @param adaptationTimeSeconds the typical time to adapt to a new load (time constant used for - * exponential smoothing) - * @param randomValueGenerator the function to use for generating the random value - */ - public static ConsistentSampler rateLimited( - double targetSpansPerSecondLimit, - double adaptationTimeSeconds, - RandomValueGenerator randomValueGenerator) { - return rateLimited( - targetSpansPerSecondLimit, adaptationTimeSeconds, randomValueGenerator, System::nanoTime); + return rateLimited(targetSpansPerSecondLimit, adaptationTimeSeconds, System::nanoTime); } /** @@ -147,16 +84,14 @@ public static ConsistentSampler rateLimited( * @param targetSpansPerSecondLimit the desired spans per second limit * @param adaptationTimeSeconds the typical time to adapt to a new load (time constant used for * exponential smoothing) - * @param randomValueGenerator the function to use for generating the random value * @param nanoTimeSupplier a supplier for the current nano time */ static ConsistentSampler rateLimited( double targetSpansPerSecondLimit, double adaptationTimeSeconds, - RandomValueGenerator randomValueGenerator, LongSupplier nanoTimeSupplier) { return new ConsistentRateLimitingSampler( - targetSpansPerSecondLimit, adaptationTimeSeconds, randomValueGenerator, nanoTimeSupplier); + targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier); } /** @@ -178,8 +113,7 @@ public ConsistentSampler and(ConsistentSampler otherConsistentSampler) { if (otherConsistentSampler == this) { return this; } - return new ConsistentComposedAndSampler( - this, otherConsistentSampler, RandomValueGenerators.getDefault()); + return new ConsistentComposedAndSampler(this, otherConsistentSampler); } /** @@ -201,14 +135,7 @@ public ConsistentSampler or(ConsistentSampler otherConsistentSampler) { if (otherConsistentSampler == this) { return this; } - return new ConsistentComposedOrSampler( - this, otherConsistentSampler, RandomValueGenerators.getDefault()); - } - - private final RandomValueGenerator randomValueGenerator; - - protected ConsistentSampler(RandomValueGenerator randomValueGenerator) { - this.randomValueGenerator = requireNonNull(randomValueGenerator); + return new ConsistentComposedOrSampler(this, otherConsistentSampler); } @Override @@ -225,9 +152,6 @@ public final SamplingResult shouldSample( boolean isRoot = !parentSpanContext.isValid(); boolean isParentSampled = parentSpanContext.isSampled(); - boolean isRandomTraceIdFlagSet = false; // TODO in future get the random trace ID flag, compare - // https://www.w3.org/TR/trace-context-2/#random-trace-id-flag - TraceState parentTraceState = parentSpanContext.getTraceState(); String otelTraceStateString = parentTraceState.get(OtelTraceState.TRACE_STATE_KEY); OtelTraceState otelTraceState = OtelTraceState.parse(otelTraceStateString); @@ -235,12 +159,8 @@ public final SamplingResult shouldSample( long randomValue; if (otelTraceState.hasValidRandomValue()) { randomValue = otelTraceState.getRandomValue(); - } else if (isRandomTraceIdFlagSet) { - randomValue = OtelTraceState.parseHex(traceId, 18, 14, getInvalidRandomValue()); } else { - randomValue = randomValueGenerator.generate(traceId); - otelTraceState.invalidateThreshold(); - otelTraceState.setRandomValue(randomValue); + randomValue = OtelTraceState.parseHex(traceId, 18, 14, getInvalidRandomValue()); } long parentThreshold; @@ -262,7 +182,11 @@ public final SamplingResult shouldSample( boolean isSampled; if (isValidThreshold(threshold)) { isSampled = (randomValue >= threshold); - otelTraceState.setThreshold(threshold); + if (isSampled) { + otelTraceState.setThreshold(threshold); + } else { + otelTraceState.invalidateThreshold(); + } } else { isSampled = isParentSampled; otelTraceState.invalidateThreshold(); diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtil.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtil.java index 8afa38bf0..4abe4bf15 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtil.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtil.java @@ -115,7 +115,7 @@ static void checkProbability(double probability) { } } - private static final char[] HEX_DIGITS = { + static final char[] HEX_DIGITS = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSamplerTest.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSamplerTest.java index 2cec02dff..7eac3ffb1 100644 --- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSamplerTest.java +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSamplerTest.java @@ -6,7 +6,7 @@ package io.opentelemetry.contrib.sampler.consistent56; import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.calculateThreshold; -import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMaxRandomValue; +import static io.opentelemetry.contrib.sampler.consistent56.TestUtil.generateRandomTraceId; import static org.assertj.core.api.Assertions.assertThat; import io.opentelemetry.api.common.Attributes; @@ -28,7 +28,6 @@ public class ConsistentFixedThresholdSamplerTest { private Context parentContext; - private String traceId; private String name; private SpanKind spanKind; private Attributes attributes; @@ -38,7 +37,6 @@ public class ConsistentFixedThresholdSamplerTest { public void init() { parentContext = Context.root(); - traceId = "0123456789abcdef0123456789abcdef"; name = "name"; spanKind = SpanKind.SERVER; attributes = Attributes.empty(); @@ -48,21 +46,20 @@ public void init() { private void testSampling(SplittableRandom rng, double samplingProbability) { int numSpans = 10000; - Sampler sampler = - ConsistentSampler.probabilityBased( - samplingProbability, s -> rng.nextLong() & getMaxRandomValue()); + Sampler sampler = ConsistentSampler.probabilityBased(samplingProbability); int numSampled = 0; for (long i = 0; i < numSpans; ++i) { SamplingResult samplingResult = - sampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + sampler.shouldSample( + parentContext, generateRandomTraceId(rng), name, spanKind, attributes, parentLinks); if (samplingResult.getDecision() == SamplingDecision.RECORD_AND_SAMPLE) { String traceStateString = samplingResult .getUpdatedTraceState(TraceState.getDefault()) .get(OtelTraceState.TRACE_STATE_KEY); OtelTraceState traceState = OtelTraceState.parse(traceStateString); - assertThat(traceState.hasValidRandomValue()).isTrue(); + assertThat(traceState.hasValidRandomValue()).isFalse(); assertThat(traceState.hasValidThreshold()).isTrue(); assertThat(traceState.getThreshold()).isEqualTo(calculateThreshold(samplingProbability)); diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSamplerTest.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSamplerTest.java index 24c70e4e5..17daaaf6b 100644 --- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSamplerTest.java +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSamplerTest.java @@ -5,7 +5,7 @@ package io.opentelemetry.contrib.sampler.consistent56; -import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMaxRandomValue; +import static io.opentelemetry.contrib.sampler.consistent56.TestUtil.generateRandomTraceId; import static org.assertj.core.api.Assertions.assertThat; import io.opentelemetry.api.common.Attributes; @@ -29,27 +29,22 @@ class ConsistentRateLimitingSamplerTest { private long[] nanoTime; private LongSupplier nanoTimeSupplier; private Context parentContext; - private String traceId; private String name; private SpanKind spanKind; private Attributes attributes; private List parentLinks; - - private static RandomValueGenerator randomValueGenerator() { - SplittableRandom random = new SplittableRandom(0L); - return s -> random.nextLong() & getMaxRandomValue(); - } + private SplittableRandom random; @BeforeEach void init() { nanoTime = new long[] {0L}; nanoTimeSupplier = () -> nanoTime[0]; parentContext = Context.root(); - traceId = "0123456789abcdef0123456789abcdef"; name = "name"; spanKind = SpanKind.SERVER; attributes = Attributes.empty(); parentLinks = Collections.emptyList(); + random = new SplittableRandom(0L); } private void advanceTime(long nanosIncrement) { @@ -68,10 +63,7 @@ void testConstantRate() { ConsistentSampler sampler = ConsistentSampler.rateLimited( - targetSpansPerSecondLimit, - adaptationTimeSeconds, - randomValueGenerator(), - nanoTimeSupplier); + targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier); long nanosBetweenSpans = TimeUnit.MICROSECONDS.toNanos(100); int numSpans = 1000000; @@ -81,7 +73,13 @@ void testConstantRate() { for (int i = 0; i < numSpans; ++i) { advanceTime(nanosBetweenSpans); SamplingResult samplingResult = - sampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + sampler.shouldSample( + parentContext, + generateRandomTraceId(random), + name, + spanKind, + attributes, + parentLinks); if (SamplingDecision.RECORD_AND_SAMPLE.equals(samplingResult.getDecision())) { spanSampledNanos.add(getCurrentTimeNanos()); } @@ -104,10 +102,7 @@ void testRateIncrease() { ConsistentSampler sampler = ConsistentSampler.rateLimited( - targetSpansPerSecondLimit, - adaptationTimeSeconds, - randomValueGenerator(), - nanoTimeSupplier); + targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier); long nanosBetweenSpans1 = TimeUnit.MICROSECONDS.toNanos(100); long nanosBetweenSpans2 = TimeUnit.MICROSECONDS.toNanos(10); @@ -119,7 +114,13 @@ void testRateIncrease() { for (int i = 0; i < numSpans1; ++i) { advanceTime(nanosBetweenSpans1); SamplingResult samplingResult = - sampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + sampler.shouldSample( + parentContext, + generateRandomTraceId(random), + name, + spanKind, + attributes, + parentLinks); if (SamplingDecision.RECORD_AND_SAMPLE.equals(samplingResult.getDecision())) { spanSampledNanos.add(getCurrentTimeNanos()); } @@ -127,7 +128,13 @@ void testRateIncrease() { for (int i = 0; i < numSpans2; ++i) { advanceTime(nanosBetweenSpans2); SamplingResult samplingResult = - sampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + sampler.shouldSample( + parentContext, + generateRandomTraceId(random), + name, + spanKind, + attributes, + parentLinks); if (SamplingDecision.RECORD_AND_SAMPLE.equals(samplingResult.getDecision())) { spanSampledNanos.add(getCurrentTimeNanos()); } @@ -162,10 +169,7 @@ void testRateDecrease() { ConsistentSampler sampler = ConsistentSampler.rateLimited( - targetSpansPerSecondLimit, - adaptationTimeSeconds, - randomValueGenerator(), - nanoTimeSupplier); + targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier); long nanosBetweenSpans1 = TimeUnit.MICROSECONDS.toNanos(10); long nanosBetweenSpans2 = TimeUnit.MICROSECONDS.toNanos(100); @@ -177,7 +181,13 @@ void testRateDecrease() { for (int i = 0; i < numSpans1; ++i) { advanceTime(nanosBetweenSpans1); SamplingResult samplingResult = - sampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + sampler.shouldSample( + parentContext, + generateRandomTraceId(random), + name, + spanKind, + attributes, + parentLinks); if (SamplingDecision.RECORD_AND_SAMPLE.equals(samplingResult.getDecision())) { spanSampledNanos.add(getCurrentTimeNanos()); } @@ -185,7 +195,13 @@ void testRateDecrease() { for (int i = 0; i < numSpans2; ++i) { advanceTime(nanosBetweenSpans2); SamplingResult samplingResult = - sampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + sampler.shouldSample( + parentContext, + generateRandomTraceId(random), + name, + spanKind, + attributes, + parentLinks); if (SamplingDecision.RECORD_AND_SAMPLE.equals(samplingResult.getDecision())) { spanSampledNanos.add(getCurrentTimeNanos()); } diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplerTest.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplerTest.java index da6236874..e3f604020 100644 --- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplerTest.java +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplerTest.java @@ -5,7 +5,6 @@ package io.opentelemetry.contrib.sampler.consistent56; -import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMaxRandomValue; import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMaxThreshold; import static org.assertj.core.api.Assertions.assertThat; @@ -22,7 +21,6 @@ import java.util.Collections; import java.util.List; import java.util.OptionalLong; -import java.util.SplittableRandom; import org.junit.jupiter.api.Test; class ConsistentSamplerTest { @@ -39,8 +37,6 @@ private static class Input { private OptionalLong parentThreshold = OptionalLong.empty(); private OptionalLong parentRandomValue = OptionalLong.empty(); - private final SplittableRandom random = new SplittableRandom(0L); - public void setParentSampled(boolean parentSampled) { this.parentSampled = parentSampled; } @@ -79,10 +75,6 @@ public static Attributes getAttributes() { public static List getParentLinks() { return parentLinks; } - - public RandomValueGenerator getRandomValueGenerator() { - return s -> random.nextLong() & getMaxRandomValue(); - } } private static class Output { @@ -164,13 +156,13 @@ void testMinThresholdWithoutParentRandomValue() { Input input = new Input(); - ConsistentSampler sampler = ConsistentSampler.alwaysOn(input.getRandomValueGenerator()); + ConsistentSampler sampler = ConsistentSampler.alwaysOn(); Output output = sample(input, sampler); assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.RECORD_AND_SAMPLE); assertThat(output.getThreshold()).hasValue(0); - assertThat(output.getRandomValue()).hasValue(0x20a8397b1dcdafL); + assertThat(output.getRandomValue()).isNotPresent(); assertThat(output.getSampledFlag()).isTrue(); } @@ -182,7 +174,7 @@ void testMinThresholdWithParentRandomValue() { Input input = new Input(); input.setParentRandomValue(parentRandomValue); - ConsistentSampler sampler = ConsistentSampler.alwaysOn(input.getRandomValueGenerator()); + ConsistentSampler sampler = ConsistentSampler.alwaysOn(); Output output = sample(input, sampler); @@ -197,14 +189,13 @@ void testMaxThreshold() { Input input = new Input(); - ConsistentSampler sampler = - new ConsistentFixedThresholdSampler(getMaxThreshold(), input.getRandomValueGenerator()); + ConsistentSampler sampler = new ConsistentFixedThresholdSampler(getMaxThreshold()); Output output = sample(input, sampler); assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.DROP); assertThat(output.getThreshold()).isEmpty(); - assertThat(output.getRandomValue()).hasValue(0x20a8397b1dcdafL); + assertThat(output.getRandomValue()).isNotPresent(); assertThat(output.getSampledFlag()).isFalse(); } @@ -214,13 +205,12 @@ void testHalfThresholdNotSampled() { Input input = new Input(); input.setParentRandomValue(0x7FFFFFFFFFFFFFL); - ConsistentSampler sampler = - new ConsistentFixedThresholdSampler(0x80000000000000L, input.getRandomValueGenerator()); + ConsistentSampler sampler = new ConsistentFixedThresholdSampler(0x80000000000000L); Output output = sample(input, sampler); assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.DROP); - assertThat(output.getThreshold()).hasValue(0x80000000000000L); + assertThat(output.getThreshold()).isNotPresent(); assertThat(output.getRandomValue()).hasValue(0x7FFFFFFFFFFFFFL); assertThat(output.getSampledFlag()).isFalse(); } @@ -231,8 +221,7 @@ void testHalfThresholdSampled() { Input input = new Input(); input.setParentRandomValue(0x80000000000000L); - ConsistentSampler sampler = - new ConsistentFixedThresholdSampler(0x80000000000000L, input.getRandomValueGenerator()); + ConsistentSampler sampler = new ConsistentFixedThresholdSampler(0x80000000000000L); Output output = sample(input, sampler); @@ -250,8 +239,7 @@ void testParentViolatingInvariant() { input.setParentRandomValue(0x80000000000000L); input.setParentSampled(false); - ConsistentSampler sampler = - new ConsistentFixedThresholdSampler(0x0L, input.getRandomValueGenerator()); + ConsistentSampler sampler = new ConsistentFixedThresholdSampler(0x0); Output output = sample(input, sampler); assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.RECORD_AND_SAMPLE); diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/TestUtil.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/TestUtil.java new file mode 100644 index 000000000..1ffede0a0 --- /dev/null +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/TestUtil.java @@ -0,0 +1,28 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.sampler.consistent56; + +import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.HEX_DIGITS; + +import java.util.SplittableRandom; + +public final class TestUtil { + + private TestUtil() {} + + static String generateRandomTraceId(SplittableRandom random) { + StringBuilder sb = new StringBuilder(16); + long hi = random.nextLong(); + long lo = random.nextLong(); + for (int i = 0; i < 64; i += 4) { + sb.append(HEX_DIGITS[(int) (hi >>> i) & 0xF]); + } + for (int i = 0; i < 64; i += 4) { + sb.append(HEX_DIGITS[(int) (lo >>> i) & 0xF]); + } + return sb.toString(); + } +}