From 7381c04fe7d9e7cef4c73c72b58a2f2ceca5585d Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Wed, 26 Feb 2025 13:48:07 -0500 Subject: [PATCH 1/3] don't instrument trigger probes if the distributed debugger is disabled --- .../debugger/el/EvaluationException.java | 18 +++ .../debugger/agent/DebuggerTransformer.java | 5 +- .../datadog/debugger/probe/TriggerProbe.java | 5 +- .../debugger/trigger/TriggerProbeTest.java | 149 ++++++++++++------ 4 files changed, 125 insertions(+), 52 deletions(-) diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationException.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationException.java index 4cb9cdb555b..68ec5814dac 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationException.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationException.java @@ -1,5 +1,7 @@ package com.datadog.debugger.el; +import java.util.Objects; + public class EvaluationException extends RuntimeException { private final String expr; @@ -16,4 +18,20 @@ public EvaluationException(String message, String expr, Throwable cause) { public String getExpr() { return expr; } + + @Override + public final boolean equals(Object o) { + if (!(o instanceof EvaluationException)) { + return false; + } + + EvaluationException that = (EvaluationException) o; + return Objects.equals(getMessage(), that.getMessage()) + && Objects.equals(getExpr(), that.getExpr()); + } + + @Override + public int hashCode() { + return Objects.hashCode(getExpr()); + } } 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 0a35c845efa..8cc150d980e 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 @@ -634,7 +634,10 @@ private List filterAndSortDefinitions( // Log and span decoration probe shared the same instrumentor: CaptureContextInstrumentor // and therefore need to be instrumented once // note: exception probes are log probes and are handled the same way - if (isCapturedContextProbe(definition)) { + if (!Config.get().isDistributedDebuggerEnabled() && definition instanceof TriggerProbe) { + log.debug( + "The distributed debugger feature is disabled. Trigger probes will not be installed."); + } else if (isCapturedContextProbe(definition)) { if (definition.isLineProbe()) { capturedContextLineProbes .computeIfAbsent(definition.getWhere(), key -> new ArrayList<>()) 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 e29e691459e..b4dd186dabf 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 @@ -59,10 +59,6 @@ public InstrumentationResult.Status instrument( .instrument(); } - public String getSessionId() { - return sessionId; - } - public TriggerProbe setSessionId(String sessionId) { this.sessionId = sessionId; return this; @@ -86,6 +82,7 @@ public TriggerProbe setProbeCondition(ProbeCondition probeCondition) { public void evaluate( CapturedContext context, CapturedContext.Status status, MethodLocation location) { + Sampling sampling = getSampling(); if (sampling == null || !sampling.inCoolDown()) { boolean sample = true; if (!hasCondition()) { diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/trigger/TriggerProbeTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/trigger/TriggerProbeTest.java index 2651a729859..f7795ab9857 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/trigger/TriggerProbeTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/trigger/TriggerProbeTest.java @@ -5,11 +5,14 @@ import static java.lang.String.format; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; import static utils.InstrumentationTestHelper.compileAndLoadClass; import com.datadog.debugger.agent.CapturingTestBase; import com.datadog.debugger.agent.Configuration; import com.datadog.debugger.agent.MockSampler; +import com.datadog.debugger.el.EvaluationException; import com.datadog.debugger.el.ProbeCondition; import com.datadog.debugger.probe.Sampling; import com.datadog.debugger.probe.TriggerProbe; @@ -50,6 +53,53 @@ public void before() { setFieldInConfig(Config.get(), "debuggerCodeOriginEnabled", true); setFieldInConfig(InstrumenterConfig.get(), "codeOriginEnabled", true); + setFieldInConfig(Config.get(), "distributedDebuggerEnabled", true); + } + + @Test + public void conditions() throws IOException, URISyntaxException { + final String className = "com.datadog.debugger.TriggerProbe02"; + TriggerProbe probe1 = + createTriggerProbe( + TRIGGER_PROBE_ID1, + TRIGGER_PROBE_SESSION_ID, + className, + "entry", + "(int)", + new ProbeCondition(when(lt(ref("value"), value(25))), "value < 25"), + new Sampling(10.0)); + installProbes(Configuration.builder().setService(SERVICE_NAME).add(probe1).build()); + Class testClass = compileAndLoadClass(className); + for (int i = 0; i < 100; i++) { + Reflect.onClass(testClass).call("main", i).get(); + } + List> allTraces = traceInterceptor.getAllTraces(); + long count = + allTraces.stream() + .map(span -> span.get(0)) + .filter( + span -> { + DDSpan ddSpan = (DDSpan) span; + PropagationTags tags = ddSpan.context().getPropagationTags(); + return (TRIGGER_PROBE_SESSION_ID + ":1").equals(tags.getDebugPropagation()); + }) + .count(); + assertEquals(100, allTraces.size(), "actual traces: " + allTraces.size()); + assertTrue(count <= 25, "Should have at most 25 debug sessions. found: " + count); + } + + private static TriggerProbe createTriggerProbe( + ProbeId id, + String sessionId, + String typeName, + String methodName, + String signature, + ProbeCondition probeCondition, + Sampling sampling) { + return new TriggerProbe(id, Where.of(typeName, methodName, signature)) + .setSessionId(sessionId) + .setProbeCondition(probeCondition) + .setSampling(sampling); } @Test @@ -104,6 +154,58 @@ public void cooldown() throws IOException, URISyntaxException { } } + @Test + public void badCondition() throws IOException, URISyntaxException { + String className = "com.datadog.debugger.TriggerProbe02"; + TriggerProbe probe1 = + createTriggerProbe( + TRIGGER_PROBE_ID1, + TRIGGER_PROBE_SESSION_ID, + className, + "entry", + "(int)", + new ProbeCondition(when(lt(ref("limit"), value(25))), "limit < 25"), + new Sampling(10.0)); + + installProbes(Configuration.builder().setService(SERVICE_NAME).add(probe1).build()); + Class testClass = compileAndLoadClass(className); + Reflect.onClass(testClass).call("main", 0).get(); + verify(probeStatusSink) + .addError( + eq(TRIGGER_PROBE_ID1), + eq(new EvaluationException("Cannot find symbol: limit", "limit"))); + } + + @Test + public void debuggerDisabled() throws IOException, URISyntaxException { + boolean original = Config.get().isDistributedDebuggerEnabled(); + try { + setFieldInConfig(Config.get(), "distributedDebuggerEnabled", false); + + MockSampler sampler = new MockSampler(); + ProbeRateLimiter.setSamplerSupplier(value -> sampler); + + final String className = "com.datadog.debugger.TriggerProbe02"; + TriggerProbe probe1 = + createTriggerProbe( + TRIGGER_PROBE_ID1, + TRIGGER_PROBE_SESSION_ID, + className, + "entry", + "(int)", + new ProbeCondition(when(lt(ref("value"), value(25))), "value < 25"), + new Sampling(10.0)); + installProbes(Configuration.builder().setService(SERVICE_NAME).add(probe1).build()); + Class testClass = compileAndLoadClass(className); + Reflect.onClass(testClass).call("main", 0).get(); + + assertEquals(0, sampler.getCallCount()); + } finally { + setFieldInConfig(Config.get(), "distributedDebuggerEnabled", original); + ProbeRateLimiter.setSamplerSupplier(null); + } + } + @Test public void sampling() throws IOException, URISyntaxException { try { @@ -132,51 +234,4 @@ public void sampling() throws IOException, URISyntaxException { ProbeRateLimiter.setSamplerSupplier(null); } } - - @Test - public void conditions() throws IOException, URISyntaxException { - - final String className = "com.datadog.debugger.TriggerProbe02"; - TriggerProbe probe1 = - createTriggerProbe( - TRIGGER_PROBE_ID1, - TRIGGER_PROBE_SESSION_ID, - className, - "entry", - "(int)", - new ProbeCondition(when(lt(ref("value"), value(25))), "value < 25"), - new Sampling(10.0)); - installProbes(Configuration.builder().setService(SERVICE_NAME).add(probe1).build()); - Class testClass = compileAndLoadClass(className); - for (int i = 0; i < 100; i++) { - Reflect.onClass(testClass).call("main", i).get(); - } - List> allTraces = traceInterceptor.getAllTraces(); - long count = - allTraces.stream() - .map(span -> span.get(0)) - .filter( - span -> { - DDSpan ddSpan = (DDSpan) span; - PropagationTags tags = ddSpan.context().getPropagationTags(); - return (TRIGGER_PROBE_SESSION_ID + ":1").equals(tags.getDebugPropagation()); - }) - .count(); - assertEquals(100, allTraces.size(), "actual traces: " + allTraces.size()); - assertTrue(count <= 25, "Should have at most 25 debug sessions. found: " + count); - } - - public static TriggerProbe createTriggerProbe( - ProbeId id, - String sessionId, - String typeName, - String methodName, - String signature, - ProbeCondition probeCondition, - Sampling sampling) { - return new TriggerProbe(id, Where.of(typeName, methodName, signature)) - .setSessionId(sessionId) - .setProbeCondition(probeCondition) - .setSampling(sampling); - } } From 3656177e1ef2544b41810e43fc08fa69fd6f553e Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Fri, 28 Feb 2025 08:59:55 -0500 Subject: [PATCH 2/3] include the message in exception hash for symmetry with equals() another test case for jacoco --- .../debugger/el/EvaluationException.java | 8 ++++--- .../debugger/trigger/TriggerProbeTest.java | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationException.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationException.java index 68ec5814dac..06b09b8e25f 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationException.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationException.java @@ -26,12 +26,14 @@ public final boolean equals(Object o) { } EvaluationException that = (EvaluationException) o; - return Objects.equals(getMessage(), that.getMessage()) - && Objects.equals(getExpr(), that.getExpr()); + return Objects.equals(getExpr(), that.getExpr()) + && Objects.equals(getMessage(), that.getMessage()); } @Override public int hashCode() { - return Objects.hashCode(getExpr()); + int result = Objects.hashCode(getExpr()); + result = 31 * result + Objects.hashCode(getMessage()); + return result; } } diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/trigger/TriggerProbeTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/trigger/TriggerProbeTest.java index f7795ab9857..367080a068d 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/trigger/TriggerProbeTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/trigger/TriggerProbeTest.java @@ -234,4 +234,27 @@ public void sampling() throws IOException, URISyntaxException { ProbeRateLimiter.setSamplerSupplier(null); } } + + @Test + public void noSampling() throws IOException, URISyntaxException { + try { + MockSampler sampler = new MockSampler(); + ProbeRateLimiter.setSamplerSupplier(value -> sampler); + + final String className = "com.datadog.debugger.TriggerProbe01"; + TriggerProbe probe1 = + createTriggerProbe( + TRIGGER_PROBE_ID1, TRIGGER_PROBE_SESSION_ID, className, "entry", "()", null, null); + Configuration config = Configuration.builder().setService(SERVICE_NAME).add(probe1).build(); + installProbes(config); + Class testClass = compileAndLoadClass(className); + for (int i = 0; i < 100; i++) { + Reflect.onClass(testClass).call("main", "").get(); + } + + assertTrue(sampler.getCallCount() != 0); + } finally { + ProbeRateLimiter.setSamplerSupplier(null); + } + } } From 88a893c3ef39003b50cff93f55d302becf0357ab Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Fri, 28 Feb 2025 11:29:55 -0500 Subject: [PATCH 3/3] exclude exception from coverage checks --- dd-java-agent/agent-debugger/debugger-el/build.gradle | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-debugger/debugger-el/build.gradle b/dd-java-agent/agent-debugger/debugger-el/build.gradle index fe2653ca9ab..ff9d5d44187 100644 --- a/dd-java-agent/agent-debugger/debugger-el/build.gradle +++ b/dd-java-agent/agent-debugger/debugger-el/build.gradle @@ -12,7 +12,8 @@ excludedClassesCoverage += [ 'com.datadog.debugger.el.Script*', 'com.datadog.debugger.el.ValueScript*', 'com.datadog.debugger.el.values.CollectionValue*', - 'com.datadog.debugger.el.InvalidValueException' + 'com.datadog.debugger.el.InvalidValueException', + 'com.datadog.debugger.el.EvaluationException' ] dependencies {