switch from acceptance to rejection threshold (#1130)

This commit is contained in:
Otmar Ertl 2023-12-13 01:22:49 +01:00 committed by GitHub
parent 73a352f96c
commit 21eee6f25f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 100 additions and 96 deletions

View File

@ -16,7 +16,7 @@ final class ConsistentAlwaysOffSampler extends ConsistentSampler {
@Override
protected long getThreshold(long parentThreshold, boolean isRoot) {
return 0;
return ConsistentSamplingUtil.getMaxThreshold();
}
@Override

View File

@ -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

View File

@ -43,7 +43,7 @@ final class ConsistentComposedAndSampler extends ConsistentSampler {
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();
}

View File

@ -43,7 +43,7 @@ final class ConsistentComposedOrSampler extends ConsistentSampler {
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 {

View File

@ -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;
}

View File

@ -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 @@ final class ConsistentRateLimitingSampler extends ConsistentSampler {
/ currentState.effectiveWindowCount;
if (samplingProbability >= 1.) {
return ConsistentSamplingUtil.getMaxThreshold();
return getMinThreshold();
} else {
return ConsistentSamplingUtil.calculateThreshold(samplingProbability);
return calculateThreshold(samplingProbability);
}
}

View File

@ -7,7 +7,6 @@ package io.opentelemetry.contrib.sampler.consistent56;
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 abstract class ConsistentSampler implements Sampler {
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 abstract class ConsistentSampler implements Sampler {
// 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();

View File

@ -10,7 +10,9 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue;
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 @@ public final class ConsistentSamplingUtil {
*/
public static double calculateSamplingProbability(long threshold) {
checkThreshold(threshold);
return threshold * 0x1p-56;
return (MAX_THRESHOLD - threshold) * 0x1p-56;
}
/**
@ -41,30 +43,18 @@ public final class ConsistentSamplingUtil {
*/
public static long calculateThreshold(double samplingProbability) {
checkProbability(samplingProbability);
return Math.round(samplingProbability * 0x1p56);
return MAX_THRESHOLD - Math.round(samplingProbability * 0x1p56);
}
/**
* Calculates the adjusted count from a given threshold.
*
* <p>Returns 1, if the threshold is invalid.
*
* <p>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.
*
* @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;
}
} else {
return 1.;
}
checkThreshold(threshold);
return 0x1p56 / (MAX_THRESHOLD - threshold);
}
/**
@ -93,6 +83,10 @@ public final class ConsistentSamplingUtil {
return MAX_RANDOM_VALUE;
}
public static long getMinThreshold() {
return MIN_THRESHOLD;
}
public static long getMaxThreshold() {
return MAX_THRESHOLD;
}
@ -102,7 +96,7 @@ public final class ConsistentSamplingUtil {
}
public static boolean isValidThreshold(long threshold) {
return 0 <= threshold && threshold <= getMaxThreshold();
return getMinThreshold() <= threshold && threshold <= getMaxThreshold();
}
public static boolean isValidProbability(double probability) {

View File

@ -21,11 +21,15 @@ public class ConsistentAlwaysOffSamplerTest {
@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());
}
}

View File

@ -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 @@ public class ConsistentAlwaysOnSamplerTest {
@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());
}
}

View File

@ -63,12 +63,8 @@ public class ConsistentFixedThresholdSamplerTest {
.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 class ConsistentFixedThresholdSamplerTest {
@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}");
}
}

View File

@ -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 @@ class ConsistentSamplerTest {
}
@Test
void testMaxThresholdWithoutParentRandomValue() {
void testMinThresholdWithoutParentRandomValue() {
Input input = new Input();
@ -168,13 +169,13 @@ class ConsistentSamplerTest {
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 @@ class ConsistentSamplerTest {
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 @@ class ConsistentSamplerTest {
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 @@ class ConsistentSamplerTest {
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();
}
}

View File

@ -10,6 +10,8 @@ import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUt
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 @@ public class ConsistentSamplingUtilTest {
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,15 @@ public class ConsistentSamplingUtilTest {
@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() - 1)).isEqualTo(0x1p56);
assertThat(calculateAdjustedCount(getMaxThreshold())).isInfinite();
assertThatIllegalArgumentException()
.isThrownBy(() -> calculateAdjustedCount(getMinThreshold() - 1));
assertThatIllegalArgumentException()
.isThrownBy(() -> calculateAdjustedCount(getMaxThreshold() + 1));
}
@Test