From 4c199acea155a91c9020c9e8fb3c02d73050c908 Mon Sep 17 00:00:00 2001 From: Mohammed Abdessetar Elyagoubi Date: Tue, 28 Jul 2026 23:04:24 +0100 Subject: [PATCH 1/2] fix: honor Jaeger sampler interval --- .../jaeger-remote-sampler/build.gradle.kts | 2 + .../jaeger/sampler/JaegerRemoteSampler.java | 7 +++ .../JaegerRemoteSamplerComponentProvider.java | 2 +- ...gerRemoteSamplerComponentProviderTest.java | 60 +++++++++++++++++++ 4 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerComponentProviderTest.java diff --git a/sdk-extensions/jaeger-remote-sampler/build.gradle.kts b/sdk-extensions/jaeger-remote-sampler/build.gradle.kts index 831774de06b..70e8d06607b 100644 --- a/sdk-extensions/jaeger-remote-sampler/build.gradle.kts +++ b/sdk-extensions/jaeger-remote-sampler/build.gradle.kts @@ -33,6 +33,8 @@ dependencies { testImplementation(project(":sdk:testing")) testImplementation(project(":sdk-extensions:autoconfigure")) + testImplementation(project(":sdk-extensions:declarative-config")) + testImplementation(project(":api:incubator")) testImplementation("com.google.guava:guava") testImplementation("com.google.protobuf:protobuf-java") testImplementation("com.linecorp.armeria:armeria-junit5") diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java index 52a8b350e7c..94ba1907d1c 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java @@ -45,6 +45,7 @@ public final class JaegerRemoteSampler implements Sampler { private final AtomicBoolean isShutdown = new AtomicBoolean(); private final GrpcSender grpcSender; + private final int pollingIntervalMs; JaegerRemoteSampler( GrpcSender grpcSender, @@ -53,6 +54,7 @@ public final class JaegerRemoteSampler implements Sampler { Sampler initialSampler) { this.serviceName = serviceName != null ? serviceName : ""; this.grpcSender = grpcSender; + this.pollingIntervalMs = pollingIntervalMs; this.sampler = initialSampler; pollExecutor = Executors.newScheduledThreadPool(1, new DaemonThreadFactory(WORKER_THREAD_NAME)); pollFuture = @@ -174,6 +176,11 @@ Sampler getSampler() { return this.sampler; } + // Visible for testing + int getPollingIntervalMs() { + return this.pollingIntervalMs; + } + public static JaegerRemoteSamplerBuilder builder() { return new JaegerRemoteSamplerBuilder(); } diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/internal/JaegerRemoteSamplerComponentProvider.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/internal/JaegerRemoteSamplerComponentProvider.java index b6dd93b77c1..b983f6751ef 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/internal/JaegerRemoteSamplerComponentProvider.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/internal/JaegerRemoteSamplerComponentProvider.java @@ -47,7 +47,7 @@ public Sampler create(DeclarativeConfigProperties config) { } builder.setInitialSampler(DeclarativeConfiguration.createSampler(initialSamplerModel)); - Long pollingIntervalMs = config.getLong("internal"); + Long pollingIntervalMs = config.getLong("interval"); if (pollingIntervalMs != null) { builder.setPollingInterval(Duration.ofMillis(pollingIntervalMs)); } diff --git a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerComponentProviderTest.java b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerComponentProviderTest.java new file mode 100644 index 00000000000..585c9439b99 --- /dev/null +++ b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerComponentProviderTest.java @@ -0,0 +1,60 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.extension.trace.jaeger.sampler; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; +import io.opentelemetry.internal.testing.slf4j.SuppressLogger; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfiguration; +import io.opentelemetry.sdk.extension.trace.jaeger.sampler.internal.JaegerRemoteSamplerComponentProvider; +import io.opentelemetry.sdk.trace.samplers.Sampler; +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import org.junit.jupiter.api.Test; + +// Connecting to the (unavailable) endpoint during the initial poll logs a warning; suppress it. +@SuppressLogger(JaegerRemoteSampler.class) +class JaegerRemoteSamplerComponentProviderTest { + + private static final int DEFAULT_POLLING_INTERVAL_MILLIS = 60000; + + @Test + void create_appliesConfiguredInterval() { + // Regression test for reading the wrong declarative config key: the property is "interval", so + // reading "internal" silently dropped the operator's value and fell back to the default. + Sampler sampler = + create( + "endpoint: http://localhost:14250\n" + + "interval: 10000\n" + + "initial_sampler:\n" + + " always_off: {}\n"); + try { + assertThat(((JaegerRemoteSampler) sampler).getPollingIntervalMs()).isEqualTo(10000); + } finally { + ((JaegerRemoteSampler) sampler).shutdown(); + } + } + + @Test + void create_defaultsIntervalWhenOmitted() { + Sampler sampler = + create("endpoint: http://localhost:14250\n" + "initial_sampler:\n" + " always_off: {}\n"); + try { + assertThat(((JaegerRemoteSampler) sampler).getPollingIntervalMs()) + .isEqualTo(DEFAULT_POLLING_INTERVAL_MILLIS); + } finally { + ((JaegerRemoteSampler) sampler).shutdown(); + } + } + + private static Sampler create(String yaml) { + DeclarativeConfigProperties config = + DeclarativeConfiguration.toConfigProperties( + new ByteArrayInputStream(yaml.getBytes(StandardCharsets.UTF_8))); + return new JaegerRemoteSamplerComponentProvider().create(config); + } +} From 27f00d548a5441e83644f68ed7d0cbeccfc0d6e0 Mon Sep 17 00:00:00 2001 From: Mohammed Abdessetar Elyagoubi Date: Thu, 30 Jul 2026 17:49:10 +0100 Subject: [PATCH 2/2] fix: improve Jaeger sampler description --- .../jaeger-remote-sampler/build.gradle.kts | 2 - .../jaeger/sampler/JaegerRemoteSampler.java | 17 ++++-- .../sampler/JaegerRemoteSamplerBuilder.java | 3 +- ...gerRemoteSamplerComponentProviderTest.java | 60 ------------------- .../sampler/JaegerRemoteSamplerTest.java | 14 +++-- .../JaegerRemoteSamplerGrpcNettyTest.java | 13 ++-- 6 files changed, 28 insertions(+), 81 deletions(-) delete mode 100644 sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerComponentProviderTest.java diff --git a/sdk-extensions/jaeger-remote-sampler/build.gradle.kts b/sdk-extensions/jaeger-remote-sampler/build.gradle.kts index 70e8d06607b..831774de06b 100644 --- a/sdk-extensions/jaeger-remote-sampler/build.gradle.kts +++ b/sdk-extensions/jaeger-remote-sampler/build.gradle.kts @@ -33,8 +33,6 @@ dependencies { testImplementation(project(":sdk:testing")) testImplementation(project(":sdk-extensions:autoconfigure")) - testImplementation(project(":sdk-extensions:declarative-config")) - testImplementation(project(":api:incubator")) testImplementation("com.google.guava:guava") testImplementation("com.google.protobuf:protobuf-java") testImplementation("com.linecorp.armeria:armeria-junit5") diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java index 94ba1907d1c..96d196da719 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java @@ -18,6 +18,7 @@ import io.opentelemetry.sdk.trace.samplers.Sampler; import io.opentelemetry.sdk.trace.samplers.SamplingResult; import java.io.IOException; +import java.net.URI; import java.util.List; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -45,15 +46,18 @@ public final class JaegerRemoteSampler implements Sampler { private final AtomicBoolean isShutdown = new AtomicBoolean(); private final GrpcSender grpcSender; + private final URI endpoint; private final int pollingIntervalMs; JaegerRemoteSampler( GrpcSender grpcSender, + URI endpoint, @Nullable String serviceName, int pollingIntervalMs, Sampler initialSampler) { this.serviceName = serviceName != null ? serviceName : ""; this.grpcSender = grpcSender; + this.endpoint = endpoint; this.pollingIntervalMs = pollingIntervalMs; this.sampler = initialSampler; pollExecutor = Executors.newScheduledThreadPool(1, new DaemonThreadFactory(WORKER_THREAD_NAME)); @@ -163,7 +167,13 @@ private static Sampler updateSampler(SamplingStrategyResponse response) throws I @Override public String getDescription() { - return String.format("JaegerRemoteSampler{%s}", this.sampler); + return "JaegerRemoteSampler{sampler=" + + this.sampler + + ", endpoint=" + + this.endpoint + + ", pollingIntervalMs=" + + this.pollingIntervalMs + + "}"; } @Override @@ -176,11 +186,6 @@ Sampler getSampler() { return this.sampler; } - // Visible for testing - int getPollingIntervalMs() { - return this.pollingIntervalMs; - } - public static JaegerRemoteSamplerBuilder builder() { return new JaegerRemoteSamplerBuilder(); } diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java index 2228956c47f..c6b2783e1a2 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java @@ -181,7 +181,8 @@ public JaegerRemoteSamplerBuilder setChannel(ManagedChannel channel) { */ public JaegerRemoteSampler build() { GrpcSender grpcSender = resolveGrpcSender(); - return new JaegerRemoteSampler(grpcSender, serviceName, pollingIntervalMillis, initialSampler); + return new JaegerRemoteSampler( + grpcSender, endpoint, serviceName, pollingIntervalMillis, initialSampler); } private GrpcSender resolveGrpcSender() { diff --git a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerComponentProviderTest.java b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerComponentProviderTest.java deleted file mode 100644 index 585c9439b99..00000000000 --- a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerComponentProviderTest.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.sdk.extension.trace.jaeger.sampler; - -import static org.assertj.core.api.Assertions.assertThat; - -import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; -import io.opentelemetry.internal.testing.slf4j.SuppressLogger; -import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfiguration; -import io.opentelemetry.sdk.extension.trace.jaeger.sampler.internal.JaegerRemoteSamplerComponentProvider; -import io.opentelemetry.sdk.trace.samplers.Sampler; -import java.io.ByteArrayInputStream; -import java.nio.charset.StandardCharsets; -import org.junit.jupiter.api.Test; - -// Connecting to the (unavailable) endpoint during the initial poll logs a warning; suppress it. -@SuppressLogger(JaegerRemoteSampler.class) -class JaegerRemoteSamplerComponentProviderTest { - - private static final int DEFAULT_POLLING_INTERVAL_MILLIS = 60000; - - @Test - void create_appliesConfiguredInterval() { - // Regression test for reading the wrong declarative config key: the property is "interval", so - // reading "internal" silently dropped the operator's value and fell back to the default. - Sampler sampler = - create( - "endpoint: http://localhost:14250\n" - + "interval: 10000\n" - + "initial_sampler:\n" - + " always_off: {}\n"); - try { - assertThat(((JaegerRemoteSampler) sampler).getPollingIntervalMs()).isEqualTo(10000); - } finally { - ((JaegerRemoteSampler) sampler).shutdown(); - } - } - - @Test - void create_defaultsIntervalWhenOmitted() { - Sampler sampler = - create("endpoint: http://localhost:14250\n" + "initial_sampler:\n" + " always_off: {}\n"); - try { - assertThat(((JaegerRemoteSampler) sampler).getPollingIntervalMs()) - .isEqualTo(DEFAULT_POLLING_INTERVAL_MILLIS); - } finally { - ((JaegerRemoteSampler) sampler).shutdown(); - } - } - - private static Sampler create(String yaml) { - DeclarativeConfigProperties config = - DeclarativeConfiguration.toConfigProperties( - new ByteArrayInputStream(yaml.getBytes(StandardCharsets.UTF_8))); - return new JaegerRemoteSamplerComponentProvider().create(config); - } -} diff --git a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerTest.java b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerTest.java index 88047b9f92d..4ed67399f87 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerTest.java +++ b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerTest.java @@ -284,7 +284,8 @@ void description() { assertThat(sampler).extracting("grpcSender").isInstanceOf(OkHttpGrpcSender.class); assertThat(sampler.getDescription()) - .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); + .startsWith("JaegerRemoteSampler{sampler=ParentBased{root:TraceIdRatioBased{0.001000}") + .contains("endpoint=" + server.httpUri(), "pollingIntervalMs=1000"); // wait until the sampling strategy is retrieved before exiting test method await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -301,7 +302,8 @@ void initialSampler() { .build()) { assertThat(sampler).extracting("grpcSender").isInstanceOf(OkHttpGrpcSender.class); - assertThat(sampler.getDescription()).startsWith("JaegerRemoteSampler{AlwaysOnSampler}"); + assertThat(sampler.getDescription()) + .startsWith("JaegerRemoteSampler{sampler=AlwaysOnSampler"); } } @@ -400,7 +402,7 @@ void perOperationSampling() { () -> { assertThat(sampler.getDescription()) .startsWith( - "JaegerRemoteSampler{ParentBased{root:PerOperationSampler{default=TraceIdRatioBased{0.550000}, perOperation={foo=TraceIdRatioBased{0.900000}, bar=TraceIdRatioBased{0.700000}}}"); + "JaegerRemoteSampler{sampler=ParentBased{root:PerOperationSampler{default=TraceIdRatioBased{0.550000}, perOperation={foo=TraceIdRatioBased{0.900000}, bar=TraceIdRatioBased{0.700000}}}"); assertThat(sampler.getDescription()).contains("bar"); }); } @@ -419,7 +421,7 @@ void internal_error_server_response() { assertThat(sampler).extracting("grpcSender").isInstanceOf(OkHttpGrpcSender.class); assertThat(sampler.getDescription()) - .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); + .startsWith("JaegerRemoteSampler{sampler=ParentBased{root:TraceIdRatioBased{0.001000}"); await() .untilAsserted( @@ -444,7 +446,7 @@ void unavailable_error_server_response() { assertThat(sampler).extracting("grpcSender").isInstanceOf(OkHttpGrpcSender.class); assertThat(sampler.getDescription()) - .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); + .startsWith("JaegerRemoteSampler{sampler=ParentBased{root:TraceIdRatioBased{0.001000}"); await() .untilAsserted( @@ -468,7 +470,7 @@ void unimplemented_error_server_response() { assertThat(sampler).extracting("grpcSender").isInstanceOf(OkHttpGrpcSender.class); assertThat(sampler.getDescription()) - .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); + .startsWith("JaegerRemoteSampler{sampler=ParentBased{root:TraceIdRatioBased{0.001000}"); await() .untilAsserted( diff --git a/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerGrpcNettyTest.java b/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerGrpcNettyTest.java index c2ddc1987ad..7ef47608f9f 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerGrpcNettyTest.java +++ b/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerGrpcNettyTest.java @@ -159,7 +159,7 @@ void description() { assertThat(sampler).extracting("grpcSender").isInstanceOf(UpstreamGrpcSender.class); assertThat(sampler.getDescription()) - .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); + .startsWith("JaegerRemoteSampler{sampler=ParentBased{root:TraceIdRatioBased{0.001000}"); // wait until the sampling strategy is retrieved before exiting test method await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -178,7 +178,8 @@ void initialSampler() { .build()) { assertThat(sampler).extracting("grpcSender").isInstanceOf(UpstreamGrpcSender.class); - assertThat(sampler.getDescription()).startsWith("JaegerRemoteSampler{AlwaysOnSampler}"); + assertThat(sampler.getDescription()) + .startsWith("JaegerRemoteSampler{sampler=AlwaysOnSampler"); } } @@ -277,7 +278,7 @@ void perOperationSampling() { () -> { assertThat(sampler.getDescription()) .startsWith( - "JaegerRemoteSampler{ParentBased{root:PerOperationSampler{default=TraceIdRatioBased{0.550000}, perOperation={foo=TraceIdRatioBased{0.900000}, bar=TraceIdRatioBased{0.700000}}}"); + "JaegerRemoteSampler{sampler=ParentBased{root:PerOperationSampler{default=TraceIdRatioBased{0.550000}, perOperation={foo=TraceIdRatioBased{0.900000}, bar=TraceIdRatioBased{0.700000}}}"); assertThat(sampler.getDescription()).contains("bar"); }); } @@ -297,7 +298,7 @@ void internal_error_server_response() { assertThat(sampler).extracting("grpcSender").isInstanceOf(UpstreamGrpcSender.class); assertThat(sampler.getDescription()) - .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); + .startsWith("JaegerRemoteSampler{sampler=ParentBased{root:TraceIdRatioBased{0.001000}"); await() .untilAsserted( @@ -323,7 +324,7 @@ void unavailable_error_server_response() { assertThat(sampler).extracting("grpcSender").isInstanceOf(UpstreamGrpcSender.class); assertThat(sampler.getDescription()) - .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); + .startsWith("JaegerRemoteSampler{sampler=ParentBased{root:TraceIdRatioBased{0.001000}"); await() .untilAsserted( @@ -348,7 +349,7 @@ void unimplemented_error_server_response() { assertThat(sampler).extracting("grpcSender").isInstanceOf(UpstreamGrpcSender.class); assertThat(sampler.getDescription()) - .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); + .startsWith("JaegerRemoteSampler{sampler=ParentBased{root:TraceIdRatioBased{0.001000}"); await() .untilAsserted(