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

Assume random trace ID and set th-field only for sampled spans #1278

Merged
merged 1 commit into from
May 8, 2024
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 @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() + '}';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}

/**
Expand All @@ -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();
}

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

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

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

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

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

/**
Expand All @@ -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
Expand All @@ -225,22 +152,15 @@ 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);

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

Expand Down
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.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;
Expand All @@ -28,7 +28,6 @@
public class ConsistentFixedThresholdSamplerTest {

private Context parentContext;
private String traceId;
private String name;
private SpanKind spanKind;
private Attributes attributes;
Expand All @@ -38,7 +37,6 @@ public class ConsistentFixedThresholdSamplerTest {
public void init() {

parentContext = Context.root();
traceId = "0123456789abcdef0123456789abcdef";
name = "name";
spanKind = SpanKind.SERVER;
attributes = Attributes.empty();
Expand All @@ -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));

Expand Down
Loading
Loading