From 71756ef19fa7ebe7624ab5412ec7ad1c3c1f2d29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Monkiewicz?= Date: Wed, 26 Oct 2022 12:58:32 +0200 Subject: [PATCH 1/3] Make description of Samplers Locale independent --- .../trace/jaeger/sampler/RateLimitingSampler.java | 12 +++++++++++- .../jaeger/sampler/RateLimitingSamplerTest.java | 9 ++++++++- .../trace/samplers/TraceIdRatioBasedSampler.java | 12 +++++++++++- .../samplers/TraceIdRatioBasedSamplerTest.java | 13 ++++++++++--- 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java index f7ae501957e..d8854a1ebbc 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java @@ -18,6 +18,8 @@ import io.opentelemetry.sdk.trace.samplers.Sampler; import io.opentelemetry.sdk.trace.samplers.SamplingDecision; import io.opentelemetry.sdk.trace.samplers.SamplingResult; +import java.text.DecimalFormat; +import java.text.DecimalFormatSymbols; import java.util.List; /** @@ -62,11 +64,19 @@ public SamplingResult shouldSample( @Override public String getDescription() { - return String.format("RateLimitingSampler{%.2f}", maxTracesPerSecond); + return String.format("RateLimitingSampler{%s}", decimalFormat(maxTracesPerSecond)); } @Override public String toString() { return getDescription(); } + + private static String decimalFormat(double value) { + DecimalFormatSymbols decimalFormatSymbols = DecimalFormatSymbols.getInstance(); + decimalFormatSymbols.setDecimalSeparator('.'); + + DecimalFormat decimalFormat = new DecimalFormat("0.00", decimalFormatSymbols); + return decimalFormat.format(value); + } } diff --git a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSamplerTest.java b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSamplerTest.java index 873e4c5c0b4..a7300f9ccaa 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSamplerTest.java +++ b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSamplerTest.java @@ -17,6 +17,7 @@ import io.opentelemetry.sdk.trace.samplers.SamplingDecision; import io.opentelemetry.sdk.trace.samplers.SamplingResult; import java.util.Collections; +import java.util.Locale; import org.junit.jupiter.api.Test; class RateLimitingSamplerTest { @@ -62,8 +63,14 @@ void sampleOneTrace() { } @Test - void description() { + void descriptionShouldBeLocaleIndependent() { RateLimitingSampler sampler = new RateLimitingSampler(15); + + // PL locale uses ',' as decimal separator + Locale.setDefault(Locale.forLanguageTag("PL")); + assertThat(sampler.getDescription()).isEqualTo("RateLimitingSampler{15.00}"); + + Locale.setDefault(Locale.ENGLISH); assertThat(sampler.getDescription()).isEqualTo("RateLimitingSampler{15.00}"); } } diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSampler.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSampler.java index b84dd374f39..9cf4e78d2c7 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSampler.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSampler.java @@ -10,6 +10,8 @@ import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; import io.opentelemetry.sdk.trace.data.LinkData; +import java.text.DecimalFormat; +import java.text.DecimalFormatSymbols; import java.util.List; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -52,7 +54,7 @@ static TraceIdRatioBasedSampler create(double ratio) { TraceIdRatioBasedSampler(double ratio, long idUpperBound) { this.idUpperBound = idUpperBound; - description = String.format("TraceIdRatioBased{%.6f}", ratio); + description = String.format("TraceIdRatioBased{%s}", decimalFormat(ratio)); } @Override @@ -108,4 +110,12 @@ long getIdUpperBound() { private static long getTraceIdRandomPart(String traceId) { return OtelEncodingUtils.longFromBase16String(traceId, 16); } + + private static String decimalFormat(double value) { + DecimalFormatSymbols decimalFormatSymbols = DecimalFormatSymbols.getInstance(); + decimalFormatSymbols.setDecimalSeparator('.'); + + DecimalFormat decimalFormat = new DecimalFormat("0.000000", decimalFormatSymbols); + return decimalFormat.format(value); + } } diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSamplerTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSamplerTest.java index 0c3a160c75a..a0279fa8613 100644 --- a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSamplerTest.java +++ b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSamplerTest.java @@ -19,6 +19,7 @@ import io.opentelemetry.sdk.trace.data.LinkData; import java.util.Collections; import java.util.List; +import java.util.Locale; import org.junit.jupiter.api.Test; class TraceIdRatioBasedSamplerTest { @@ -67,9 +68,15 @@ void outOfRangeLowProbability() { } @Test - void getDescription() { - assertThat(Sampler.traceIdRatioBased(0.5).getDescription()) - .isEqualTo(String.format("TraceIdRatioBased{%.6f}", 0.5)); + void descriptionShouldBeLocaleIndependent() { + Sampler sampler = Sampler.traceIdRatioBased(0.5); + + // PL locale uses ',' as decimal separator + Locale.setDefault(Locale.forLanguageTag("PL")); + assertThat(sampler.getDescription()).isEqualTo("TraceIdRatioBased{0.500000}"); + + Locale.setDefault(Locale.ENGLISH); + assertThat(sampler.getDescription()).isEqualTo("TraceIdRatioBased{0.500000}"); } @Test From 21860956d97daed3c11fe754050c95cecbc56ab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Monkiewicz?= Date: Wed, 26 Oct 2022 18:07:27 +0200 Subject: [PATCH 2/3] Switch from String.format to concatanation --- .../sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java | 2 +- .../sdk/trace/samplers/TraceIdRatioBasedSampler.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java index d8854a1ebbc..286377bc00e 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java @@ -64,7 +64,7 @@ public SamplingResult shouldSample( @Override public String getDescription() { - return String.format("RateLimitingSampler{%s}", decimalFormat(maxTracesPerSecond)); + return "RateLimitingSampler{" + decimalFormat(maxTracesPerSecond) + "}"; } @Override diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSampler.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSampler.java index 9cf4e78d2c7..59a63856318 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSampler.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/TraceIdRatioBasedSampler.java @@ -54,7 +54,7 @@ static TraceIdRatioBasedSampler create(double ratio) { TraceIdRatioBasedSampler(double ratio, long idUpperBound) { this.idUpperBound = idUpperBound; - description = String.format("TraceIdRatioBased{%s}", decimalFormat(ratio)); + description = "TraceIdRatioBased{" + decimalFormat(ratio) + "}"; } @Override From 0a477da0a61d53e8cea46b236c0de6bb5e8e11ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Monkiewicz?= Date: Thu, 27 Oct 2022 09:29:16 +0200 Subject: [PATCH 3/3] Moved description creation to constructor --- .../extension/trace/jaeger/sampler/RateLimitingSampler.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java index 286377bc00e..6a6caa171e6 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/RateLimitingSampler.java @@ -31,10 +31,10 @@ class RateLimitingSampler implements Sampler { static final AttributeKey SAMPLER_TYPE = stringKey("sampler.type"); static final AttributeKey SAMPLER_PARAM = doubleKey("sampler.param"); - private final double maxTracesPerSecond; private final RateLimiter rateLimiter; private final SamplingResult onSamplingResult; private final SamplingResult offSamplingResult; + private final String description; /** * Creates rate limiting sampler. @@ -42,13 +42,13 @@ class RateLimitingSampler implements Sampler { * @param maxTracesPerSecond the maximum number of sampled traces per second. */ RateLimitingSampler(int maxTracesPerSecond) { - this.maxTracesPerSecond = maxTracesPerSecond; double maxBalance = maxTracesPerSecond < 1.0 ? 1.0 : maxTracesPerSecond; this.rateLimiter = new RateLimiter(maxTracesPerSecond, maxBalance, Clock.getDefault()); Attributes attributes = Attributes.of(SAMPLER_TYPE, TYPE, SAMPLER_PARAM, (double) maxTracesPerSecond); this.onSamplingResult = SamplingResult.create(SamplingDecision.RECORD_AND_SAMPLE, attributes); this.offSamplingResult = SamplingResult.create(SamplingDecision.DROP, attributes); + description = "RateLimitingSampler{" + decimalFormat(maxTracesPerSecond) + "}"; } @Override @@ -64,7 +64,7 @@ public SamplingResult shouldSample( @Override public String getDescription() { - return "RateLimitingSampler{" + decimalFormat(maxTracesPerSecond) + "}"; + return description; } @Override