diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableAlwaysOnSampler.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableAlwaysOnSampler.java index 54dbd948e46..c85b499d8a0 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableAlwaysOnSampler.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableAlwaysOnSampler.java @@ -10,7 +10,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.sdk.trace.data.LinkData; import java.util.List; -import java.util.function.Function; enum ComposableAlwaysOnSampler implements ComposableSampler { INSTANCE; @@ -20,7 +19,7 @@ enum ComposableAlwaysOnSampler implements ComposableSampler { ImmutableSamplingIntent.MIN_THRESHOLD, /* adjustedCountReliable= */ true, Attributes.empty(), - Function.identity()); + ImmutableSamplingIntent.NO_TRACE_STATE_UPDATE); @Override public SamplingIntent getSamplingIntent( diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableParentThresholdSampler.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableParentThresholdSampler.java index 7860857bd30..7d5da47535a 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableParentThresholdSampler.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableParentThresholdSampler.java @@ -7,6 +7,7 @@ import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.INVALID_THRESHOLD; import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.MIN_THRESHOLD; +import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.NO_TRACE_STATE_UPDATE; import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.isValidThreshold; import io.opentelemetry.api.common.Attributes; @@ -16,7 +17,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.sdk.trace.data.LinkData; import java.util.List; -import java.util.function.Function; final class ComposableParentThresholdSampler implements ComposableSampler { @@ -40,19 +40,23 @@ public SamplingIntent getSamplingIntent( return rootSampler.getSamplingIntent(parentContext, name, spanKind, attributes, parentLinks); } + boolean parentSampled = parentSpanContext.getTraceFlags().isSampled(); OtelTraceState otTraceState = OtelTraceState.parse(parentSpanContext.getTraceState()); - if (isValidThreshold(otTraceState.getThreshold())) { + long parentThreshold = otTraceState.getThreshold(); + if (isValidThreshold(parentThreshold) + && (!parentSampled + || parentThreshold + <= otTraceState.getRandomValueOrFromTraceId(parentSpanContext.getTraceId()))) { return ImmutableSamplingIntent.create( - otTraceState.getThreshold(), + parentThreshold, /* adjustedCountReliable= */ true, Attributes.empty(), - Function.identity()); + NO_TRACE_STATE_UPDATE); } - long threshold = - parentSpanContext.getTraceFlags().isSampled() ? MIN_THRESHOLD : INVALID_THRESHOLD; + long threshold = parentSampled ? MIN_THRESHOLD : INVALID_THRESHOLD; return ImmutableSamplingIntent.create( - threshold, /* adjustedCountReliable= */ false, Attributes.empty(), Function.identity()); + threshold, /* adjustedCountReliable= */ false, Attributes.empty(), NO_TRACE_STATE_UPDATE); } @Override diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableProbabilitySampler.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableProbabilitySampler.java index 0bfa1b8441b..32734b9b414 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableProbabilitySampler.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableProbabilitySampler.java @@ -10,7 +10,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.sdk.trace.data.LinkData; import java.util.List; -import java.util.function.Function; final class ComposableProbabilitySampler implements ComposableSampler { private static long calculateThreshold(double ratio) { @@ -35,7 +34,7 @@ private static long calculateThreshold(double ratio) { ImmutableSamplingIntent.INVALID_THRESHOLD, /* adjustedCountReliable= */ false, Attributes.empty(), - Function.identity()); + ImmutableSamplingIntent.NO_TRACE_STATE_UPDATE); } else { StringBuilder sb = new StringBuilder(); OtelTraceState.serializeTh(threshold, sb); @@ -46,7 +45,7 @@ private static long calculateThreshold(double ratio) { threshold, /* adjustedCountReliable= */ true, Attributes.empty(), - Function.identity()); + ImmutableSamplingIntent.NO_TRACE_STATE_UPDATE); } this.description = "ComposableProbabilitySampler{threshold=" + thresholdStr + ", ratio=" + ratio + "}"; diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/CompositeSampler.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/CompositeSampler.java index fe95b6609ac..8f3f28dc705 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/CompositeSampler.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/CompositeSampler.java @@ -6,12 +6,10 @@ package io.opentelemetry.sdk.extension.incubator.trace.samplers; import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.INVALID_THRESHOLD; -import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.isValidRandomValue; import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.isValidThreshold; import static io.opentelemetry.sdk.extension.incubator.trace.samplers.OtelTraceState.OTEL_TRACE_STATE_KEY; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.internal.OtelEncodingUtils; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.TraceState; @@ -61,12 +59,7 @@ public SamplingResult shouldSample( adjustedCountReliable = intent.isAdjustedCountReliable(); long randomValue; if (adjustedCountReliable) { - if (isValidRandomValue(otelTraceState.getRandomValue())) { - randomValue = otelTraceState.getRandomValue(); - } else { - // Use last 56 bits of trace ID as random value. - randomValue = OtelEncodingUtils.longFromBase16String(traceId, 16) & 0x00FFFFFFFFFFFFFFL; - } + randomValue = otelTraceState.getRandomValueOrFromTraceId(traceId); } else { randomValue = RandomSupplier.platformDefault().get().nextLong() & 0x00FFFFFFFFFFFFFFL; } @@ -99,10 +92,12 @@ public Attributes getAttributes() { @Override public TraceState getUpdatedTraceState(TraceState parentTraceState) { - TraceState newTraceState = intent.getTraceStateUpdater().apply(traceState); + TraceState newTraceState = intent.getTraceStateUpdater().apply(traceState, decision); if (!serializedState.isEmpty()) { newTraceState = newTraceState.toBuilder().put(OTEL_TRACE_STATE_KEY, serializedState).build(); + } else if (newTraceState.get(OTEL_TRACE_STATE_KEY) != null) { + newTraceState = newTraceState.toBuilder().remove(OTEL_TRACE_STATE_KEY).build(); } return newTraceState; } diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ImmutableSamplingIntent.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ImmutableSamplingIntent.java index 73962ceb07d..d1194891fa6 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ImmutableSamplingIntent.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ImmutableSamplingIntent.java @@ -8,7 +8,8 @@ import com.google.auto.value.AutoValue; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.TraceState; -import java.util.function.Function; +import io.opentelemetry.sdk.trace.samplers.SamplingDecision; +import java.util.function.BiFunction; @AutoValue abstract class ImmutableSamplingIntent implements SamplingIntent { @@ -20,12 +21,15 @@ abstract class ImmutableSamplingIntent implements SamplingIntent { static final long MAX_THRESHOLD = 1L << RANDOM_VALUE_BITS; static final long MAX_RANDOM_VALUE = MAX_THRESHOLD - 1; + static final BiFunction NO_TRACE_STATE_UPDATE = + (traceState, decision) -> traceState; + static final SamplingIntent NON_SAMPLING_INTENT = create( ImmutableSamplingIntent.INVALID_THRESHOLD, /* adjustedCountReliable= */ false, Attributes.empty(), - Function.identity()); + NO_TRACE_STATE_UPDATE); static boolean isValidThreshold(long threshold) { return threshold >= MIN_THRESHOLD && threshold <= MAX_THRESHOLD; @@ -39,7 +43,7 @@ static ImmutableSamplingIntent create( long threshold, boolean adjustedCountReliable, Attributes attributes, - Function traceStateUpdater) { + BiFunction traceStateUpdater) { return new AutoValue_ImmutableSamplingIntent( threshold, adjustedCountReliable, attributes, traceStateUpdater); } diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/OtelTraceState.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/OtelTraceState.java index b108c82f573..e9cd38ec7b9 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/OtelTraceState.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/OtelTraceState.java @@ -7,6 +7,7 @@ import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.INVALID_RANDOM_VALUE; import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.INVALID_THRESHOLD; +import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.MAX_RANDOM_VALUE; import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.MAX_THRESHOLD; import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.isValidRandomValue; import static io.opentelemetry.sdk.extension.incubator.trace.samplers.ImmutableSamplingIntent.isValidThreshold; @@ -128,6 +129,17 @@ long getRandomValue() { return randomValue; } + /** + * Returns the explicit randomness value if present, otherwise the randomness derived from the + * last 56 bits of the trace ID. + */ + long getRandomValueOrFromTraceId(String traceId) { + if (isValidRandomValue(randomValue)) { + return randomValue; + } + return OtelEncodingUtils.longFromBase16String(traceId, 16) & MAX_RANDOM_VALUE; + } + long getThreshold() { return threshold; } diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/SamplingIntent.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/SamplingIntent.java index f6c5625b9fe..798dc44c259 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/SamplingIntent.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/SamplingIntent.java @@ -7,7 +7,8 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.TraceState; -import java.util.function.Function; +import io.opentelemetry.sdk.trace.samplers.SamplingDecision; +import java.util.function.BiFunction; /** Information to make a sampling decision. */ public interface SamplingIntent { @@ -17,7 +18,7 @@ static SamplingIntent create( long threshold, boolean adjustedCountReliable, Attributes attributes, - Function traceStateUpdater) { + BiFunction traceStateUpdater) { return ImmutableSamplingIntent.create( threshold, adjustedCountReliable, attributes, traceStateUpdater); } @@ -40,6 +41,10 @@ static SamplingIntent create( /** Returns any attributes to add to the span to record the sampling result. */ Attributes getAttributes(); - /** Returns a function to apply to the tracestate of the span to possibly update it. */ - Function getTraceStateUpdater(); + /** + * Returns a function to apply to the parent tracestate and final sampling decision to possibly + * update the tracestate of the span. The function must not modify the OpenTelemetry {@code ot} + * entry of the tracestate. + */ + BiFunction getTraceStateUpdater(); } diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableParentThresholdSamplerTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableParentThresholdSamplerTest.java index a49d6a00687..ac635c7c299 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableParentThresholdSamplerTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/ComposableParentThresholdSamplerTest.java @@ -44,6 +44,29 @@ void rootSpan() { @Test void parentWithThreshold() { + SpanContext parentSpanContext = + SpanContext.create( + TraceId.fromLongs(1, 2), + SpanId.fromLong(3), + TraceFlags.getSampled(), + TraceState.builder() + .put("ot", new OtelTraceState(10, 10, Collections.emptyList()).serialize()) + .build()); + SamplingIntent intent = + ComposableSampler.parentThreshold(ComposableSampler.alwaysOn()) + .getSamplingIntent( + Context.root().with(Span.wrap(parentSpanContext)), + "span", + SpanKind.SERVER, + Attributes.empty(), + Collections.emptyList()); + assertThat(intent.getThreshold()).isEqualTo(10); + assertThat(intent.isAdjustedCountReliable()).isTrue(); + } + + @Test + void parentWithInconsistentThresholdSampled() { + // Sampled even though rv < th, so the threshold is erased and the sampled flag is used. SpanContext parentSpanContext = SpanContext.create( TraceId.fromLongs(1, 2), @@ -52,16 +75,59 @@ void parentWithThreshold() { TraceState.builder() .put("ot", new OtelTraceState(1, 10, Collections.emptyList()).serialize()) .build()); - assertThat( - ComposableSampler.parentThreshold(ComposableSampler.alwaysOn()) - .getSamplingIntent( - Context.root().with(Span.wrap(parentSpanContext)), - "span", - SpanKind.SERVER, - Attributes.empty(), - Collections.emptyList()) - .getThreshold()) - .isEqualTo(10); + SamplingIntent intent = + ComposableSampler.parentThreshold(ComposableSampler.alwaysOn()) + .getSamplingIntent( + Context.root().with(Span.wrap(parentSpanContext)), + "span", + SpanKind.SERVER, + Attributes.empty(), + Collections.emptyList()); + assertThat(intent.getThreshold()).isZero(); + assertThat(intent.isAdjustedCountReliable()).isFalse(); + } + + @Test + void parentWithInconsistentThresholdSampledTraceIdRandomness() { + // No rv, so randomness comes from the trace ID, which is 2 here. + SpanContext parentSpanContext = + SpanContext.create( + TraceId.fromLongs(1, 2), + SpanId.fromLong(3), + TraceFlags.getSampled(), + TraceState.builder().put("ot", "th:8").build()); + SamplingIntent intent = + ComposableSampler.parentThreshold(ComposableSampler.alwaysOn()) + .getSamplingIntent( + Context.root().with(Span.wrap(parentSpanContext)), + "span", + SpanKind.SERVER, + Attributes.empty(), + Collections.emptyList()); + assertThat(intent.getThreshold()).isZero(); + assertThat(intent.isAdjustedCountReliable()).isFalse(); + } + + @Test + void parentWithThresholdNotSampled() { + SpanContext parentSpanContext = + SpanContext.create( + TraceId.fromLongs(1, 2), + SpanId.fromLong(3), + TraceFlags.getDefault(), + TraceState.builder() + .put("ot", new OtelTraceState(1, 10, Collections.emptyList()).serialize()) + .build()); + SamplingIntent intent = + ComposableSampler.parentThreshold(ComposableSampler.alwaysOn()) + .getSamplingIntent( + Context.root().with(Span.wrap(parentSpanContext)), + "span", + SpanKind.SERVER, + Attributes.empty(), + Collections.emptyList()); + assertThat(intent.getThreshold()).isEqualTo(10); + assertThat(intent.isAdjustedCountReliable()).isTrue(); } @Test diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/CompositeSamplerTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/CompositeSamplerTest.java index 8f2c91bc481..82bc79b6551 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/CompositeSamplerTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/samplers/CompositeSamplerTest.java @@ -24,8 +24,13 @@ import io.opentelemetry.sdk.trace.samplers.SamplingResult; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.OptionalLong; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; class CompositeSamplerTest { private static class Input { @@ -275,4 +280,116 @@ void testParentViolatingInvariant() { assertThat(output.getThreshold()).hasValue(0x0L); assertThat(output.getRandomValue()).hasValue(0x80000000000000L); } + + // The trace ID used here has randomness 0, so th:8 is inconsistent with a sampled flag. + @ParameterizedTest + @MethodSource("parentThresholdTraceStateCases") + void parentThresholdTraceState( + boolean parentSampled, + TraceState parentTraceState, + SamplingDecision expectedDecision, + TraceState expectedTraceState) { + TraceFlags traceFlags = parentSampled ? TraceFlags.getSampled() : TraceFlags.getDefault(); + Context parentContext = + Span.wrap( + SpanContext.create( + Input.getTraceId(), "0123456789abcdef", traceFlags, parentTraceState)) + .storeInContext(Context.root()); + + Sampler sampler = + CompositeSampler.wrap(ComposableSampler.parentThreshold(ComposableSampler.alwaysOn())); + SamplingResult result = + sampler.shouldSample( + parentContext, + Input.getTraceId(), + Input.getName(), + Input.getSpanKind(), + Input.getAttributes(), + Input.getParentLinks()); + + assertThat(result.getDecision()).isEqualTo(expectedDecision); + assertThat(result.getUpdatedTraceState(parentTraceState).asMap()) + .isEqualTo(expectedTraceState.asMap()); + } + + private static Stream parentThresholdTraceStateCases() { + return Stream.of( + Arguments.argumentSet( + "dropped, only th", + false, + TraceState.builder().put("ot", "th:f").build(), + SamplingDecision.DROP, + TraceState.getDefault()), + Arguments.argumentSet( + "dropped, other vendor", + false, + TraceState.builder().put("ot", "th:8").put("vendor", "x").build(), + SamplingDecision.DROP, + TraceState.builder().put("vendor", "x").build()), + Arguments.argumentSet( + "dropped, keeps rv", + false, + TraceState.builder().put("ot", "th:8;rv:7fffffffffffff").build(), + SamplingDecision.DROP, + TraceState.builder().put("ot", "rv:7fffffffffffff").build()), + Arguments.argumentSet( + "sampled, consistent", + true, + TraceState.builder().put("ot", "th:8;rv:80000000000000").build(), + SamplingDecision.RECORD_AND_SAMPLE, + TraceState.builder().put("ot", "th:8;rv:80000000000000").build()), + Arguments.argumentSet( + "sampled, inconsistent with trace ID", + true, + TraceState.builder().put("ot", "th:8").build(), + SamplingDecision.RECORD_AND_SAMPLE, + TraceState.getDefault()), + Arguments.argumentSet( + "sampled, inconsistent with rv", + true, + TraceState.builder().put("ot", "th:8;rv:7fffffffffffff").build(), + SamplingDecision.RECORD_AND_SAMPLE, + TraceState.builder().put("ot", "rv:7fffffffffffff").build())); + } + + @ParameterizedTest + @MethodSource("traceStateUpdaterCases") + void traceStateUpdaterReceivesDecision(long threshold, SamplingDecision expectedDecision) { + ComposableSampler delegate = + new ComposableSampler() { + @Override + public SamplingIntent getSamplingIntent( + Context parentContext, + String name, + SpanKind spanKind, + Attributes attributes, + List parentLinks) { + return SamplingIntent.create( + threshold, + /* adjustedCountReliable= */ true, + Attributes.empty(), + (traceState, decision) -> + traceState.toBuilder() + .put("decision", decision.name().toLowerCase(Locale.ROOT)) + .build()); + } + + @Override + public String getDescription() { + return "DecisionRecordingSampler"; + } + }; + + Output output = sample(new Input(), CompositeSampler.wrap(delegate)); + + assertThat(output.samplingResult.getDecision()).isEqualTo(expectedDecision); + assertThat(output.samplingResult.getUpdatedTraceState(TraceState.getDefault()).get("decision")) + .isEqualTo(expectedDecision.name().toLowerCase(Locale.ROOT)); + } + + private static Stream traceStateUpdaterCases() { + return Stream.of( + Arguments.argumentSet("sampled", 0L, SamplingDecision.RECORD_AND_SAMPLE), + Arguments.argumentSet("dropped", INVALID_THRESHOLD, SamplingDecision.DROP)); + } }