Update consistent sampling to SDK incubator ComposableSampler API#2785
Update consistent sampling to SDK incubator ComposableSampler API#2785trask wants to merge 2 commits into
Conversation
1c52555 to
2bd5c3d
Compare
| @@ -5,17 +5,26 @@ | |||
|
|
|||
There was a problem hiding this comment.
Copied from consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSampler.java.
GitHub shows this as a modification of the old consistent file; this is the smaller diff against the consistent56 source used for the mechanical port.
index 0075c569..1dbc26d3 100644
--- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSampler.java
+++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentRateLimitingSampler.java
@@ -3,28 +3,28 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.contrib.sampler.consistent56;
+package io.opentelemetry.contrib.sampler.consistent;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.calculateSamplingProbability;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.calculateThreshold;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getInvalidThreshold;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.isValidThreshold;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.calculateSamplingProbability;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.calculateThreshold;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.getInvalidThreshold;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.isValidThreshold;
import static java.util.Objects.requireNonNull;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.SpanKind;
-import io.opentelemetry.api.trace.TraceState;
import io.opentelemetry.context.Context;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.ComposableSampler;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.SamplingIntent;
import io.opentelemetry.sdk.trace.data.LinkData;
-import io.opentelemetry.sdk.trace.samplers.Sampler;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.LongSupplier;
import javax.annotation.concurrent.Immutable;
/**
- * This consistent {@link Sampler} adjusts the sampling probability dynamically to limit the rate of
- * sampled spans.
+ * This consistent {@link ComposableSampler} adjusts the sampling probability dynamically to limit
+ * the rate of sampled spans.
*
* <p>This sampler uses exponential smoothing to estimate on irregular data (compare Wright, David
* J. "Forecasting data published at irregular time intervals using an extension of Holt's method."
@@ -93,7 +93,7 @@ import javax.annotation.concurrent.Immutable;
*
* <p>{@code 1 / (adaptationTimeSeconds * targetSpansPerSecondLimit)}
*/
-final class ConsistentRateLimitingSampler extends ConsistentSampler {
+final class ConsistentRateLimitingSampler implements ComposableSampler {
private static final double NANOS_IN_SECONDS = 1e-9;
@@ -122,7 +122,7 @@ final class ConsistentRateLimitingSampler extends ConsistentSampler {
private final double targetSpansPerNanosecondLimit;
private final double probabilitySmoothingFactor;
private final AtomicReference<State> state;
- private final Composable delegate;
+ private final ComposableSampler delegate;
/**
* Constructor.
@@ -133,7 +133,7 @@ final class ConsistentRateLimitingSampler extends ConsistentSampler {
* @param nanoTimeSupplier a supplier for the current nano time
*/
ConsistentRateLimitingSampler(
- Composable delegate,
+ ComposableSampler delegate,
double targetSpansPerSecondLimit,
double adaptationTimeSeconds,
LongSupplier nanoTimeSupplier) {
@@ -217,6 +217,7 @@ final class ConsistentRateLimitingSampler extends ConsistentSampler {
@Override
public SamplingIntent getSamplingIntent(
Context parentContext,
+ String traceId,
String name,
SpanKind spanKind,
Attributes attributes,
@@ -225,7 +226,7 @@ final class ConsistentRateLimitingSampler extends ConsistentSampler {
long suggestedThreshold;
SamplingIntent delegateIntent =
- delegate.getSamplingIntent(parentContext, name, spanKind, attributes, parentLinks);
+ delegate.getSamplingIntent(parentContext, traceId, name, spanKind, attributes, parentLinks);
long delegateThreshold = delegateIntent.getThreshold();
if (isValidThreshold(delegateThreshold)) {
@@ -249,27 +250,11 @@ final class ConsistentRateLimitingSampler extends ConsistentSampler {
suggestedThreshold = getInvalidThreshold();
}
- return new SamplingIntent() {
- @Override
- public long getThreshold() {
- return suggestedThreshold;
- }
-
- @Override
- public boolean isAdjustedCountReliable() {
- return delegateIntent.isAdjustedCountReliable();
- }
-
- @Override
- public Attributes getAttributes() {
- return delegateIntent.getAttributes();
- }
-
- @Override
- public TraceState updateTraceState(TraceState previousState) {
- return delegateIntent.updateTraceState(previousState);
- }
- };
+ return SamplingIntent.create(
+ suggestedThreshold,
+ delegateIntent.isThresholdReliable(),
+ delegateIntent.getAttributes(),
+ delegateIntent.getTraceStateUpdater());
}
@Override| @@ -5,12 +5,22 @@ | |||
|
|
|||
There was a problem hiding this comment.
Copied from consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSamplerTest.java.
GitHub shows this as a modification of the old consistent test; this is the smaller diff against the consistent56 source used for the mechanical port.
index d5cb6b64..0676565c 100644
--- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSamplerTest.java
+++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentRateLimitingSamplerTest.java
@@ -3,9 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.contrib.sampler.consistent56;
+package io.opentelemetry.contrib.sampler.consistent;
-import static io.opentelemetry.contrib.sampler.consistent56.TestUtil.generateRandomTraceId;
+import static io.opentelemetry.contrib.sampler.consistent.TestUtil.generateRandomTraceId;
import static org.assertj.core.api.Assertions.assertThat;
import io.opentelemetry.api.common.AttributeKey;
@@ -16,7 +16,11 @@ import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.api.trace.TraceFlags;
import io.opentelemetry.api.trace.TraceState;
import io.opentelemetry.context.Context;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.ComposableSampler;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.CompositeSampler;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.SamplingIntent;
import io.opentelemetry.sdk.trace.data.LinkData;
+import io.opentelemetry.sdk.trace.samplers.Sampler;
import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
import io.opentelemetry.sdk.trace.samplers.SamplingResult;
import java.util.ArrayList;
@@ -68,11 +72,12 @@ class ConsistentRateLimitingSamplerTest {
double targetSpansPerSecondLimit = 1000;
double adaptationTimeSeconds = 5;
- Composable delegate =
- new CoinFlipSampler(ConsistentSampler.alwaysOff(), ConsistentSampler.probabilityBased(0.8));
- ConsistentSampler sampler =
- ConsistentSampler.rateLimited(
- delegate, targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier);
+ ComposableSampler delegate =
+ new CoinFlipSampler(ComposableSampler.alwaysOff(), ComposableSampler.probability(0.8));
+ Sampler sampler =
+ CompositeSampler.wrap(
+ ConsistentSampler.rateLimited(
+ delegate, targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier));
long nanosBetweenSpans = TimeUnit.MICROSECONDS.toNanos(100);
int numSpans = 1000000;
@@ -109,11 +114,15 @@ class ConsistentRateLimitingSamplerTest {
double targetSpansPerSecondLimit = 1000;
double adaptationTimeSeconds = 5;
- Composable delegate =
- new CoinFlipSampler(ConsistentSampler.alwaysOff(), ConsistentSampler.probabilityBased(0.8));
- ConsistentSampler sampler =
- ConsistentSampler.rateLimited(
- delegate, targetSpansPerSecondLimit, adaptationTimeSeconds, lowResolutionTimeSupplier);
+ ComposableSampler delegate =
+ new CoinFlipSampler(ComposableSampler.alwaysOff(), ComposableSampler.probability(0.8));
+ Sampler sampler =
+ CompositeSampler.wrap(
+ ConsistentSampler.rateLimited(
+ delegate,
+ targetSpansPerSecondLimit,
+ adaptationTimeSeconds,
+ lowResolutionTimeSupplier));
long nanosBetweenSpans = TimeUnit.MICROSECONDS.toNanos(100);
int numSpans = 1000000;
@@ -150,9 +159,10 @@ class ConsistentRateLimitingSamplerTest {
double targetSpansPerSecondLimit = 1000;
double adaptationTimeSeconds = 5;
- ConsistentSampler sampler =
- ConsistentSampler.rateLimited(
- targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier);
+ Sampler sampler =
+ CompositeSampler.wrap(
+ ConsistentSampler.rateLimited(
+ targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier));
long nanosBetweenSpans1 = TimeUnit.MICROSECONDS.toNanos(100);
long nanosBetweenSpans2 = TimeUnit.MICROSECONDS.toNanos(10);
@@ -217,9 +227,10 @@ class ConsistentRateLimitingSamplerTest {
double targetSpansPerSecondLimit = 1000;
double adaptationTimeSeconds = 5;
- ConsistentSampler sampler =
- ConsistentSampler.rateLimited(
- targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier);
+ Sampler sampler =
+ CompositeSampler.wrap(
+ ConsistentSampler.rateLimited(
+ targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier));
long nanosBetweenSpans1 = TimeUnit.MICROSECONDS.toNanos(10);
long nanosBetweenSpans2 = TimeUnit.MICROSECONDS.toNanos(100);
@@ -309,14 +320,15 @@ class ConsistentRateLimitingSamplerTest {
double adaptationTimeSeconds = 5;
AttributeKey<String> key = AttributeKey.stringKey("category");
- Composable delegate =
+ ComposableSampler delegate =
new CoinFlipSampler(
- new MarkingSampler(ConsistentSampler.probabilityBased(0.6), key, "A"),
- new MarkingSampler(ConsistentSampler.probabilityBased(0.4), key, "B"));
+ new MarkingSampler(ComposableSampler.probability(0.6), key, "A"),
+ new MarkingSampler(ComposableSampler.probability(0.4), key, "B"));
- ConsistentSampler sampler =
- ConsistentSampler.rateLimited(
- delegate, targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier);
+ Sampler sampler =
+ CompositeSampler.wrap(
+ ConsistentSampler.rateLimited(
+ delegate, targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier));
long averageRequestRatePerSecond = 10000;
int numSpans = 1000000;
@@ -372,12 +384,13 @@ class ConsistentRateLimitingSamplerTest {
double targetSpansPerSecondLimit = 1000;
double adaptationTimeSeconds = 5;
- Composable delegate =
- new CoinFlipSampler(ConsistentSampler.alwaysOff(), ConsistentSampler.alwaysOn());
+ ComposableSampler delegate =
+ new CoinFlipSampler(ComposableSampler.alwaysOff(), ComposableSampler.alwaysOn());
- ConsistentSampler sampler =
- ConsistentSampler.rateLimited(
- delegate, targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier);
+ Sampler sampler =
+ CompositeSampler.wrap(
+ ConsistentSampler.rateLimited(
+ delegate, targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier));
long averageRequestRatePerSecond = 10000;
int numSpans = 1000000;
@@ -418,36 +431,38 @@ class ConsistentRateLimitingSamplerTest {
// Assume the following setup:
// The root span is sampled by the legacy sampler AlwaysOn.
// One of its descendant spans, which we will call "parent" span, is sampled with
- // stage1: ConsistentRateLimitingSampler(ConsistentParentBasedSampler, 5000/s).
+ // stage1: ConsistentRateLimitingSampler(legacy-like root sampler, 5000/s).
// This will sample approximately 50% of the spans.
// Its "child" is similarly sampled by
- // stage2: ConsistentRateLimitingSampler(ConsistentParentBasedSampler, 2500/s).
+ // stage2: ConsistentRateLimitingSampler(parentThreshold(alwaysOff), 2500/s).
// This sampler will generate the same output as the root span described above:
// - the threshold will be 0, so all spans will be sampled
- // - isAdjustedCountReliable will be false
+ // - thresholdReliable will be false
// - there will be no threshold in TraceState, but the sampling flag will be set
- Composable mockRootSampler = new LegacyLikeComposable(ConsistentSampler.alwaysOn());
+ ComposableSampler mockRootSampler = new LegacyLikeComposable(ComposableSampler.alwaysOn());
double targetSpansPerSecondLimit = 2500; // for stage2
double adaptationTimeSeconds = 5;
// The sampler for "parent" spans
- ConsistentSampler stage1 =
- ConsistentSampler.rateLimited(
- mockRootSampler,
- 2 * targetSpansPerSecondLimit,
- adaptationTimeSeconds,
- nanoTimeSupplier);
+ Sampler stage1 =
+ CompositeSampler.wrap(
+ ConsistentSampler.rateLimited(
+ mockRootSampler,
+ 2 * targetSpansPerSecondLimit,
+ adaptationTimeSeconds,
+ nanoTimeSupplier));
// The sampler for "child" spans (it will never see root spans)
- ConsistentSampler stage2 =
- ConsistentSampler.rateLimited(
- ConsistentSampler.parentBased(ConsistentSampler.alwaysOff()),
- targetSpansPerSecondLimit,
- adaptationTimeSeconds,
- nanoTimeSupplier);
+ Sampler stage2 =
+ CompositeSampler.wrap(
+ ConsistentSampler.rateLimited(
+ ComposableSampler.parentThreshold(ComposableSampler.alwaysOff()),
+ targetSpansPerSecondLimit,
+ adaptationTimeSeconds,
+ nanoTimeSupplier));
int numSpans = 1000000;
int stage1SampledCount = 0;
@@ -506,47 +521,33 @@ class ConsistentRateLimitingSamplerTest {
* An auxiliary class used to simulate the behavior of a legacy (non consistent-probability)
* sampler, just for testing mixed environment
*/
- static class LegacyLikeComposable implements Composable {
+ static class LegacyLikeComposable implements ComposableSampler {
- private final Composable delegate;
+ private final ComposableSampler delegate;
- public LegacyLikeComposable(Composable delegate) {
+ public LegacyLikeComposable(ComposableSampler delegate) {
this.delegate = delegate;
}
@Override
public SamplingIntent getSamplingIntent(
Context parentContext,
+ String traceId,
String name,
SpanKind spanKind,
Attributes attributes,
List<LinkData> parentLinks) {
SamplingIntent delegateIntent =
- delegate.getSamplingIntent(parentContext, name, spanKind, attributes, parentLinks);
-
- return new SamplingIntent() {
- @Override
- public long getThreshold() {
- return delegateIntent.getThreshold();
- }
-
- @Override
- public boolean isAdjustedCountReliable() {
- // Forcing "legacy" behavior, no threshold will be put into TraceState
- return false;
- }
-
- @Override
- public Attributes getAttributes() {
- return delegateIntent.getAttributes();
- }
-
- @Override
- public TraceState updateTraceState(TraceState previousState) {
- return delegateIntent.updateTraceState(previousState);
- }
- };
+ delegate.getSamplingIntent(
+ parentContext, traceId, name, spanKind, attributes, parentLinks);
+
+ // Forcing "legacy" behavior, no threshold will be put into TraceState
+ return SamplingIntent.create(
+ delegateIntent.getThreshold(),
+ /* thresholdReliable= */ false,
+ delegateIntent.getAttributes(),
+ delegateIntent.getTraceStateUpdater());
}
@Override
@@ -560,7 +561,7 @@ class ConsistentRateLimitingSamplerTest {
double targetSpansPerSecondLimit = 123.456;
double adaptationTimeSeconds = 7.89;
- ConsistentSampler sampler =
+ ComposableSampler sampler =
ConsistentSampler.rateLimited(targetSpansPerSecondLimit, adaptationTimeSeconds);
assertThat(sampler.getDescription())| @@ -5,401 +5,94 @@ | |||
|
|
|||
There was a problem hiding this comment.
Copied from consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java.
GitHub shows this as a large modification of the old consistent factory; this is the more relevant diff against the consistent56 factory.
index 22ee83b8..90fa4c58 100644
--- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java
+++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentSampler.java
@@ -3,153 +3,69 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.contrib.sampler.consistent56;
+package io.opentelemetry.contrib.sampler.consistent;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getInvalidRandomValue;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.isValidThreshold;
-
-import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.api.trace.SpanContext;
-import io.opentelemetry.api.trace.SpanKind;
-import io.opentelemetry.api.trace.TraceState;
-import io.opentelemetry.context.Context;
-import io.opentelemetry.sdk.trace.data.LinkData;
-import io.opentelemetry.sdk.trace.samplers.Sampler;
-import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
-import io.opentelemetry.sdk.trace.samplers.SamplingResult;
-import java.util.List;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.ComposableSampler;
import java.util.function.LongSupplier;
-import javax.annotation.Nullable;
-
-/** Abstract base class for consistent samplers. */
-@SuppressWarnings("InconsistentOverloads")
-public abstract class ConsistentSampler implements Sampler, Composable {
-
- /**
- * Returns a {@link ConsistentSampler} that samples all spans.
- *
- * @return a sampler
- */
- public static ConsistentSampler alwaysOn() {
- return ConsistentAlwaysOnSampler.getInstance();
- }
-
- /**
- * Returns a {@link ConsistentSampler} that does not sample any span.
- *
- * @return a sampler
- */
- public static ConsistentSampler alwaysOff() {
- return ConsistentAlwaysOffSampler.getInstance();
- }
- /**
- * Returns a {@link ConsistentSampler} that samples each span with a fixed probability.
- *
- * @param samplingProbability the sampling probability
- * @return a sampler
- */
- public static ConsistentSampler probabilityBased(double samplingProbability) {
- long threshold = ConsistentSamplingUtil.calculateThreshold(samplingProbability);
- return new ConsistentFixedThresholdSampler(threshold);
- }
-
- /**
- * Returns a {@link ConsistentSampler} that samples each span with a known probability, where the
- * probablity can be dynamically updated.
- *
- * @param samplingProbability the sampling probability
- * @return a sampler
- */
- public static ConsistentSampler updateableProbabilityBased(double samplingProbability) {
- return new ConsistentVariableThresholdSampler(samplingProbability);
- }
-
- /**
- * 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
- */
- public static ConsistentSampler parentBased(Composable rootSampler) {
- return new ConsistentParentBasedSampler(rootSampler);
- }
-
- /**
- * Constructs a new consistent rule based sampler using the given sequence of Predicates and
- * delegate Samplers.
- *
- * @param spanKindToMatch the SpanKind for which the Sampler applies, null value indicates all
- * SpanKinds
- * @param samplers the PredicatedSamplers to evaluate and query
- */
- public static ConsistentRuleBasedSampler ruleBased(
- @Nullable SpanKind spanKindToMatch, PredicatedSampler... samplers) {
- return new ConsistentRuleBasedSampler(spanKindToMatch, samplers);
- }
+/**
+ * Factory entry points for the contrib-only consistent probability samplers that are not part of
+ * the upstream {@link ComposableSampler} API.
+ *
+ * <ul>
+ * <li>{@link ConsistentRateLimitingSampler} — adaptive rate limiting
+ * <li>{@link ConsistentVariableThresholdSampler} — fixed probability that can be updated at
+ * runtime
+ * <li>{@link ConsistentAnyOf} — the minimum-threshold combination of several composable
+ * samplers
+ * </ul>
+ *
+ * <p>For the common samplers (always-on/off, fixed probability, parent-based, rule-based,
+ * annotating) use {@link ComposableSampler}'s static factories directly. To turn a {@link
+ * ComposableSampler} into a {@link io.opentelemetry.sdk.trace.samplers.Sampler} use {@link
+ * io.opentelemetry.sdk.extension.incubator.trace.samplers.CompositeSampler#wrap(ComposableSampler)}.
+ */
+public final class ConsistentSampler {
/**
- * 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)
+ * Returns a {@link ComposableSampler} that attempts to adjust the sampling probability
+ * dynamically to meet the target span rate. Spans are first passed to {@link
+ * ComposableSampler#alwaysOn()} and then rate-limited.
*/
- static ConsistentSampler rateLimited(
+ static ComposableSampler rateLimited(
double targetSpansPerSecondLimit, double adaptationTimeSeconds) {
- return rateLimited(alwaysOn(), targetSpansPerSecondLimit, adaptationTimeSeconds);
+ return rateLimited(
+ ComposableSampler.alwaysOn(), targetSpansPerSecondLimit, adaptationTimeSeconds);
}
/**
- * Returns a new {@link ConsistentSampler} that honors the delegate sampling decision as long as
- * it seems to meet the target span rate. In case the delegate sampling rate seems to exceed the
- * target, the sampler attempts to decrease the effective sampling probability dynamically to meet
- * the target span rate.
- *
- * @param delegate the delegate sampler
- * @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)
+ * Returns a {@link ComposableSampler} that honors the delegate's sampling decision as long as it
+ * seems to meet the target span rate. In case the delegate's sampling rate seems to exceed the
+ * target, the sampler attempts to decrease the effective sampling probability dynamically.
*/
- public static ConsistentSampler rateLimited(
- Composable delegate, double targetSpansPerSecondLimit, double adaptationTimeSeconds) {
+ @SuppressWarnings("InconsistentOverloads")
+ public static ComposableSampler rateLimited(
+ ComposableSampler delegate, double targetSpansPerSecondLimit, double adaptationTimeSeconds) {
return rateLimited(
delegate, targetSpansPerSecondLimit, adaptationTimeSeconds, System::nanoTime);
}
- /**
- * 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 nanoTimeSupplier a supplier for the current nano time
- */
- static ConsistentSampler rateLimited(
+ // Package-private overloads exposing the nanoTimeSupplier for tests.
+
+ static ComposableSampler rateLimited(
double targetSpansPerSecondLimit,
double adaptationTimeSeconds,
LongSupplier nanoTimeSupplier) {
return rateLimited(
- alwaysOn(), targetSpansPerSecondLimit, adaptationTimeSeconds, nanoTimeSupplier);
+ ComposableSampler.alwaysOn(),
+ targetSpansPerSecondLimit,
+ adaptationTimeSeconds,
+ nanoTimeSupplier);
}
- /**
- * Returns a new {@link ConsistentSampler} that honors the delegate sampling decision as long as
- * it seems to meet the target span rate. In case the delegate sampling rate seems to exceed the
- * target, the sampler attempts to decrease the effective sampling probability dynamically to meet
- * the target span rate.
- *
- * @param delegate the delegate sampler
- * @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 nanoTimeSupplier a supplier for the current nano time
- */
- static ConsistentSampler rateLimited(
- Composable delegate,
+ @SuppressWarnings("InconsistentOverloads")
+ static ComposableSampler rateLimited(
+ ComposableSampler delegate,
double targetSpansPerSecondLimit,
double adaptationTimeSeconds,
LongSupplier nanoTimeSupplier) {
@@ -158,100 +74,25 @@ public abstract class ConsistentSampler implements Sampler, Composable {
}
/**
- * Returns a {@link ConsistentSampler} that queries its delegate Samplers for their sampling
- * threshold before determining what threshold to use. The intention is to make a positive
- * sampling decision if any of the delegates would make a positive decision.
- *
- * <p>The returned sampler takes care of setting the trace state correctly, which would not happen
- * if the {@link #shouldSample(Context, String, String, SpanKind, Attributes, List)} method was
- * called for each sampler individually. Also, the combined sampler is more efficient than
- * evaluating the samplers individually and combining the results afterwards.
+ * Returns a {@link ComposableSampler} with a fixed sampling probability that can be updated at
+ * runtime via {@link ConsistentVariableThresholdSampler#setSamplingProbability(double)}.
+ */
+ public static ConsistentVariableThresholdSampler updateableProbabilityBased(
+ double samplingProbability) {
+ return new ConsistentVariableThresholdSampler(samplingProbability);
+ }
+
+ /**
+ * Returns a {@link ComposableSampler} that queries all its delegates for their sampling
+ * threshold. The intention is to make a positive sampling decision if any of the delegates would
+ * make a positive decision. The returned sampler uses the minimum threshold value found among all
+ * delegates.
*
* @param delegates the delegate samplers, at least one delegate must be specified
- * @return the ConsistentAnyOf sampler
*/
- public static ConsistentSampler anyOf(Composable... delegates) {
+ public static ComposableSampler anyOf(ComposableSampler... delegates) {
return new ConsistentAnyOf(delegates);
}
- @Override
- public final SamplingResult shouldSample(
- Context parentContext,
- String traceId,
- String name,
- SpanKind spanKind,
- Attributes attributes,
- List<LinkData> parentLinks) {
- Span parentSpan = Span.fromContext(parentContext);
- SpanContext parentSpanContext = parentSpan.getSpanContext();
-
- TraceState parentTraceState = parentSpanContext.getTraceState();
- String otelTraceStateString = parentTraceState.get(OtelTraceState.TRACE_STATE_KEY);
- OtelTraceState otelTraceState = OtelTraceState.parse(otelTraceStateString);
-
- SamplingIntent intent =
- getSamplingIntent(parentContext, name, spanKind, attributes, parentLinks);
- long threshold = intent.getThreshold();
-
- // determine sampling decision
- boolean isSampled;
- boolean isAdjustedCountCorrect;
- if (isValidThreshold(threshold)) {
- isAdjustedCountCorrect = intent.isAdjustedCountReliable();
- // determine the randomness value to use
- long randomness;
- if (isAdjustedCountCorrect) {
- randomness = getRandomness(otelTraceState, traceId);
- } else {
- // We cannot assume any particular distribution of the provided trace randomness,
- // because the sampling decision may depend directly or indirectly on the randomness value;
- // however, we still want to sample with probability corresponding to the obtained threshold
- randomness = RandomValueGenerators.getDefault().generate(traceId);
- }
- isSampled = threshold <= randomness;
- } else { // invalid threshold, DROP
- isSampled = false;
- isAdjustedCountCorrect = false;
- }
-
- SamplingDecision samplingDecision =
- isSampled ? SamplingDecision.RECORD_AND_SAMPLE : SamplingDecision.DROP;
-
- // determine tracestate changes
- if (isSampled && isAdjustedCountCorrect) {
- otelTraceState.setThreshold(threshold);
- } else {
- otelTraceState.invalidateThreshold();
- }
-
- String newOtTraceState = otelTraceState.serialize();
-
- return new SamplingResult() {
-
- @Override
- public SamplingDecision getDecision() {
- return samplingDecision;
- }
-
- @Override
- public Attributes getAttributes() {
- return intent.getAttributes();
- }
-
- @Override
- public TraceState getUpdatedTraceState(TraceState parentTraceState) {
- return intent.updateTraceState(parentTraceState).toBuilder()
- .put(OtelTraceState.TRACE_STATE_KEY, newOtTraceState)
- .build();
- }
- };
- }
-
- private static long getRandomness(OtelTraceState otelTraceState, String traceId) {
- if (otelTraceState.hasValidRandomValue()) {
- return otelTraceState.getRandomValue();
- } else {
- return OtelTraceState.parseHex(traceId, 18, 14, getInvalidRandomValue());
- }
- }
+ private ConsistentSampler() {}
}| @@ -5,73 +5,24 @@ | |||
|
|
|||
There was a problem hiding this comment.
Copied from consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/TestUtil.java.
GitHub shows this against the old consistent helper; this is the source diff against the consistent56 helper.
index 1ffede0a..418322bd 100644
--- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/TestUtil.java
+++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/TestUtil.java
@@ -3,9 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.contrib.sampler.consistent56;
+package io.opentelemetry.contrib.sampler.consistent;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.HEX_DIGITS;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.HEX_DIGITS;
import java.util.SplittableRandom;| @@ -0,0 +1,77 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/TestUtil.java.
GitHub shows this as an added file; this is the source diff against the old consistent helper that contained the reservoir-only statistical assertion.
index d1ed0643..5b8d839b 100644
--- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/TestUtil.java
+++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentReservoirSamplingSpanProcessorTestUtil.java
@@ -11,9 +11,9 @@ import java.util.HashMap;
import java.util.Map;
import org.hipparchus.stat.inference.GTest;
-public final class TestUtil {
+public final class ConsistentReservoirSamplingSpanProcessorTestUtil {
- private TestUtil() {}
+ private ConsistentReservoirSamplingSpanProcessorTestUtil() {}
public static void verifyObservedPvaluesUsingGtest(
long originalNumberOfSpans, Map<Integer, Long> observedPvalues, double samplingProbability) {| @@ -0,0 +1,85 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentVariableThresholdSampler.java.
GitHub shows this as an added file; this is the source diff against the consistent56 implementation.
index 9d356d49..5515e165 100644
--- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentVariableThresholdSampler.java
+++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentVariableThresholdSampler.java
@@ -3,14 +3,24 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.contrib.sampler.consistent56;
+package io.opentelemetry.contrib.sampler.consistent;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.calculateSamplingProbability;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.calculateThreshold;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.checkThreshold;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getMaxThreshold;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.calculateSamplingProbability;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.calculateThreshold;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.checkThreshold;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.getInvalidThreshold;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.getMaxThreshold;
-public class ConsistentVariableThresholdSampler extends ConsistentThresholdSampler {
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.ComposableSampler;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.SamplingIntent;
+import io.opentelemetry.sdk.trace.data.LinkData;
+import java.util.List;
+import java.util.function.Function;
+
+public class ConsistentVariableThresholdSampler implements ComposableSampler {
private volatile long threshold;
private volatile String description = "";
@@ -19,12 +29,27 @@ public class ConsistentVariableThresholdSampler extends ConsistentThresholdSampl
updateSamplingProbability(samplingProbability);
}
+ @Override
+ public SamplingIntent getSamplingIntent(
+ Context parentContext,
+ String traceId,
+ String name,
+ SpanKind spanKind,
+ Attributes attributes,
+ List<LinkData> parentLinks) {
+ long threshold = this.threshold;
+ if (threshold == getMaxThreshold()) {
+ return SamplingIntent.create(
+ getInvalidThreshold(), false, Attributes.empty(), Function.identity());
+ }
+ return SamplingIntent.create(threshold, true, Attributes.empty(), Function.identity());
+ }
+
@Override
public String getDescription() {
return description;
}
- @Override
public long getThreshold() {
return threshold;
}| @@ -0,0 +1,130 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAnyOfTest.java.
GitHub shows this as an added file; this is the source diff against the consistent56 test.
index 873ed04d..55acc2d4 100644
--- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAnyOfTest.java
+++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentAnyOfTest.java
@@ -3,49 +3,44 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.contrib.sampler.consistent56;
+package io.opentelemetry.contrib.sampler.consistent;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getInvalidThreshold;
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.getInvalidThreshold;
import static org.assertj.core.api.Assertions.assertThat;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.context.Context;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.ComposableSampler;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.SamplingIntent;
import io.opentelemetry.sdk.trace.data.LinkData;
import java.util.List;
+import java.util.function.Function;
import org.junit.jupiter.api.Test;
class ConsistentAnyOfTest {
- static class TestSampler implements Composable {
+ static class TestSampler implements ComposableSampler {
private final long threshold;
- private final boolean isAdjustedCountCorrect;
+ private final boolean thresholdReliable;
- public TestSampler(long threshold, boolean isAdjustedCountCorrect) {
+ public TestSampler(long threshold, boolean thresholdReliable) {
this.threshold = threshold;
- this.isAdjustedCountCorrect = isAdjustedCountCorrect;
+ this.thresholdReliable = thresholdReliable;
}
@Override
public SamplingIntent getSamplingIntent(
Context parentContext,
+ String traceId,
String name,
SpanKind spanKind,
Attributes attributes,
List<LinkData> parentLinks) {
- return new SamplingIntent() {
- @Override
- public long getThreshold() {
- return threshold;
- }
-
- @Override
- public boolean isAdjustedCountReliable() {
- return isAdjustedCountCorrect;
- }
- };
+ return SamplingIntent.create(
+ threshold, thresholdReliable, Attributes.empty(), Function.identity());
}
@Override
@@ -55,43 +50,47 @@ class ConsistentAnyOfTest {
}
@Test
- void testMinimumThresholdWithAdjustedCount() {
- Composable delegate1 = new TestSampler(0x80000000000000L, /* isAdjustedCountCorrect= */ false);
- Composable delegate2 = new TestSampler(0x30000000000000L, /* isAdjustedCountCorrect= */ true);
- Composable delegate3 = new TestSampler(0xa0000000000000L, /* isAdjustedCountCorrect= */ false);
- Composable delegate4 = new TestSampler(0x30000000000000L, /* isAdjustedCountCorrect= */ false);
-
- Composable sampler = ConsistentSampler.anyOf(delegate1, delegate2, delegate3, delegate4);
- SamplingIntent intent = sampler.getSamplingIntent(null, "span_name", null, null, null);
+ void testMinimumThresholdReliable() {
+ ComposableSampler delegate1 =
+ new TestSampler(0x80000000000000L, /* thresholdReliable= */ false);
+ ComposableSampler delegate2 = new TestSampler(0x30000000000000L, /* thresholdReliable= */ true);
+ ComposableSampler delegate3 =
+ new TestSampler(0xa0000000000000L, /* thresholdReliable= */ false);
+ ComposableSampler delegate4 =
+ new TestSampler(0x30000000000000L, /* thresholdReliable= */ false);
+
+ ComposableSampler sampler = ConsistentSampler.anyOf(delegate1, delegate2, delegate3, delegate4);
+ SamplingIntent intent = sampler.getSamplingIntent(null, "tid", "span_name", null, null, null);
assertThat(intent.getThreshold()).isEqualTo(0x30000000000000L);
- assertThat(intent.isAdjustedCountReliable()).isTrue();
+ assertThat(intent.isThresholdReliable()).isTrue();
// Change the delegate order
sampler = ConsistentSampler.anyOf(delegate1, delegate4, delegate3, delegate2);
- intent = sampler.getSamplingIntent(null, "span_name", null, null, null);
+ intent = sampler.getSamplingIntent(null, "tid", "span_name", null, null, null);
assertThat(intent.getThreshold()).isEqualTo(0x30000000000000L);
- assertThat(intent.isAdjustedCountReliable()).isTrue();
+ assertThat(intent.isThresholdReliable()).isTrue();
}
@Test
- void testMinimumThresholdWithoutAdjustedCount() {
- Composable delegate1 = new TestSampler(0x80000000000000L, /* isAdjustedCountCorrect= */ true);
- Composable delegate2 = new TestSampler(0x30000000000000L, /* isAdjustedCountCorrect= */ false);
- Composable delegate3 = new TestSampler(0xa0000000000000L, /* isAdjustedCountCorrect= */ true);
-
- Composable sampler = ConsistentSampler.anyOf(delegate1, delegate2, delegate3);
- SamplingIntent intent = sampler.getSamplingIntent(null, "span_name", null, null, null);
+ void testMinimumThresholdUnreliable() {
+ ComposableSampler delegate1 = new TestSampler(0x80000000000000L, /* thresholdReliable= */ true);
+ ComposableSampler delegate2 =
+ new TestSampler(0x30000000000000L, /* thresholdReliable= */ false);
+ ComposableSampler delegate3 = new TestSampler(0xa0000000000000L, /* thresholdReliable= */ true);
+
+ ComposableSampler sampler = ConsistentSampler.anyOf(delegate1, delegate2, delegate3);
+ SamplingIntent intent = sampler.getSamplingIntent(null, "tid", "span_name", null, null, null);
assertThat(intent.getThreshold()).isEqualTo(0x30000000000000L);
- assertThat(intent.isAdjustedCountReliable()).isFalse();
+ assertThat(intent.isThresholdReliable()).isFalse();
}
@Test
void testAlwaysDrop() {
- Composable delegate1 = ConsistentSampler.alwaysOff();
- Composable sampler = ConsistentSampler.anyOf(delegate1);
- SamplingIntent intent = sampler.getSamplingIntent(null, "span_name", null, null, null);
+ ComposableSampler delegate1 = ComposableSampler.alwaysOff();
+ ComposableSampler sampler = ConsistentSampler.anyOf(delegate1);
+ SamplingIntent intent = sampler.getSamplingIntent(null, "tid", "span_name", null, null, null);
assertThat(intent.getThreshold()).isEqualTo(getInvalidThreshold());
- assertThat(intent.isAdjustedCountReliable()).isFalse();
+ assertThat(intent.isThresholdReliable()).isFalse();
}
@Test
@@ -99,29 +98,33 @@ class ConsistentAnyOfTest {
AttributeKey<String> key1 = AttributeKey.stringKey("tag1");
AttributeKey<String> key2 = AttributeKey.stringKey("tag2");
AttributeKey<String> key3 = AttributeKey.stringKey("tag3");
- Composable delegate1 =
- new MarkingSampler(new ConsistentFixedThresholdSampler(0x30000000000000L), key1, "a");
- Composable delegate2 =
- new MarkingSampler(new ConsistentFixedThresholdSampler(0x50000000000000L), key2, "b");
- Composable delegate3 = new MarkingSampler(ConsistentSampler.alwaysOff(), key3, "c");
- Composable sampler = ConsistentSampler.anyOf(delegate1, delegate2, delegate3);
- SamplingIntent intent = sampler.getSamplingIntent(null, "span_name", null, null, null);
+ ComposableSampler delegate1 =
+ new MarkingSampler(
+ new TestSampler(0x30000000000000L, /* thresholdReliable= */ true), key1, "a");
+ ComposableSampler delegate2 =
+ new MarkingSampler(
+ new TestSampler(0x50000000000000L, /* thresholdReliable= */ true), key2, "b");
+ ComposableSampler delegate3 = new MarkingSampler(ComposableSampler.alwaysOff(), key3, "c");
+ ComposableSampler sampler = ConsistentSampler.anyOf(delegate1, delegate2, delegate3);
+ SamplingIntent intent = sampler.getSamplingIntent(null, "tid", "span_name", null, null, null);
assertThat(intent.getAttributes().get(key1)).isEqualTo("a");
assertThat(intent.getAttributes().get(key2)).isEqualTo("b");
assertThat(intent.getAttributes().get(key3)).isEqualTo("c");
assertThat(intent.getThreshold()).isEqualTo(0x30000000000000L);
- assertThat(intent.isAdjustedCountReliable()).isTrue();
+ assertThat(intent.isThresholdReliable()).isTrue();
}
@Test
void testSpanAttributeOverride() {
AttributeKey<String> key1 = AttributeKey.stringKey("shared");
- Composable delegate1 =
- new MarkingSampler(new ConsistentFixedThresholdSampler(0x30000000000000L), key1, "a");
- Composable delegate2 =
- new MarkingSampler(new ConsistentFixedThresholdSampler(0x50000000000000L), key1, "b");
- Composable sampler = ConsistentSampler.anyOf(delegate1, delegate2);
- SamplingIntent intent = sampler.getSamplingIntent(null, "span_name", null, null, null);
+ ComposableSampler delegate1 =
+ new MarkingSampler(
+ new TestSampler(0x30000000000000L, /* thresholdReliable= */ true), key1, "a");
+ ComposableSampler delegate2 =
+ new MarkingSampler(
+ new TestSampler(0x50000000000000L, /* thresholdReliable= */ true), key1, "b");
+ ComposableSampler sampler = ConsistentSampler.anyOf(delegate1, delegate2);
+ SamplingIntent intent = sampler.getSamplingIntent(null, "tid", "span_name", null, null, null);
assertThat(intent.getAttributes().get(key1)).isEqualTo("b");
}
}| @@ -0,0 +1,147 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/UseCaseTest.java.
GitHub shows this as an added file; this is the source diff against the consistent56 test.
index 5a413247..62542fe7 100644
--- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/UseCaseTest.java
+++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/UseCaseTest.java
@@ -3,19 +3,21 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.contrib.sampler.consistent56;
-
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSampler.alwaysOff;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSampler.alwaysOn;
-import static io.opentelemetry.contrib.sampler.consistent56.ConsistentSamplingUtil.getInvalidThreshold;
-import static io.opentelemetry.contrib.sampler.consistent56.Predicate.anySpan;
-import static io.opentelemetry.contrib.sampler.consistent56.Predicate.isRootSpan;
-import static io.opentelemetry.contrib.sampler.consistent56.PredicatedSampler.onMatch;
+package io.opentelemetry.contrib.sampler.consistent;
+
+import static io.opentelemetry.contrib.sampler.consistent.ConsistentSamplingUtil.getInvalidThreshold;
import static org.assertj.core.api.Assertions.assertThat;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanContext;
import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.ComposableSampler;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.SamplingIntent;
+import io.opentelemetry.sdk.extension.incubator.trace.samplers.SamplingPredicate;
+import java.util.Collections;
import org.junit.jupiter.api.Test;
/**
@@ -38,14 +40,14 @@ class UseCaseTest {
//
// S = ConsistentRateLimiting(
// ConsistentAnyOf(
- // ConsistentParentBased(
- // ConsistentRuleBased(ROOT, {
- // (http.target == /healthcheck) => ConsistentAlwaysOff,
- // (http.target == /checkout) => ConsistentAlwaysOn,
- // true => ConsistentFixedThreshold(0.25)
+ // parentThreshold(
+ // ruleBased(ROOT, {
+ // (http.target == /healthcheck) => alwaysOff,
+ // (http.target == /checkout) => alwaysOn,
+ // true => probability(0.25)
// }),
- // ConsistentRuleBased(CLIENT, {
- // (http.url == /foo) => ConsistentAlwaysOn
+ // ruleBased(CLIENT, {
+ // (http.url == /foo) => alwaysOn
// }
// ),
// 1000.0
@@ -54,74 +56,91 @@ class UseCaseTest {
private static final AttributeKey<String> httpTarget = AttributeKey.stringKey("http.target");
private static final AttributeKey<String> httpUrl = AttributeKey.stringKey("http.url");
- private static ConsistentSampler buildSampler() {
- Predicate healthCheck =
- Predicate.and(
- isRootSpan(),
- (parentContext, name, spanKind, attributes, parentLinks) -> {
- return "/healthCheck".equals(attributes.get(httpTarget));
- });
- Predicate checkout =
- Predicate.and(
- isRootSpan(),
- (parentContext, name, spanKind, attributes, parentLinks) -> {
- return "/checkout".equals(attributes.get(httpTarget));
- });
- Composable s1 =
- ConsistentSampler.parentBased(
- ConsistentSampler.ruleBased(
- null,
- onMatch(healthCheck, alwaysOff()),
- onMatch(checkout, alwaysOn()),
- onMatch(anySpan(), ConsistentSampler.probabilityBased(0.25))));
- Predicate foo =
- (parentContext, name, spanKind, attributes, parentLinks) -> {
- return "/foo".equals(attributes.get(httpUrl));
+ private static ComposableSampler buildSampler() {
+ SamplingPredicate healthCheck =
+ (parentContext, traceId, name, spanKind, attributes, parentLinks) -> {
+ return isRootSpan(parentContext) && "/healthCheck".equals(attributes.get(httpTarget));
+ };
+ SamplingPredicate checkout =
+ (parentContext, traceId, name, spanKind, attributes, parentLinks) -> {
+ return isRootSpan(parentContext) && "/checkout".equals(attributes.get(httpTarget));
};
- Composable s2 = ConsistentSampler.ruleBased(SpanKind.CLIENT, onMatch(foo, alwaysOn()));
- Composable s3 = ConsistentSampler.anyOf(s1, s2);
+ ComposableSampler s1 =
+ ComposableSampler.parentThreshold(
+ ComposableSampler.ruleBasedBuilder()
+ .add(healthCheck, ComposableSampler.alwaysOff())
+ .add(checkout, ComposableSampler.alwaysOn())
+ .add(
+ (parentContext, traceId, name, spanKind, attributes, parentLinks) -> true,
+ ComposableSampler.probability(0.25))
+ .build());
+
+ SamplingPredicate foo =
+ (parentContext, traceId, name, spanKind, attributes, parentLinks) -> {
+ return spanKind == SpanKind.CLIENT && "/foo".equals(attributes.get(httpUrl));
+ };
+
+ ComposableSampler s2 =
+ ComposableSampler.ruleBasedBuilder().add(foo, ComposableSampler.alwaysOn()).build();
+ ComposableSampler s3 = ConsistentSampler.anyOf(s1, s2);
return ConsistentSampler.rateLimited(s3, 1000.0, 5, UseCaseTest::nanoTime);
}
@Test
void testDropHealthcheck() {
- ConsistentSampler s = buildSampler();
+ ComposableSampler s = buildSampler();
Attributes attributes = createAttributes(httpTarget, "/healthCheck");
- SamplingIntent intent = s.getSamplingIntent(null, "A", SpanKind.SERVER, attributes, null);
+ SamplingIntent intent =
+ s.getSamplingIntent(
+ Context.root(), "A", "span_name", SpanKind.SERVER, attributes, Collections.emptyList());
assertThat(intent.getThreshold()).isEqualTo(getInvalidThreshold());
}
@Test
void testSampleCheckout() {
- ConsistentSampler s = buildSampler();
+ ComposableSampler s = buildSampler();
advanceTime(1000000);
Attributes attributes = createAttributes(httpTarget, "/checkout");
- SamplingIntent intent = s.getSamplingIntent(null, "B", SpanKind.SERVER, attributes, null);
+ SamplingIntent intent =
+ s.getSamplingIntent(
+ Context.root(), "B", "span_name", SpanKind.SERVER, attributes, Collections.emptyList());
assertThat(intent.getThreshold()).isEqualTo(0L);
advanceTime(1000); // rate limiting should kick in
- intent = s.getSamplingIntent(null, "B", SpanKind.SERVER, attributes, null);
+ intent =
+ s.getSamplingIntent(
+ Context.root(), "B", "span_name", SpanKind.SERVER, attributes, Collections.emptyList());
assertThat(intent.getThreshold()).isGreaterThan(0L);
}
@Test
void testSampleClient() {
- ConsistentSampler s = buildSampler();
+ ComposableSampler s = buildSampler();
advanceTime(1000000);
Attributes attributes = createAttributes(httpUrl, "/foo");
- SamplingIntent intent = s.getSamplingIntent(null, "C", SpanKind.CLIENT, attributes, null);
+ SamplingIntent intent =
+ s.getSamplingIntent(
+ Context.root(), "C", "span_name", SpanKind.CLIENT, attributes, Collections.emptyList());
assertThat(intent.getThreshold()).isEqualTo(0L);
}
@Test
void testOtherRoot() {
- ConsistentSampler s = buildSampler();
+ ComposableSampler s = buildSampler();
advanceTime(1000000);
Attributes attributes = Attributes.empty();
- SamplingIntent intent = s.getSamplingIntent(null, "D", SpanKind.SERVER, attributes, null);
+ SamplingIntent intent =
+ s.getSamplingIntent(
+ Context.root(), "D", "span_name", SpanKind.SERVER, attributes, Collections.emptyList());
assertThat(intent.getThreshold()).isEqualTo(0xc0000000000000L);
}
+ private static boolean isRootSpan(Context parentContext) {
+ Span parentSpan = Span.fromContext(parentContext);
+ SpanContext parentSpanContext = parentSpan.getSpanContext();
+ return !parentSpanContext.isValid();
+ }
+
private static Attributes createAttributes(AttributeKey<String> key, String value) {
return Attributes.builder().put(key, value).build();
}2bd5c3d to
ef070cf
Compare
ef070cf to
b42b0e2
Compare
There was a problem hiding this comment.
Pull request overview
Rebases the consistent sampling module onto the OpenTelemetry SDK incubator composable-samplers API, consolidating the prior consistent and consistent56 implementations into a single io.opentelemetry.contrib.sampler.consistent package while retaining contrib-only samplers (rate limiting, updateable probability, any-of) and reservoir sampling support.
Changes:
- Replace contrib-defined composable sampler APIs with SDK incubator
ComposableSampler/CompositeSamplerbuilding blocks and update the autoconfigure provider accordingly. - Collapse/relocate implementations and tests from
consistent56intoconsistent, removing superseded classes. - Update docs/changelog and adjust tests/utilities for the new API shape.
Reviewed changes
Copilot reviewed 56 out of 56 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/UseCaseTest.java | Removed legacy consistent56 use-case test after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/TestUtil.java | Removed consistent56 test utility after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/RandomValueGeneratorsTest.java | Removed consistent56 RNG test after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/OtelTraceStateTest.java | Removed consistent56 trace-state test after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSamplerTest.java | Removed consistent56 sampler tests after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSamplerTest.java | Removed consistent56 rule-based tests after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSamplerTest.java | Removed consistent56 rate limiting tests after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSamplerTest.java | Removed consistent56 fixed-threshold tests after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAnyOfTest.java | Removed consistent56 any-of tests after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSamplerTest.java | Removed consistent56 always-on test after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSamplerTest.java | Removed consistent56 always-off test after consolidation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/UseCaseTest.java | Added new use-case test using incubator composable sampler APIs. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/TestUtil.java | Repurposed test util to only provide random trace-id generation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/MarkingSampler.java | Updated test helper to implement incubator ComposableSampler and new SamplingIntent. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentVariableThresholdSamplerTest.java | Moved/updated test to new package and utilities. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentSamplingUtilTest.java | Updated package/imports to consolidated consistent module. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentSamplerTest.java | Removed legacy p/r-based sampler tests as those implementations were removed. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentReservoirSamplingSpanProcessorTestUtil.java | Added dedicated test util for G-test verification after TestUtil cleanup. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentReservoirSamplingSpanProcessorTest.java | Updated reservoir sampling tests for new sampler wiring and utility extraction. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentRateLimitingSamplerTest.java | Updated rate limiting tests to use incubator composable samplers + CompositeSampler.wrap. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentProbabilityBasedSamplerTest.java | Removed legacy probability-based sampler test (implementation removed). |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/ConsistentAnyOfTest.java | Added any-of tests targeting the new ComposableSampler-based implementation. |
| consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/CoinFlipSampler.java | Updated test helper to implement incubator ComposableSampler signature. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/SamplingIntent.java | Removed contrib-defined SamplingIntent (replaced by incubator API). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/RandomValueGenerators.java | Removed consistent56 RNG utilities (replaced by incubator building blocks). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/RandomValueGenerator.java | Removed consistent56 random value generator API. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/PredicatedSampler.java | Removed consistent56 predicate pairing utility. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/Predicate.java | Removed consistent56 predicate API (replaced by incubator predicate/rule-based APIs). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/OtelTraceState.java | Removed consistent56 threshold-based tracestate implementation after consolidation. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentVariableThresholdSampler.java | Removed consistent56 variable-threshold sampler (migrated into consistent). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentThresholdSampler.java | Removed consistent56 threshold sampler base class. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java | Removed consistent56 sampler factory/base (replaced by incubator APIs + contrib-only factories). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSampler.java | Removed consistent56 rule-based sampler (replaced by incubator rule-based builder). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSampler.java | Removed consistent56 rate limiter implementation (migrated into consistent). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentParentBasedSampler.java | Removed consistent56 parent-based sampler (replaced by incubator parentThreshold). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentFixedThresholdSampler.java | Removed consistent56 fixed-threshold sampler (replaced by incubator probability). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOnSampler.java | Removed consistent56 always-on sampler (replaced by incubator alwaysOn). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAlwaysOffSampler.java | Removed consistent56 always-off sampler (replaced by incubator alwaysOff). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/Composable.java | Removed contrib-defined composable interface (replaced by incubator ComposableSampler). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/RValueGenerators.java | Removed legacy p/r generator utilities from the old implementation. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/RValueGenerator.java | Removed legacy p/r generator API from the old implementation. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ParentBasedConsistentProbabilitySamplerProvider.java | Updated provider to build via incubator ComposableSampler/CompositeSampler. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentVariableThresholdSampler.java | Added ComposableSampler-based updateable probability sampler. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentSamplingUtil.java | Moved util into consolidated package. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentSampler.java | Replaced legacy base class with contrib-only factory entry points over incubator APIs. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentRateLimitingSampler.java | Migrated rate limiter to incubator ComposableSampler + new SamplingIntent shape. |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentProbabilityBasedSampler.java | Removed legacy probability-based sampler implementation (use incubator probability). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentParentBasedSampler.java | Removed legacy parent-based sampler implementation (use incubator parentThreshold). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentComposedOrSampler.java | Removed legacy OR composition (use incubator composites / rule-based as appropriate). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentComposedAndSampler.java | Removed legacy AND composition (use incubator composites / rule-based as appropriate). |
| consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentAnyOf.java | Updated any-of implementation to incubator ComposableSampler and SamplingIntent. |
| consistent-sampling/build.gradle.kts | Added SDK incubator extension dependency required for composable sampler APIs. |
| consistent-sampling/README.md | Updated documentation to describe incubator composable API usage and remaining contrib-only samplers. |
| CHANGELOG.md | Documented breaking changes, removed classes, and migration guidance to incubator APIs. |
Comments suppressed due to low confidence (2)
consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent/ConsistentAnyOf.java:45
ConsistentAnyOfstores the caller-provided varargs array directly (this.delegates = delegates) and doesn’t validate individual entries. This violates the@Immutablecontract (callers can mutate the array after construction) and can lead to NPEs if any delegate is null. Consider making a defensive copy (e.g.,Arrays.copyOf),requireNonNulleach delegate, and building thedescriptionfromdelegate.getDescription()(rather than relying ontoString()) so it’s stable and meaningful.
consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent/MarkingSampler.java:24- Javadoc still refers to this as a "Composable", but the type is now
ComposableSampler. Updating terminology here will avoid confusion when reading tests/examples after the API migration.
| @Override | ||
| public SamplingIntent getSamplingIntent( | ||
| Context parentContext, | ||
| String traceId, |
There was a problem hiding this comment.
The specification for ComposableSamplers (https://github.com/open-telemetry/opentelemetry-specification/blob/v1.57.0/oteps/trace/0250-Composite_Samplers.md) explicitly omit passing traceId to getSamplingIntent(). This is because the Threshold proposed by the sampler MUST NOT depend on traceId.
There was a problem hiding this comment.
This signature comes from the upstream SDK incubator ComposableSampler.java, can you send a PR there if it should be removed?
There was a problem hiding this comment.
Oh, I missed that. Should I create an Issue for that or just go for a PR?
There was a problem hiding this comment.
I'd just send the PR, often easier and more likely to get discussion going when there's code involved 😄
There was a problem hiding this comment.
Done - see open-telemetry/opentelemetry-java#8450
|
|
||
| // If any of the delegates returning the threshold value equal to T returns true upon calling | ||
| // its IsAdjustedCountReliable() method, the resulting isAdjustedCountReliable is true, | ||
| // its isThresholdReliable() method, the resulting isThresholdReliable is true, |
There was a problem hiding this comment.
The specification and the original implementation use isAdjustedCountReliable() rather than isThresholdReliable(). There's a subtle difference. The method is expected to return false in a corner case where it is impossible to know what the adjusted count is, but the getThreshold() method returns a valid threshold in the sense that it still should be used to make the final sampling decision.
There was a problem hiding this comment.
should the method name be changed in the upstream SDK incubator SamplingIntent.java?
|
The list of modified files seems to be incomplete - for example, I do not see the ComposableSampler class definition (or perhaps it just hard to find, the PR is rather large). |
ComposableSampler.java now lives in the upstream SDK incubator. |
…-to-upstream # Conflicts: # CHANGELOG.md # consistent-sampling/README.md
Summary
Rebases the consistent sampling module onto the OpenTelemetry SDK incubator composable sampler API.
This collapses the old
consistentandconsistent56implementations into the existingio.opentelemetry.contrib.sampler.consistentpackage, using SDK incubator sampler building blocks where available and keeping contrib-only APIs for the remaining functionality.Changes
ComposableSampler/CompositeSamplerAPIs.