From a8443c470396e4403b33d5ad32f0cf79ff5daab2 Mon Sep 17 00:00:00 2001 From: Peter Findeisen Date: Tue, 3 Sep 2024 13:40:22 -0700 Subject: [PATCH] Refactoring, adding a new test case based on https://github.com/open-telemetry/oteps/pull/250 --- .../ConsistentRuleBasedSampler.java | 16 +-- .../consistent56/ConsistentSampler.java | 35 ++++- .../sampler/consistent56/Predicate.java | 30 +++- .../consistent56/PredicatedSampler.java | 2 +- .../ConsistentRuleBasedSamplerTest.java | 14 +- .../sampler/consistent56/MarkingSampler.java | 8 +- .../sampler/consistent56/UseCaseTest.java | 129 ++++++++++++++++++ 7 files changed, 202 insertions(+), 32 deletions(-) create mode 100644 consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/UseCaseTest.java diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSampler.java index 7fde6b097..81acde8cb 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSampler.java @@ -28,21 +28,7 @@ final class ConsistentRuleBasedSampler extends ConsistentSampler { private final String description; - /** - * 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 create( - @Nullable SpanKind spanKindToMatch, PredicatedSampler... samplers) { - return new ConsistentRuleBasedSampler(spanKindToMatch, samplers); - } - - private ConsistentRuleBasedSampler( - @Nullable SpanKind spanKindToMatch, PredicatedSampler... samplers) { + ConsistentRuleBasedSampler(@Nullable SpanKind spanKindToMatch, PredicatedSampler... samplers) { this.spanKindToMatch = spanKindToMatch; this.samplers = samplers; diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java index 0568138ea..5592b3215 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java @@ -20,6 +20,7 @@ import io.opentelemetry.sdk.trace.samplers.SamplingResult; import java.util.List; import java.util.function.LongSupplier; +import javax.annotation.Nullable; /** Abstract base class for consistent samplers. */ @SuppressWarnings("InconsistentOverloads") @@ -64,6 +65,19 @@ public static ConsistentSampler parentBased(ComposableSampler 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); + } + /** * Returns a new {@link ConsistentSampler} that attempts to adjust the sampling probability * dynamically to meet the target span rate. @@ -72,9 +86,26 @@ public static ConsistentSampler parentBased(ComposableSampler rootSampler) { * @param adaptationTimeSeconds the typical time to adapt to a new load (time constant used for * exponential smoothing) */ - public static ConsistentSampler rateLimited( + static ConsistentSampler rateLimited( double targetSpansPerSecondLimit, double adaptationTimeSeconds) { - return rateLimited(targetSpansPerSecondLimit, adaptationTimeSeconds, System::nanoTime); + return rateLimited(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) + */ + public static ConsistentSampler rateLimited( + ComposableSampler delegate, double targetSpansPerSecondLimit, double adaptationTimeSeconds) { + return rateLimited( + delegate, targetSpansPerSecondLimit, adaptationTimeSeconds, System::nanoTime); } /** diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/Predicate.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/Predicate.java index db96146ae..56ce59c46 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/Predicate.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/Predicate.java @@ -30,7 +30,7 @@ boolean spanMatches( /* * Return a Predicate that will match ROOT Spans only */ - static Predicate forRootSpan() { + static Predicate isRootSpan() { return (parentContext, name, spanKind, attributes, parentLinks) -> { Span parentSpan = Span.fromContext(parentContext); SpanContext parentSpanContext = parentSpan.getSpanContext(); @@ -41,9 +41,33 @@ static Predicate forRootSpan() { /* * Return a Predicate that matches all Spans */ - static Predicate forAnySpan() { + static Predicate anySpan() { return (parentContext, name, spanKind, attributes, parentLinks) -> true; } - // TO DO: allow for composing Predicates: and(p1,p2), or(p1,p2), and not(p1). + /* + * Return a Predicate that represents logical AND of the argument predicates + */ + static Predicate and(Predicate p1, Predicate p2) { + return (parentContext, name, spanKind, attributes, parentLinks) -> + p1.spanMatches(parentContext, name, spanKind, attributes, parentLinks) + && p2.spanMatches(parentContext, name, spanKind, attributes, parentLinks); + } + + /* + * Return a Predicate that represents logical negation of the argument predicate + */ + static Predicate not(Predicate p) { + return (parentContext, name, spanKind, attributes, parentLinks) -> + !p.spanMatches(parentContext, name, spanKind, attributes, parentLinks); + } + + /* + * Return a Predicate that represents logical OR of the argument predicates + */ + static Predicate or(Predicate p1, Predicate p2) { + return (parentContext, name, spanKind, attributes, parentLinks) -> + p1.spanMatches(parentContext, name, spanKind, attributes, parentLinks) + || p2.spanMatches(parentContext, name, spanKind, attributes, parentLinks); + } } diff --git a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/PredicatedSampler.java b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/PredicatedSampler.java index 4a52649dc..dabec5b74 100644 --- a/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/PredicatedSampler.java +++ b/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/PredicatedSampler.java @@ -10,7 +10,7 @@ /** A class for holding a pair (Predicate, ComposableSampler) */ public final class PredicatedSampler { - public static PredicatedSampler create(Predicate predicate, ComposableSampler sampler) { + public static PredicatedSampler onMatch(Predicate predicate, ComposableSampler sampler) { return new PredicatedSampler(predicate, sampler); } diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSamplerTest.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSamplerTest.java index 52a79df6e..bf79de0c5 100644 --- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSamplerTest.java +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSamplerTest.java @@ -16,7 +16,7 @@ class ConsistentRuleBasedSamplerTest { @Test void testEmptySet() { - ComposableSampler sampler = ConsistentRuleBasedSampler.create(SpanKind.SERVER); + ComposableSampler sampler = ConsistentSampler.ruleBased(SpanKind.SERVER); SamplingIntent intent = sampler.getSamplingIntent(null, "span_name", SpanKind.SERVER, null, null); assertThat(intent.getThreshold()).isEqualTo(getInvalidThreshold()); @@ -44,11 +44,11 @@ void testChoice() { new MarkingSampler(new ConsistentFixedThresholdSampler(0x30000000000000L), key3, "c"); ComposableSampler sampler = - ConsistentRuleBasedSampler.create( + ConsistentSampler.ruleBased( null, - PredicatedSampler.create(matchSpanName("A"), delegate1), - PredicatedSampler.create(matchSpanName("B"), delegate2), - PredicatedSampler.create(matchSpanName("C"), delegate3)); + PredicatedSampler.onMatch(matchSpanName("A"), delegate1), + PredicatedSampler.onMatch(matchSpanName("B"), delegate2), + PredicatedSampler.onMatch(matchSpanName("C"), delegate3)); SamplingIntent intent; @@ -80,9 +80,9 @@ void testChoice() { @Test void testSpanKindMatch() { ComposableSampler sampler = - ConsistentRuleBasedSampler.create( + ConsistentSampler.ruleBased( SpanKind.CLIENT, - PredicatedSampler.create(Predicate.forAnySpan(), ConsistentSampler.alwaysOn())); + PredicatedSampler.onMatch(Predicate.anySpan(), ConsistentSampler.alwaysOn())); SamplingIntent intent; diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/MarkingSampler.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/MarkingSampler.java index a07fbfca1..687cd532a 100644 --- a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/MarkingSampler.java +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/MarkingSampler.java @@ -18,12 +18,12 @@ import javax.annotation.concurrent.Immutable; /** - * A consistent sampler that makes the same sampling decision as the delegate, but it additionally - * sets a Span attribute according to the provided attribute key and value. This is used by unit - * tests, but could be also offered as a general utility. + * A Composable that creates the same sampling intent as the delegate, but it additionally sets a + * Span attribute according to the provided attribute key and value. This is used by unit tests, but + * could be also offered as a general utility. */ @Immutable -final class MarkingSampler extends ConsistentSampler { +final class MarkingSampler implements ComposableSampler { private final ComposableSampler delegate; private final AttributeKey attributeKey; diff --git a/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/UseCaseTest.java b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/UseCaseTest.java new file mode 100644 index 000000000..434ddcfdb --- /dev/null +++ b/consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/UseCaseTest.java @@ -0,0 +1,129 @@ +/* + * Copyright The OpenTelemetry Authors + * 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; +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 org.junit.jupiter.api.Test; + +/** + * Testing a "real life" sampler configuration, as provided as an example in + * https://github.com/open-telemetry/oteps/pull/250. The example uses many different composite + * samplers combining them together to demonstrate the expressiveness and flexibility of the + * proposed specification. + */ +class UseCaseTest { + private static final long[] nanoTime = new long[] {0L}; + + private static final long nanoTime() { + return nanoTime[0]; + } + ; + + private static void advanceTime(long nanosIncrement) { + nanoTime[0] += nanosIncrement; + } + + // + // S = ConsistentRateLimiting( + // ConsistentAnyOf( + // ConsistentParentBased( + // ConsistentRuleBased(ROOT, { + // (http.target == /healthcheck) => ConsistentAlwaysOff, + // (http.target == /checkout) => ConsistentAlwaysOn, + // true => ConsistentFixedThreshold(0.25) + // }), + // ConsistentRuleBased(CLIENT, { + // (http.url == /foo) => ConsistentAlwaysOn + // } + // ), + // 1000.0 + // ) + // + private static final AttributeKey httpTarget = AttributeKey.stringKey("http.target"); + private static final AttributeKey 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)); + }); + ComposableSampler 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)); + }; + + ComposableSampler s2 = ConsistentSampler.ruleBased(SpanKind.CLIENT, onMatch(foo, alwaysOn())); + ComposableSampler s3 = ConsistentSampler.anyOf(s1, s2); + return ConsistentSampler.rateLimited(s3, 1000.0, 5, UseCaseTest::nanoTime); + } + + @Test + void testDropHealthcheck() { + ConsistentSampler s = buildSampler(); + Attributes attributes = createAttributes(httpTarget, "/healthCheck"); + SamplingIntent intent = s.getSamplingIntent(null, "A", SpanKind.SERVER, attributes, null); + assertThat(intent.getThreshold()).isEqualTo(getInvalidThreshold()); + } + + @Test + void testSampleCheckout() { + ConsistentSampler s = buildSampler(); + advanceTime(1000000); + Attributes attributes = createAttributes(httpTarget, "/checkout"); + SamplingIntent intent = s.getSamplingIntent(null, "B", SpanKind.SERVER, attributes, null); + assertThat(intent.getThreshold()).isEqualTo(0L); + advanceTime(1000); // rate limiting should kick in + intent = s.getSamplingIntent(null, "B", SpanKind.SERVER, attributes, null); + assertThat(intent.getThreshold()).isGreaterThan(0L); + } + + @Test + void testSampleClient() { + ConsistentSampler s = buildSampler(); + advanceTime(1000000); + Attributes attributes = createAttributes(httpUrl, "/foo"); + SamplingIntent intent = s.getSamplingIntent(null, "C", SpanKind.CLIENT, attributes, null); + assertThat(intent.getThreshold()).isEqualTo(0L); + } + + @Test + void testOtherRoot() { + ConsistentSampler s = buildSampler(); + advanceTime(1000000); + Attributes attributes = Attributes.empty(); + SamplingIntent intent = s.getSamplingIntent(null, "D", SpanKind.SERVER, attributes, null); + assertThat(intent.getThreshold()).isEqualTo(0xc0000000000000L); + } + + private static Attributes createAttributes(AttributeKey key, String value) { + return Attributes.builder().put(key, value).build(); + } +}