From 47b56543cf1897af53eca1061f1e3dbbf439ff5a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 21 Jan 2025 16:23:49 -0500 Subject: [PATCH 1/3] Small improvement to agent feedback sampling Previously, I updated this code to be case-insensitive. In doing so, I introduced a call to String.toLowerCase which had a negative impact on response time and allocation. By switching to TreeMap, I can use String::compareToIgnoreCase which avoids the allocation and has a better average complexity than toLowerCase. This change provides a 1-1.5% improvement in a span creation throughput stress tests. --- .../sampling/RateByServiceTraceSampler.java | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java index a0f4a6bd37d..980be22729e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java @@ -1,13 +1,12 @@ package datadog.trace.common.sampling; -import datadog.trace.api.cache.DDCache; -import datadog.trace.api.cache.DDCaches; -import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.sampling.SamplingMechanism; import datadog.trace.common.writer.RemoteResponseListener; import datadog.trace.core.CoreSpan; -import java.util.HashMap; +import java.util.Collections; import java.util.Map; +import java.util.TreeMap; + import java.util.function.Function; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -71,8 +70,8 @@ public void onResponse( } log.debug("Update service sampler rates: {} -> {}", endpoint, responseJson); - final Map> updatedEnvServiceRates = - new HashMap<>(newServiceRates.size() * 2); + final TreeMap> updatedEnvServiceRates = + new TreeMap<>(String::compareToIgnoreCase); RateSampler fallbackSampler = RateSamplersByEnvAndService.DEFAULT_SAMPLER; for (final Map.Entry entry : newServiceRates.entrySet()) { @@ -87,7 +86,7 @@ public void onResponse( } else { Map serviceRates = updatedEnvServiceRates.computeIfAbsent( - envAndService.lowerEnv, env -> new HashMap<>(newServiceRates.size() * 2)); + envAndService.lowerEnv, env -> new TreeMap<>(String::compareToIgnoreCase)); serviceRates.computeIfAbsent( envAndService.lowerService, @@ -114,38 +113,34 @@ private static RateSampler createRateSampler(final double sampleRate) { private static final class RateSamplersByEnvAndService { private static final RateSampler DEFAULT_SAMPLER = createRateSampler(DEFAULT_RATE); - private final Map> envServiceRates; + private final Map> envServiceRates; private final RateSampler fallbackSampler; RateSamplersByEnvAndService() { - this(new HashMap<>(0), DEFAULT_SAMPLER); + this(Collections.emptyMap(), DEFAULT_SAMPLER); } RateSamplersByEnvAndService( - Map> envServiceRates, RateSampler fallbackSampler) { + Map> envServiceRates, RateSampler fallbackSampler) { this.envServiceRates = envServiceRates; this.fallbackSampler = fallbackSampler; } // used in tests only RateSampler getSampler(EnvAndService envAndService) { - return getSamplerImpl(envAndService.lowerEnv, envAndService.lowerService); + return getSampler(envAndService.lowerEnv, envAndService.lowerService); } public RateSampler getSampler(String env, String service) { - return getSamplerImpl(env.toLowerCase(), service.toLowerCase()); - } - - private RateSampler getSamplerImpl(String lowerEnv, String lowerService) { - if (EnvAndService.isFallback(lowerEnv, lowerService)) { + if (EnvAndService.isFallback(env, service)) { return fallbackSampler; } - Map serviceRates = envServiceRates.get(lowerEnv); + Map serviceRates = envServiceRates.get(env); if (serviceRates == null) { return fallbackSampler; } - RateSampler sampler = serviceRates.get(lowerService); + RateSampler sampler = serviceRates.get(service); return null == sampler ? fallbackSampler : sampler; } } From 3758be5876629e5da6410a69e3c5b9c48fe92a45 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 22 Jan 2025 08:52:38 -0500 Subject: [PATCH 2/3] spotless --- .../datadog/trace/common/sampling/RateByServiceTraceSampler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java index 980be22729e..8456fe59040 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java @@ -6,7 +6,6 @@ import java.util.Collections; import java.util.Map; import java.util.TreeMap; - import java.util.function.Function; import org.slf4j.Logger; import org.slf4j.LoggerFactory; From 9a3fbb2a32f802482b1456e8187d8dcff430309a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 23 Jan 2025 09:37:29 -0500 Subject: [PATCH 3/3] Fixing missing imports - that my IDE settings screwed up --- .../trace/common/sampling/RateByServiceTraceSampler.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java index 8456fe59040..52957b643a0 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/RateByServiceTraceSampler.java @@ -1,5 +1,8 @@ package datadog.trace.common.sampling; +import datadog.trace.api.cache.DDCache; +import datadog.trace.api.cache.DDCaches; +import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.sampling.SamplingMechanism; import datadog.trace.common.writer.RemoteResponseListener; import datadog.trace.core.CoreSpan;