diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/ProbeRateLimiter.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/ProbeRateLimiter.java index e7c7aa22f8d..fc991a5315b 100644 --- a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/ProbeRateLimiter.java +++ b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/ProbeRateLimiter.java @@ -5,8 +5,6 @@ import datadog.trace.api.sampling.Sampler; import java.time.Duration; import java.time.temporal.ChronoUnit; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.function.DoubleFunction; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -20,19 +18,15 @@ public class ProbeRateLimiter { private static final Duration TEN_SECONDS_WINDOW = Duration.of(10, ChronoUnit.SECONDS); private static final double DEFAULT_GLOBAL_SNAPSHOT_RATE = DEFAULT_SNAPSHOT_RATE * 100; private static final double DEFAULT_GLOBAL_LOG_RATE = 5000.0; - private static final ConcurrentMap PROBE_SAMPLERS = - new ConcurrentHashMap<>(); - private static Sampler GLOBAL_SNAPSHOT_SAMPLER = createSampler(DEFAULT_GLOBAL_SNAPSHOT_RATE); - private static Sampler GLOBAL_LOG_SAMPLER = createSampler(DEFAULT_GLOBAL_LOG_RATE); - private static DoubleFunction samplerSupplier = ProbeRateLimiter::createSampler; + private static Sampler GLOBAL_SNAPSHOT_SAMPLER = + defaultCreateSampler(DEFAULT_GLOBAL_SNAPSHOT_RATE); + private static Sampler GLOBAL_LOG_SAMPLER = defaultCreateSampler(DEFAULT_GLOBAL_LOG_RATE); + private static DoubleFunction samplerSupplier = ProbeRateLimiter::defaultCreateSampler; - public static boolean tryProbe(String probeId) { - RateLimitInfo rateLimitInfo = - PROBE_SAMPLERS.computeIfAbsent(probeId, ProbeRateLimiter::getDefaultRateLimitInfo); - Sampler globalSampler = - rateLimitInfo.isCaptureSnapshot ? GLOBAL_SNAPSHOT_SAMPLER : GLOBAL_LOG_SAMPLER; + public static boolean tryProbe(Sampler sampler, boolean useGlobalLowRate) { + Sampler globalSampler = useGlobalLowRate ? GLOBAL_SNAPSHOT_SAMPLER : GLOBAL_LOG_SAMPLER; if (globalSampler.sample()) { - return rateLimitInfo.sampler.sample(); + return sampler.sample(); } return false; } @@ -42,8 +36,8 @@ private static RateLimitInfo getDefaultRateLimitInfo(String probeId) { return new RateLimitInfo(samplerSupplier.apply(DEFAULT_SNAPSHOT_RATE), true); } - public static void setRate(String probeId, double rate, boolean isCaptureSnapshot) { - PROBE_SAMPLERS.put(probeId, new RateLimitInfo(samplerSupplier.apply(rate), isCaptureSnapshot)); + public static Sampler createSampler(double rate) { + return samplerSupplier.apply(rate); } public static void setGlobalSnapshotRate(double rate) { @@ -54,25 +48,16 @@ public static void setGlobalLogRate(double rate) { GLOBAL_LOG_SAMPLER = samplerSupplier.apply(rate); } - public static void resetRate(String probeId) { - PROBE_SAMPLERS.remove(probeId); - } - public static void resetGlobalRate() { setGlobalSnapshotRate(DEFAULT_GLOBAL_LOG_RATE); } - public static void resetAll() { - PROBE_SAMPLERS.clear(); - resetGlobalRate(); - } - public static void setSamplerSupplier(DoubleFunction samplerSupplier) { ProbeRateLimiter.samplerSupplier = - samplerSupplier != null ? samplerSupplier : ProbeRateLimiter::createSampler; + samplerSupplier != null ? samplerSupplier : ProbeRateLimiter::defaultCreateSampler; } - private static Sampler createSampler(double rate) { + private static Sampler defaultCreateSampler(double rate) { if (rate < 0) { return new ConstantSampler(true); } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java index 16c8f0c03ad..e66e52404ec 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java @@ -10,7 +10,6 @@ import com.datadog.debugger.probe.LogProbe; import com.datadog.debugger.probe.ProbeDefinition; import com.datadog.debugger.probe.Sampled; -import com.datadog.debugger.probe.Sampling; import com.datadog.debugger.sink.DebuggerSink; import com.datadog.debugger.util.ExceptionHelper; import datadog.trace.api.Config; @@ -136,8 +135,8 @@ private void applyNewConfiguration(Configuration newConfiguration) { new ConfigurationComparer( originalConfiguration, newConfiguration, instrumentationResults); if (changes.hasRateLimitRelatedChanged()) { - // apply rate limit config first to avoid racing with execution/instrumentation of log - // probes + // apply rate limit config first to avoid racing with execution/instrumentation + // of probes requiring samplers applyRateLimiter(changes, newConfiguration.getSampling()); } currentConfiguration = newConfiguration; @@ -282,18 +281,7 @@ private static void applyRateLimiter( for (ProbeDefinition added : changes.getAddedDefinitions()) { if (added instanceof Sampled) { Sampled probe = (Sampled) added; - Sampling sampling = probe.getSampling(); - double rate = getDefaultRateLimitPerProbe(probe); - if (sampling != null && sampling.getEventsPerSecond() != 0) { - rate = sampling.getEventsPerSecond(); - } - ProbeRateLimiter.setRate(probe.getId(), rate, probe.isCaptureSnapshot()); - } - } - // remove rate for all removed probes - for (ProbeDefinition removedDefinition : changes.getRemovedDefinitions()) { - if (removedDefinition instanceof LogProbe) { - ProbeRateLimiter.resetRate(removedDefinition.getId()); + probe.initSamplers(); } } // set global sampling @@ -302,12 +290,6 @@ private static void applyRateLimiter( } } - private static double getDefaultRateLimitPerProbe(Sampled probe) { - return probe.isCaptureSnapshot() - ? ProbeRateLimiter.DEFAULT_SNAPSHOT_RATE - : ProbeRateLimiter.DEFAULT_LOG_RATE; - } - private void removeCurrentTransformer() { if (currentTransformer == null) { return; diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java index 99751bab13b..63a568b7775 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java @@ -370,6 +370,7 @@ private void createMethodProbe(MethodInfo methodInfo, List prob .where(methodInfo.getClassNode().name, methodInfo.getMethodNode().name) .captureSnapshot(false) .build(); + probe.initSamplers(); probes.add(probe); } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ExceptionProbe.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ExceptionProbe.java index 7be31c4eb2a..861683c375a 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ExceptionProbe.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ExceptionProbe.java @@ -50,6 +50,7 @@ public ExceptionProbe( null); this.exceptionProbeManager = exceptionProbeManager; this.chainedExceptionIdx = chainedExceptionIdx; + initSamplers(); } @Override diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/LogProbe.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/LogProbe.java index 1b394c3a1d8..f19fd8b24b3 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/LogProbe.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/LogProbe.java @@ -25,6 +25,7 @@ import datadog.trace.api.Config; import datadog.trace.api.CorrelationIdentifier; import datadog.trace.api.DDTraceId; +import datadog.trace.api.sampling.Sampler; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.CapturedContextProbe; import datadog.trace.bootstrap.debugger.DebuggerContext; @@ -323,6 +324,7 @@ public String toString() { private transient Consumer snapshotProcessor; protected transient Map budget = Collections.synchronizedMap(new WeakIdentityHashMap<>()); + protected transient Sampler sampler; // no-arg constructor is required by Moshi to avoid creating instance with unsafe and by-passing // constructors, including field initializers. @@ -408,6 +410,7 @@ public LogProbe(LogProbe.Builder builder) { builder.sampling, builder.captureExpressions); this.snapshotProcessor = builder.snapshotProcessor; + initSamplers(); } public LogProbe copy() { @@ -450,6 +453,16 @@ public Sampling getSampling() { return sampling; } + public void initSamplers() { + double rate = + sampling != null + ? sampling.getEventsPerSecond() + : (isCaptureSnapshot() + ? ProbeRateLimiter.DEFAULT_SNAPSHOT_RATE + : ProbeRateLimiter.DEFAULT_LOG_RATE); + sampler = ProbeRateLimiter.createSampler(rate); + } + public List getCaptureExpressions() { return captureExpressions; } @@ -487,7 +500,7 @@ public InstrumentationResult.Status instrument( public boolean isReadyToCapture() { if (!hasCondition()) { // we are sampling here to avoid creating CapturedContext when the sampling result is negative - return ProbeRateLimiter.tryProbe(id); + return ProbeRateLimiter.tryProbe(sampler, isCaptureSnapshot()); } return true; } @@ -553,7 +566,8 @@ private void sample(LogStatus logStatus, MethodLocation methodLocation) { return; } boolean sampled = - !logStatus.getDebugSessionStatus().isDisabled() && ProbeRateLimiter.tryProbe(id); + !logStatus.getDebugSessionStatus().isDisabled() + && ProbeRateLimiter.tryProbe(sampler, isCaptureSnapshot()); logStatus.setSampled(sampled); if (!sampled) { DebuggerAgent.getSink() diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/Sampled.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/Sampled.java index 6787df09529..5fbdf25b789 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/Sampled.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/Sampled.java @@ -1,9 +1,5 @@ package com.datadog.debugger.probe; public interface Sampled { - Sampling getSampling(); - - String getId(); - - boolean isCaptureSnapshot(); + void initSamplers(); } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/SpanDecorationProbe.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/SpanDecorationProbe.java index 3961bf5f9c0..a4ed8a2fb12 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/SpanDecorationProbe.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/SpanDecorationProbe.java @@ -11,6 +11,7 @@ import com.datadog.debugger.instrumentation.MethodInfo; import com.datadog.debugger.sink.Snapshot; import datadog.trace.api.Pair; +import datadog.trace.api.sampling.Sampler; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.CapturedContextProbe; import datadog.trace.bootstrap.debugger.EvaluationError; @@ -30,7 +31,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class SpanDecorationProbe extends ProbeDefinition implements CapturedContextProbe { +public class SpanDecorationProbe extends ProbeDefinition implements CapturedContextProbe, Sampled { private static final Logger LOGGER = LoggerFactory.getLogger(SpanDecorationProbe.class); private static final String PROBEID_DD_TAGS_FORMAT = "_dd.di.%s.probe_id"; private static final String EVALERROR_DD_TAGS_FORMAT = "_dd.di.%s.evaluation_error"; @@ -157,6 +158,7 @@ public int hashCode() { private final TargetSpan targetSpan; private final List decorations; + private transient Sampler errorSampler; // no-arg constructor is required by Moshi to avoid creating instance with unsafe and by-passing // constructors, including field initializers. @@ -295,7 +297,7 @@ private void handleEvaluationErrors(SpanDecorationStatus status) { if (status.getErrors().isEmpty()) { return; } - boolean sampled = ProbeRateLimiter.tryProbe(id); + boolean sampled = ProbeRateLimiter.tryProbe(errorSampler, true); if (!sampled) { return; } @@ -317,6 +319,11 @@ public List getDecorations() { return decorations; } + @Override + public void initSamplers() { + errorSampler = ProbeRateLimiter.createSampler(1.0); + } + @Generated @Override public int hashCode() { diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/TriggerProbe.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/TriggerProbe.java index 4b7972de6aa..78eae9dd9e2 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/TriggerProbe.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/TriggerProbe.java @@ -9,6 +9,7 @@ import com.datadog.debugger.instrumentation.DiagnosticMessage; import com.datadog.debugger.instrumentation.InstrumentationResult; import com.datadog.debugger.instrumentation.MethodInfo; +import datadog.trace.api.sampling.Sampler; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.CapturedContextProbe; import datadog.trace.bootstrap.debugger.MethodLocation; @@ -29,6 +30,7 @@ public class TriggerProbe extends ProbeDefinition implements Sampled, CapturedCo private ProbeCondition probeCondition; private Sampling sampling; private String sessionId; + private transient Sampler sampler; // no-arg constructor is required by Moshi to avoid creating instance with unsafe and by-passing // constructors, including field initializers. @@ -68,6 +70,12 @@ public Sampling getSampling() { return sampling; } + @Override + public void initSamplers() { + double rate = sampling != null ? sampling.getEventsPerSecond() : 1.0; + sampler = ProbeRateLimiter.createSampler(rate); + } + @Override public boolean isCaptureSnapshot() { return false; @@ -104,7 +112,8 @@ public void evaluate( if (sampling == null || !sampling.inCoolDown()) { boolean sample = true; if (!hasCondition()) { - sample = MethodLocation.isSame(location, evaluateAt) && ProbeRateLimiter.tryProbe(id); + sample = + MethodLocation.isSame(location, evaluateAt) && ProbeRateLimiter.tryProbe(sampler, true); } boolean value = evaluateCondition(context); diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturingTestBase.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturingTestBase.java index 3af160c2e09..4fa605c2cc1 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturingTestBase.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturingTestBase.java @@ -11,6 +11,7 @@ import com.datadog.debugger.instrumentation.InstrumentationResult; import com.datadog.debugger.probe.LogProbe; import com.datadog.debugger.probe.ProbeDefinition; +import com.datadog.debugger.probe.Sampled; import com.datadog.debugger.sink.DebuggerSink; import com.datadog.debugger.sink.ProbeStatusSink; import com.datadog.debugger.util.MoshiHelper; @@ -64,7 +65,7 @@ public void after() { if (currentTransformer != null) { instr.removeTransformer(currentTransformer); } - ProbeRateLimiter.resetAll(); + ProbeRateLimiter.resetGlobalRate(); Assertions.assertFalse(DebuggerContext.isInProbe()); Redaction.clearUserDefinedTypes(); } @@ -354,10 +355,9 @@ protected TestSnapshotListener installProbes(Configuration configuration) { DebuggerContext.initClassFilter(new DenyListHelper(null)); DebuggerContext.initValueSerializer(new JsonSnapshotSerializer()); - for (LogProbe probe : configuration.getLogProbes()) { - if (probe.getSampling() != null) { - ProbeRateLimiter.setRate( - probe.getId(), probe.getSampling().getEventsPerSecond(), probe.isCaptureSnapshot()); + for (ProbeDefinition probe : configuration.getDefinitions()) { + if (probe instanceof Sampled) { + ((Sampled) probe).initSamplers(); } } if (configuration.getSampling() != null) { diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java index 2cb3509af26..6858a40f40f 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java @@ -13,6 +13,8 @@ import com.datadog.debugger.el.DSL; import com.datadog.debugger.el.ProbeCondition; import com.datadog.debugger.probe.LogProbe; +import com.datadog.debugger.probe.ProbeDefinition; +import com.datadog.debugger.probe.Sampled; import com.datadog.debugger.sink.ProbeStatusSink; import com.datadog.debugger.sink.Snapshot; import com.datadog.debugger.util.TestSnapshotListener; @@ -55,7 +57,7 @@ public void after() { if (currentTransformer != null) { instr.removeTransformer(currentTransformer); } - ProbeRateLimiter.resetAll(); + ProbeRateLimiter.resetGlobalRate(); } @Test @@ -564,6 +566,11 @@ private TestSnapshotListener installProbes(Configuration configuration) { .thenReturn("http://localhost:8126/debugger/v1/input"); when(config.getFinalDebuggerSymDBUrl()).thenReturn("http://localhost:8126/symdb/v1/input"); when(config.getDynamicInstrumentationUploadBatchSize()).thenReturn(100); + for (ProbeDefinition probe : configuration.getDefinitions()) { + if (probe instanceof Sampled) { + ((Sampled) probe).initSamplers(); + } + } ProbeMetadata probeMetadata = new ProbeMetadata(); currentTransformer = new DebuggerTransformer(config, probeMetadata, configuration); instr.addTransformer(currentTransformer); diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java index f7febac94c9..d70d8d4c1d3 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java @@ -28,6 +28,7 @@ import com.datadog.debugger.el.values.StringValue; import com.datadog.debugger.probe.LogProbe; import com.datadog.debugger.probe.ProbeDefinition; +import com.datadog.debugger.probe.Sampled; import com.datadog.debugger.probe.SpanDecorationProbe; import com.datadog.debugger.sink.DebuggerSink; import com.datadog.debugger.sink.ProbeStatusSink; @@ -79,7 +80,7 @@ public void setUp() { public void after() { super.after(); Redaction.clearUserDefinedTypes(); - ProbeRateLimiter.resetAll(); + ProbeRateLimiter.resetGlobalRate(); } @Test @@ -758,6 +759,11 @@ private void installSpanDecorationProbes(String expectedClassName, Configuration .thenReturn("http://localhost:8126/debugger/v1/input"); when(config.getFinalDebuggerSymDBUrl()).thenReturn("http://localhost:8126/symdb/v1/input"); probeStatusSink = mock(ProbeStatusSink.class); + for (ProbeDefinition probe : configuration.getDefinitions()) { + if (probe instanceof Sampled) { + ((Sampled) probe).initSamplers(); + } + } ProbeMetadata probeMetadata = new ProbeMetadata(); currentTransformer = new DebuggerTransformer( diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java index af925cc0009..e72e14c4201 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java @@ -114,7 +114,7 @@ public void after() { instr.removeTransformer(currentTransformer); } ProbeRateLimiter.setSamplerSupplier(null); - ProbeRateLimiter.resetAll(); + ProbeRateLimiter.resetGlobalRate(); } @Test diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/probe/LogProbeTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/probe/LogProbeTest.java index 16d9ceb0811..07b7ae05185 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/probe/LogProbeTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/probe/LogProbeTest.java @@ -143,7 +143,7 @@ private int runTrace(TracerAPI tracer, boolean captureSnapshot, Integer line, St builder.tags("session_id:" + sessionId); } LogProbe logProbe = builder.build(); - ProbeRateLimiter.setRate(logProbe.id, -1, captureSnapshot); + logProbe.initSamplers(); CapturedContext entryContext = capturedContext(span, logProbe); CapturedContext exitContext = capturedContext(span, logProbe); diff --git a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/BaseIntegrationTest.java b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/BaseIntegrationTest.java index 28671e1157c..cc35394a502 100644 --- a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/BaseIntegrationTest.java +++ b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/BaseIntegrationTest.java @@ -140,7 +140,7 @@ void teardown(TestInfo testInfo) throws Exception { } datadogAgentServer.shutdown(); statsDServer.close(); - ProbeRateLimiter.resetAll(); + ProbeRateLimiter.resetGlobalRate(); LOG.info("===== Ending {} ====", testInfo.getDisplayName()); }