From be58f300f7dde1bc567864ab62971b2adf326998 Mon Sep 17 00:00:00 2001 From: Otmar Ertl Date: Sun, 10 Dec 2023 21:07:59 +0100 Subject: [PATCH 1/2] switch from acceptance to rejection threshold --- .../ConsistentAlwaysOffSampler.java | 2 +- .../ConsistentAlwaysOnSampler.java | 4 +- .../ConsistentComposedAndSampler.java | 2 +- .../ConsistentComposedOrSampler.java | 2 +- .../ConsistentParentBasedSampler.java | 3 +- .../ConsistentRateLimitingSampler.java | 6 ++- .../consistent56/ConsistentSampler.java | 15 ++----- .../consistent56/ConsistentSamplingUtil.java | 25 ++++++----- .../ConsistentAlwaysOffSamplerTest.java | 16 ++++--- .../ConsistentAlwaysOnSamplerTest.java | 18 ++++---- .../ConsistentFixedThresholdSamplerTest.java | 16 +++---- .../consistent56/ConsistentSamplerTest.java | 44 ++++++++++--------- .../ConsistentSamplingUtilTest.java | 35 +++++++++------ 13 files changed, 98 insertions(+), 90 deletions(-) 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 2b95c19ce..405c1c019 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 @@ -16,7 +16,7 @@ final class ConsistentAlwaysOffSampler extends ConsistentSampler { @Override protected long getThreshold(long parentThreshold, boolean isRoot) { - return 0; + return ConsistentSamplingUtil.getMaxThreshold(); } @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 93db59560..9f9a79e44 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 @@ -5,7 +5,7 @@ package io.opentelemetry.contrib.sampler.consistent56; -import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMaxThreshold; +import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMinThreshold; import javax.annotation.concurrent.Immutable; @@ -18,7 +18,7 @@ final class ConsistentAlwaysOnSampler extends ConsistentSampler { @Override protected long getThreshold(long parentThreshold, boolean isRoot) { - return getMaxThreshold(); + return getMinThreshold(); } @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 de22a7056..f9210692f 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 @@ -43,7 +43,7 @@ protected long getThreshold(long parentThreshold, boolean isRoot) { long threshold2 = sampler2.getThreshold(parentThreshold, isRoot); if (ConsistentSamplingUtil.isValidThreshold(threshold1) && ConsistentSamplingUtil.isValidThreshold(threshold2)) { - return Math.min(threshold1, threshold2); + return Math.max(threshold1, threshold2); } else { return ConsistentSamplingUtil.getInvalidThreshold(); } 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 542cc65bc..40c473793 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 @@ -43,7 +43,7 @@ protected long getThreshold(long parentThreshold, boolean isRoot) { long threshold2 = sampler2.getThreshold(parentThreshold, isRoot); if (ConsistentSamplingUtil.isValidThreshold(threshold1)) { if (ConsistentSamplingUtil.isValidThreshold(threshold2)) { - return Math.max(threshold1, threshold2); + return Math.min(threshold1, threshold2); } return threshold1; } else { 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 9d40793d9..43c8a0027 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 @@ -5,6 +5,7 @@ package io.opentelemetry.contrib.sampler.consistent56; +import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getInvalidThreshold; import static java.util.Objects.requireNonNull; import javax.annotation.concurrent.Immutable; @@ -38,7 +39,7 @@ final class ConsistentParentBasedSampler extends ConsistentSampler { @Override protected long getThreshold(long parentThreshold, boolean isRoot) { if (isRoot) { - return rootSampler.getThreshold(ConsistentSamplingUtil.getInvalidThreshold(), isRoot); + return rootSampler.getThreshold(getInvalidThreshold(), isRoot); } else { return parentThreshold; } 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 3e741df8d..b04221df1 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 @@ -5,6 +5,8 @@ package io.opentelemetry.contrib.sampler.consistent56; +import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.calculateThreshold; +import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMinThreshold; import static java.util.Objects.requireNonNull; import io.opentelemetry.sdk.trace.samplers.Sampler; @@ -147,9 +149,9 @@ protected long getThreshold(long parentThreshold, boolean isRoot) { / currentState.effectiveWindowCount; if (samplingProbability >= 1.) { - return ConsistentSamplingUtil.getMaxThreshold(); + return getMinThreshold(); } else { - return ConsistentSamplingUtil.calculateThreshold(samplingProbability); + return calculateThreshold(samplingProbability); } } 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 005c22d90..e8beec0c7 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 @@ -7,7 +7,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.getMaxThreshold; import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.isValidThreshold; import static java.util.Objects.requireNonNull; @@ -247,15 +246,13 @@ public final SamplingResult shouldSample( long parentThreshold; if (otelTraceState.hasValidThreshold()) { long threshold = otelTraceState.getThreshold(); - if (((randomValue < threshold) == isParentSampled) || threshold == 0) { + if ((randomValue >= threshold) == isParentSampled) { // test invariant parentThreshold = threshold; } else { parentThreshold = getInvalidThreshold(); } - } else if (isParentSampled) { - parentThreshold = getMaxThreshold(); } else { - parentThreshold = 0; + parentThreshold = getInvalidThreshold(); } // determine new threshold that is used for the sampling decision @@ -264,12 +261,8 @@ public final SamplingResult shouldSample( // determine sampling decision boolean isSampled; if (isValidThreshold(threshold)) { - isSampled = (randomValue < threshold); - if (0 < threshold && threshold < getMaxThreshold()) { - otelTraceState.setThreshold(threshold); - } else { - otelTraceState.invalidateThreshold(); - } + isSampled = (randomValue >= threshold); + otelTraceState.setThreshold(threshold); } 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 80a6bffd2..5baa2f75d 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 @@ -10,7 +10,9 @@ public final class ConsistentSamplingUtil { private static final int RANDOM_VALUE_BITS = 56; - private static final long MAX_THRESHOLD = 1L << RANDOM_VALUE_BITS; + private static final long MAX_THRESHOLD = + 1L << RANDOM_VALUE_BITS; // corresponds to 0% sampling probability + private static final long MIN_THRESHOLD = 0; // corresponds to 100% sampling probability private static final long MAX_RANDOM_VALUE = MAX_THRESHOLD - 1; private static final long INVALID_THRESHOLD = -1; private static final long INVALID_RANDOM_VALUE = -1; @@ -29,7 +31,7 @@ private ConsistentSamplingUtil() {} */ public static double calculateSamplingProbability(long threshold) { checkThreshold(threshold); - return threshold * 0x1p-56; + return (MAX_THRESHOLD - threshold) * 0x1p-56; } /** @@ -41,7 +43,7 @@ public static double calculateSamplingProbability(long threshold) { */ public static long calculateThreshold(double samplingProbability) { checkProbability(samplingProbability); - return Math.round(samplingProbability * 0x1p56); + return MAX_THRESHOLD - Math.round(samplingProbability * 0x1p56); } /** @@ -49,21 +51,16 @@ public static long calculateThreshold(double samplingProbability) { * *

Returns 1, if the threshold is invalid. * - *

Returns 0, if the threshold is 0. A span with zero threshold is only sampled due to a - * non-probabilistic sampling decision and therefore does not contribute to the adjusted count. + *

Returns 0, if there is no valid threshold. * * @param threshold the threshold * @return the adjusted count */ public static double calculateAdjustedCount(long threshold) { if (isValidThreshold(threshold)) { - if (threshold > 0) { - return 0x1p56 / threshold; - } else { - return 0; - } + return 0x1p56 / (MAX_THRESHOLD - threshold); } else { - return 1.; + return 0; } } @@ -93,6 +90,10 @@ public static long getMaxRandomValue() { return MAX_RANDOM_VALUE; } + public static long getMinThreshold() { + return MIN_THRESHOLD; + } + public static long getMaxThreshold() { return MAX_THRESHOLD; } @@ -102,7 +103,7 @@ public static boolean isValidRandomValue(long randomValue) { } public static boolean isValidThreshold(long threshold) { - return 0 <= threshold && threshold <= getMaxThreshold(); + return getMinThreshold() <= threshold && threshold <= getMaxThreshold(); } public static boolean isValidProbability(double probability) { diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSamplerTest.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSamplerTest.java index 60f97ac23..04d266ac2 100644 --- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSamplerTest.java +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSamplerTest.java @@ -21,11 +21,15 @@ void testDescription() { @Test void testThreshold() { - assertThat(ConsistentSampler.alwaysOff().getThreshold(getInvalidThreshold(), false)).isZero(); - assertThat(ConsistentSampler.alwaysOff().getThreshold(getInvalidThreshold(), true)).isZero(); - assertThat(ConsistentSampler.alwaysOff().getThreshold(getMaxThreshold(), false)).isZero(); - assertThat(ConsistentSampler.alwaysOff().getThreshold(getMaxThreshold(), true)).isZero(); - assertThat(ConsistentSampler.alwaysOff().getThreshold(0, false)).isZero(); - assertThat(ConsistentSampler.alwaysOff().getThreshold(0, true)).isZero(); + assertThat(ConsistentSampler.alwaysOff().getThreshold(getInvalidThreshold(), false)) + .isEqualTo(getMaxThreshold()); + assertThat(ConsistentSampler.alwaysOff().getThreshold(getInvalidThreshold(), true)) + .isEqualTo(getMaxThreshold()); + assertThat(ConsistentSampler.alwaysOff().getThreshold(getMaxThreshold(), false)) + .isEqualTo(getMaxThreshold()); + assertThat(ConsistentSampler.alwaysOff().getThreshold(getMaxThreshold(), true)) + .isEqualTo(getMaxThreshold()); + assertThat(ConsistentSampler.alwaysOff().getThreshold(0, false)).isEqualTo(getMaxThreshold()); + assertThat(ConsistentSampler.alwaysOff().getThreshold(0, true)).isEqualTo(getMaxThreshold()); } } diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSamplerTest.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSamplerTest.java index 96943be5e..6df53066b 100644 --- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSamplerTest.java +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSamplerTest.java @@ -6,7 +6,7 @@ package io.opentelemetry.contrib.sampler.consistent56; import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getInvalidThreshold; -import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMaxThreshold; +import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMinThreshold; import static org.assertj.core.api.Assertions.assertThat; import org.junit.jupiter.api.Test; @@ -22,14 +22,14 @@ void testDescription() { @Test void testThreshold() { assertThat(ConsistentSampler.alwaysOn().getThreshold(getInvalidThreshold(), false)) - .isEqualTo(getMaxThreshold()); + .isEqualTo(getMinThreshold()); assertThat(ConsistentSampler.alwaysOn().getThreshold(getInvalidThreshold(), true)) - .isEqualTo(getMaxThreshold()); - assertThat(ConsistentSampler.alwaysOn().getThreshold(getMaxThreshold(), false)) - .isEqualTo(getMaxThreshold()); - assertThat(ConsistentSampler.alwaysOn().getThreshold(getMaxThreshold(), true)) - .isEqualTo(getMaxThreshold()); - assertThat(ConsistentSampler.alwaysOn().getThreshold(0, false)).isEqualTo(getMaxThreshold()); - assertThat(ConsistentSampler.alwaysOn().getThreshold(0, true)).isEqualTo(getMaxThreshold()); + .isEqualTo(getMinThreshold()); + assertThat(ConsistentSampler.alwaysOn().getThreshold(getMinThreshold(), false)) + .isEqualTo(getMinThreshold()); + assertThat(ConsistentSampler.alwaysOn().getThreshold(getMinThreshold(), true)) + .isEqualTo(getMinThreshold()); + assertThat(ConsistentSampler.alwaysOn().getThreshold(0, false)).isEqualTo(getMinThreshold()); + assertThat(ConsistentSampler.alwaysOn().getThreshold(0, true)).isEqualTo(getMinThreshold()); } } 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 340f6641f..2cec02dff 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 @@ -63,12 +63,8 @@ private void testSampling(SplittableRandom rng, double samplingProbability) { .get(OtelTraceState.TRACE_STATE_KEY); OtelTraceState traceState = OtelTraceState.parse(traceStateString); assertThat(traceState.hasValidRandomValue()).isTrue(); - if (samplingProbability == 1.) { - assertThat(traceState.hasValidThreshold()).isFalse(); - } else { - assertThat(traceState.hasValidThreshold()).isTrue(); - assertThat(traceState.getThreshold()).isEqualTo(calculateThreshold(samplingProbability)); - } + assertThat(traceState.hasValidThreshold()).isTrue(); + assertThat(traceState.getThreshold()).isEqualTo(calculateThreshold(samplingProbability)); numSampled += 1; } @@ -101,14 +97,14 @@ public void testSampling() { @Test public void testDescription() { assertThat(ConsistentSampler.probabilityBased(1.0).getDescription()) - .isEqualTo("ConsistentFixedThresholdSampler{threshold=max, sampling probability=1.0}"); + .isEqualTo("ConsistentFixedThresholdSampler{threshold=0, sampling probability=1.0}"); assertThat(ConsistentSampler.probabilityBased(0.5).getDescription()) .isEqualTo("ConsistentFixedThresholdSampler{threshold=8, sampling probability=0.5}"); assertThat(ConsistentSampler.probabilityBased(0.25).getDescription()) - .isEqualTo("ConsistentFixedThresholdSampler{threshold=4, sampling probability=0.25}"); + .isEqualTo("ConsistentFixedThresholdSampler{threshold=c, sampling probability=0.25}"); assertThat(ConsistentSampler.probabilityBased(1e-300).getDescription()) - .isEqualTo("ConsistentFixedThresholdSampler{threshold=0, sampling probability=0.0}"); + .isEqualTo("ConsistentFixedThresholdSampler{threshold=max, sampling probability=0.0}"); assertThat(ConsistentSampler.probabilityBased(0).getDescription()) - .isEqualTo("ConsistentFixedThresholdSampler{threshold=0, sampling probability=0.0}"); + .isEqualTo("ConsistentFixedThresholdSampler{threshold=max, sampling probability=0.0}"); } } 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 e36868a56..da6236874 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 @@ -6,6 +6,7 @@ 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; import io.opentelemetry.api.common.Attributes; @@ -159,7 +160,7 @@ private static Output sample(Input input, ConsistentSampler sampler) { } @Test - void testMaxThresholdWithoutParentRandomValue() { + void testMinThresholdWithoutParentRandomValue() { Input input = new Input(); @@ -168,13 +169,13 @@ void testMaxThresholdWithoutParentRandomValue() { Output output = sample(input, sampler); assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.RECORD_AND_SAMPLE); - assertThat(output.getThreshold()).isEmpty(); + assertThat(output.getThreshold()).hasValue(0); assertThat(output.getRandomValue()).hasValue(0x20a8397b1dcdafL); assertThat(output.getSampledFlag()).isTrue(); } @Test - void testMaxThresholdWithParentRandomValue() { + void testMinThresholdWithParentRandomValue() { long parentRandomValue = 0x7f99aa40c02744L; @@ -186,18 +187,18 @@ void testMaxThresholdWithParentRandomValue() { Output output = sample(input, sampler); assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.RECORD_AND_SAMPLE); - assertThat(output.getThreshold()).isEmpty(); + assertThat(output.getThreshold()).hasValue(0); assertThat(output.getRandomValue()).hasValue(parentRandomValue); assertThat(output.getSampledFlag()).isTrue(); } @Test - void testMinThreshold() { + void testMaxThreshold() { Input input = new Input(); ConsistentSampler sampler = - new ConsistentFixedThresholdSampler(0L, input.getRandomValueGenerator()); + new ConsistentFixedThresholdSampler(getMaxThreshold(), input.getRandomValueGenerator()); Output output = sample(input, sampler); @@ -211,16 +212,16 @@ void testMinThreshold() { void testHalfThresholdNotSampled() { Input input = new Input(); - input.setParentRandomValue(0x8000000000000L); + input.setParentRandomValue(0x7FFFFFFFFFFFFFL); ConsistentSampler sampler = - new ConsistentFixedThresholdSampler(0x8000000000000L, input.getRandomValueGenerator()); + new ConsistentFixedThresholdSampler(0x80000000000000L, input.getRandomValueGenerator()); Output output = sample(input, sampler); assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.DROP); - assertThat(output.getThreshold()).hasValue(0x8000000000000L); - assertThat(output.getRandomValue()).hasValue(0x8000000000000L); + assertThat(output.getThreshold()).hasValue(0x80000000000000L); + assertThat(output.getRandomValue()).hasValue(0x7FFFFFFFFFFFFFL); assertThat(output.getSampledFlag()).isFalse(); } @@ -228,34 +229,35 @@ void testHalfThresholdNotSampled() { void testHalfThresholdSampled() { Input input = new Input(); - input.setParentRandomValue(0x7ffffffffffffL); + input.setParentRandomValue(0x80000000000000L); ConsistentSampler sampler = - new ConsistentFixedThresholdSampler(0x8000000000000L, input.getRandomValueGenerator()); + new ConsistentFixedThresholdSampler(0x80000000000000L, input.getRandomValueGenerator()); Output output = sample(input, sampler); assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.RECORD_AND_SAMPLE); - assertThat(output.getThreshold()).hasValue(0x8000000000000L); - assertThat(output.getRandomValue()).hasValue(0x7ffffffffffffL); + assertThat(output.getThreshold()).hasValue(0x80000000000000L); + assertThat(output.getRandomValue()).hasValue(0x80000000000000L); assertThat(output.getSampledFlag()).isTrue(); } @Test - void testParentExtraordinarySampledButChildNotSampled() { + void testParentViolatingInvariant() { Input input = new Input(); - input.setParentThreshold(0L); - input.setParentSampled(true); + input.setParentThreshold(0x80000000000000L); + input.setParentRandomValue(0x80000000000000L); + input.setParentSampled(false); ConsistentSampler sampler = new ConsistentFixedThresholdSampler(0x0L, input.getRandomValueGenerator()); Output output = sample(input, sampler); - assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.DROP); + assertThat(output.samplingResult.getDecision()).isEqualTo(SamplingDecision.RECORD_AND_SAMPLE); - assertThat(output.getThreshold()).isEmpty(); - assertThat(output.getRandomValue()).isNotEmpty(); - assertThat(output.getSampledFlag()).isFalse(); + assertThat(output.getThreshold()).hasValue(0x0L); + assertThat(output.getRandomValue()).hasValue(0x80000000000000L); + assertThat(output.getSampledFlag()).isTrue(); } } diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtilTest.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtilTest.java index 944ace75b..e3ad255c3 100644 --- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtilTest.java +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtilTest.java @@ -10,6 +10,8 @@ import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.calculateThreshold; 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.getMaxThreshold; +import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMinThreshold; import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.isValidRandomValue; import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.isValidThreshold; import static org.assertj.core.api.Assertions.assertThat; @@ -21,21 +23,23 @@ public class ConsistentSamplingUtilTest { @Test void testCalculateSamplingProbability() { - assertThat(calculateSamplingProbability(0L)).isEqualTo(0.); - assertThat(calculateSamplingProbability(0x40000000000000L)).isEqualTo(0.25); + assertThat(calculateSamplingProbability(getMinThreshold())).isOne(); + assertThat(calculateSamplingProbability(0xc0000000000000L)).isEqualTo(0.25); assertThat(calculateSamplingProbability(0x80000000000000L)).isEqualTo(0.5); - assertThat(calculateSamplingProbability(0x100000000000000L)).isEqualTo(1.); + assertThat(calculateSamplingProbability(getMaxThreshold())).isZero(); assertThatIllegalArgumentException().isThrownBy(() -> calculateSamplingProbability(-1)); assertThatIllegalArgumentException() - .isThrownBy(() -> calculateSamplingProbability(0x100000000000001L)); + .isThrownBy(() -> calculateSamplingProbability(getMaxThreshold() + 1)); + assertThatIllegalArgumentException() + .isThrownBy(() -> calculateSamplingProbability(getMinThreshold() - 1)); } @Test void testCalculateThreshold() { - assertThat(calculateThreshold(0.)).isEqualTo(0L); - assertThat(calculateThreshold(0.25)).isEqualTo(0x40000000000000L); + assertThat(calculateThreshold(0.)).isEqualTo(getMaxThreshold()); + assertThat(calculateThreshold(0.25)).isEqualTo(0xc0000000000000L); assertThat(calculateThreshold(0.5)).isEqualTo(0x80000000000000L); - assertThat(calculateThreshold(1.)).isEqualTo(0x100000000000000L); + assertThat(calculateThreshold(1.)).isEqualTo(getMinThreshold()); assertThatIllegalArgumentException().isThrownBy(() -> calculateThreshold(Math.nextDown(0.))); assertThatIllegalArgumentException().isThrownBy(() -> calculateThreshold(Math.nextUp(1.))); assertThatIllegalArgumentException() @@ -55,9 +59,14 @@ void testGetInvalidThreshold() { assertThat(isValidThreshold(getInvalidThreshold())).isFalse(); } + @Test + void testGetMinThreshold() { + assertThat(getMinThreshold()).isZero(); + } + @Test void testGetMaxThreshold() { - assertThat(ConsistentSamplingUtil.getMaxThreshold()).isEqualTo(0x100000000000000L); + assertThat(getMaxThreshold()).isEqualTo(0x100000000000000L); } @Test @@ -67,12 +76,12 @@ void testGetMaxRandomValue() { @Test void testCalculateAdjustedCount() { - assertThat(calculateAdjustedCount(0L)).isZero(); - assertThat(calculateAdjustedCount(0x40000000000000L)).isEqualTo(4.); + assertThat(calculateAdjustedCount(getMinThreshold())).isOne(); + assertThat(calculateAdjustedCount(0xc0000000000000L)).isEqualTo(4.); assertThat(calculateAdjustedCount(0x80000000000000L)).isEqualTo(2.); - assertThat(calculateAdjustedCount(0x100000000000000L)).isOne(); - assertThat(calculateAdjustedCount(-1)).isOne(); - assertThat(calculateAdjustedCount(0x100000000000001L)).isOne(); + assertThat(calculateAdjustedCount(getMaxThreshold())).isInfinite(); + assertThat(calculateAdjustedCount(-1)).isZero(); + assertThat(calculateAdjustedCount(0x100000000000001L)).isZero(); } @Test From af8f1b299c8142c24f88cf99bf98a50951a6eefa Mon Sep 17 00:00:00 2001 From: Otmar Ertl Date: Mon, 11 Dec 2023 22:05:13 +0100 Subject: [PATCH 2/2] changed behavior of calculateAdjustedCount for invalid thresholds, fixed javadoc --- .../sampler/consistent56/ConsistentSamplingUtil.java | 11 ++--------- .../consistent56/ConsistentSamplingUtilTest.java | 7 +++++-- 2 files changed, 7 insertions(+), 11 deletions(-) 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 5baa2f75d..8afa38bf0 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 @@ -49,19 +49,12 @@ public static long calculateThreshold(double samplingProbability) { /** * Calculates the adjusted count from a given threshold. * - *

Returns 1, if the threshold is invalid. - * - *

Returns 0, if there is no valid threshold. - * * @param threshold the threshold * @return the adjusted count */ public static double calculateAdjustedCount(long threshold) { - if (isValidThreshold(threshold)) { - return 0x1p56 / (MAX_THRESHOLD - threshold); - } else { - return 0; - } + checkThreshold(threshold); + return 0x1p56 / (MAX_THRESHOLD - threshold); } /** diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtilTest.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtilTest.java index e3ad255c3..fcf2dcd8d 100644 --- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtilTest.java +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplingUtilTest.java @@ -79,9 +79,12 @@ void testCalculateAdjustedCount() { assertThat(calculateAdjustedCount(getMinThreshold())).isOne(); assertThat(calculateAdjustedCount(0xc0000000000000L)).isEqualTo(4.); assertThat(calculateAdjustedCount(0x80000000000000L)).isEqualTo(2.); + assertThat(calculateAdjustedCount(getMaxThreshold() - 1)).isEqualTo(0x1p56); assertThat(calculateAdjustedCount(getMaxThreshold())).isInfinite(); - assertThat(calculateAdjustedCount(-1)).isZero(); - assertThat(calculateAdjustedCount(0x100000000000001L)).isZero(); + assertThatIllegalArgumentException() + .isThrownBy(() -> calculateAdjustedCount(getMinThreshold() - 1)); + assertThatIllegalArgumentException() + .isThrownBy(() -> calculateAdjustedCount(getMaxThreshold() + 1)); } @Test