Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch from acceptance to rejection threshold #1130

Merged
merged 2 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ final class ConsistentAlwaysOffSampler extends ConsistentSampler {

@Override
protected long getThreshold(long parentThreshold, boolean isRoot) {
return 0;
return ConsistentSamplingUtil.getMaxThreshold();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -18,7 +18,7 @@ final class ConsistentAlwaysOnSampler extends ConsistentSampler {

@Override
protected long getThreshold(long parentThreshold, boolean isRoot) {
return getMaxThreshold();
return getMinThreshold();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,7 +31,7 @@ private ConsistentSamplingUtil() {}
*/
public static double calculateSamplingProbability(long threshold) {
checkThreshold(threshold);
return threshold * 0x1p-56;
return (MAX_THRESHOLD - threshold) * 0x1p-56;
}

/**
Expand All @@ -41,30 +43,18 @@ 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);
}

/**
* 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);
}

/**
Expand Down Expand Up @@ -93,6 +83,10 @@ public static long getMaxRandomValue() {
return MAX_RANDOM_VALUE;
}

public static long getMinThreshold() {
return MIN_THRESHOLD;
}

public static long getMaxThreshold() {
return MAX_THRESHOLD;
}
Expand All @@ -102,7 +96,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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}");
}
}
Loading
Loading