From d2e70dc2aedcffafa9684d294f7fa576f4e4b6a7 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Wed, 15 Jan 2025 15:45:12 -0500 Subject: [PATCH 1/4] Enhance log probes to honor debug session tags --- .../bootstrap/debugger/DebuggerContext.java | 22 ++++- .../com/datadog/debugger/probe/LogProbe.java | 61 +++++++++++++- .../debugger/probe/ProbeDefinition.java | 51 ++++++++++- .../datadog/debugger/sink/DebuggerSink.java | 18 +--- .../debugger/agent/CapturingTestBase.java | 22 +++-- .../datadog/debugger/probe/LogProbeTest.java | 84 +++++++++++++++++++ 6 files changed, 228 insertions(+), 30 deletions(-) diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java index 42012b25001..663cd88033f 100644 --- a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java +++ b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java @@ -21,8 +21,26 @@ public class DebuggerContext { private static final ThreadLocal IN_PROBE = ThreadLocal.withInitial(() -> Boolean.FALSE); public enum SkipCause { - RATE, - CONDITION + RATE { + @Override + public String tag() { + return "cause:rate"; + } + }, + CONDITION { + @Override + public String tag() { + return "cause:condition"; + } + }, + DEBUG_SESSION_DISABLED { + @Override + public String tag() { + return "cause:debug session disabled"; + } + }; + + public abstract String tag(); } public interface ProbeResolver { 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 c9896878b4d..e831f815e6a 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 @@ -40,6 +40,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.function.Consumer; import org.slf4j.Logger; @@ -434,7 +435,8 @@ public void evaluate( // sample when no condition associated sample(logStatus, methodLocation); } - logStatus.setCondition(evaluateCondition(context, logStatus)); + logStatus.setCondition( + !logStatus.getDebugSessionStatus().isDisabled() && evaluateCondition(context, logStatus)); CapturedContext.CapturedThrowable throwable = context.getCapturedThrowable(); if (logStatus.hasConditionErrors() && throwable != null) { logStatus.addError( @@ -503,10 +505,16 @@ private void sample(LogStatus logStatus, MethodLocation methodLocation) { if (!MethodLocation.isSame(methodLocation, evaluateAt)) { return; } - boolean sampled = ProbeRateLimiter.tryProbe(id); + boolean sampled = + !logStatus.getDebugSessionStatus().isDisabled() && ProbeRateLimiter.tryProbe(id); logStatus.setSampled(sampled); if (!sampled) { - DebuggerAgent.getSink().skipSnapshot(id, DebuggerContext.SkipCause.RATE); + DebuggerAgent.getSink() + .skipSnapshot( + id, + logStatus.getDebugSessionStatus().isDisabled() + ? DebuggerContext.SkipCause.DEBUG_SESSION_DISABLED + : DebuggerContext.SkipCause.RATE); } } @@ -685,6 +693,7 @@ public static class LogStatus extends CapturedContext.Status { new LogStatus(ProbeImplementation.UNKNOWN, true); private boolean condition = true; + private final DebugSessionStatus debugSessionStatus; private boolean hasLogTemplateErrors; private boolean hasConditionErrors; private boolean sampled = true; @@ -693,10 +702,11 @@ public static class LogStatus extends CapturedContext.Status { public LogStatus(ProbeImplementation probeImplementation) { super(probeImplementation); + debugSessionStatus = debugSessionStatus(); } private LogStatus(ProbeImplementation probeImplementation, boolean condition) { - super(probeImplementation); + this(probeImplementation); this.condition = condition; } @@ -722,6 +732,26 @@ public boolean getCondition() { return condition; } + public DebugSessionStatus getDebugSessionStatus() { + return debugSessionStatus; + } + + private DebugSessionStatus debugSessionStatus() { + if (probeImplementation instanceof ProbeDefinition) { + ProbeDefinition definition = (ProbeDefinition) probeImplementation; + Map sessions = getDebugSessions(); + String sessionId = definition.getDebugSessionId(); + if (sessionId == null) { + return DebugSessionStatus.NONE; + } + return "1".equals(sessions.get(sessionId)) || "1".equals(sessions.get("*")) + ? DebugSessionStatus.ACTIVE + : DebugSessionStatus.DISABLED; + } + + return DebugSessionStatus.NONE; + } + public void setCondition(boolean value) { this.condition = value; } @@ -765,6 +795,29 @@ public boolean isForceSampling() { public void setForceSampling(boolean forceSampling) { this.forceSampling = forceSampling; } + + @Override + public String toString() { + return "LogStatus{" + + ", probeId=" + + probeImplementation.getId() + + ", condition=" + + condition + + ", debugSessionStatus=" + + debugSessionStatus + + ", forceSampling=" + + forceSampling + + ", hasConditionErrors=" + + hasConditionErrors + + ", hasLogTemplateErrors=" + + hasLogTemplateErrors + + ", message='" + + message + + '\'' + + ", sampled=" + + sampled + + '}'; + } } @Generated diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java index 042ac86ff54..c4c99079669 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java @@ -12,6 +12,11 @@ import datadog.trace.bootstrap.debugger.ProbeId; import datadog.trace.bootstrap.debugger.ProbeImplementation; import datadog.trace.bootstrap.debugger.ProbeLocation; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI; +import datadog.trace.core.DDSpan; +import datadog.trace.core.DDSpanContext; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -26,7 +31,7 @@ public abstract class ProbeDefinition implements ProbeImplementation { protected static final String LANGUAGE = "java"; protected final String language; - protected final String id; + protected String id; protected final int version; protected transient ProbeId probeId; protected final Tag[] tags; @@ -54,6 +59,9 @@ protected ProbeDefinition( @Override public String getId() { + if (id == null) { + id = probeId != null ? probeId.getId() : null; + } return id; } @@ -93,6 +101,9 @@ public String concatTags() { } public Map getTagMap() { + if (tagMap.isEmpty() && tags != null) { + initTagMap(tagMap, tags); + } return Collections.unmodifiableMap(tagMap); } @@ -136,6 +147,10 @@ public ProbeLocation getLocation() { public void evaluate( CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {} + protected String getDebugSessionId() { + return getTagMap().get("sessionId"); + } + @Override public void commit( CapturedContext entryContext, @@ -166,6 +181,16 @@ public boolean isLineProbe() { return sourceLines != null && sourceLines.length > 0; } + public enum DebugSessionStatus { + NONE, + ACTIVE, + DISABLED; + + public boolean isDisabled() { + return this == DISABLED; + } + } + public abstract static class Builder { protected String language = LANGUAGE; protected ProbeId probeId; @@ -360,4 +385,28 @@ public void toJson(JsonWriter jsonWriter, Tag[] tags) throws IOException { jsonWriter.endArray(); } } + + public static Map getDebugSessions() { + HashMap sessions = new HashMap<>(); + TracerAPI tracer = AgentTracer.get(); + if (tracer != null) { + AgentSpan span = tracer.activeSpan(); + if (span instanceof DDSpan) { + DDSpanContext context = (DDSpanContext) span.context(); + String debug = context.getPropagationTags().getDebugPropagation(); + if (debug != null) { + String[] entries = debug.split(","); + for (String entry : entries) { + if (!entry.contains(":")) { + sessions.put("*", entry); + } else { + String[] values = entry.split(":"); + sessions.put(values[0], values[1]); + } + } + } + } + } + return sessions; + } } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/DebuggerSink.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/DebuggerSink.java index 98cb01d62f4..8af4cf36ead 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/DebuggerSink.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/DebuggerSink.java @@ -4,7 +4,7 @@ import com.datadog.debugger.uploader.BatchUploader; import com.datadog.debugger.util.DebuggerMetrics; import datadog.trace.api.Config; -import datadog.trace.bootstrap.debugger.DebuggerContext; +import datadog.trace.bootstrap.debugger.DebuggerContext.SkipCause; import datadog.trace.bootstrap.debugger.ProbeId; import datadog.trace.util.AgentTaskScheduler; import java.util.List; @@ -223,20 +223,8 @@ private void reportError(ProbeId probeId, DiagnosticMessage msg) { } /** Notifies the snapshot was skipped for one of the SkipCause reason */ - public void skipSnapshot(String probeId, DebuggerContext.SkipCause cause) { - String causeTag; - switch (cause) { - case RATE: - causeTag = "cause:rate"; - break; - case CONDITION: - causeTag = "cause:condition"; - break; - default: - throw new IllegalArgumentException("Unknown cause: " + cause); - } - String probeIdTag = "probe_id:" + probeId; - debuggerMetrics.incrementCounter(PREFIX + "skip", causeTag, probeIdTag); + public void skipSnapshot(String probeId, SkipCause cause) { + debuggerMetrics.incrementCounter(PREFIX + "skip", cause.tag(), "probe_id:" + probeId); } long getCurrentLowRateFlushInterval() { 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 92fb5c57b41..1a1b3fd77b5 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 @@ -352,14 +352,7 @@ public static LogProbe.Builder createProbeBuilder( protected TestSnapshotListener installProbes( Configuration configuration, ProbeDefinition... probes) { - config = mock(Config.class); - when(config.isDebuggerEnabled()).thenReturn(true); - when(config.isDebuggerClassFileDumpEnabled()).thenReturn(true); - when(config.isDebuggerVerifyByteCode()).thenReturn(false); - when(config.getFinalDebuggerSnapshotUrl()) - .thenReturn("http://localhost:8126/debugger/v1/input"); - when(config.getFinalDebuggerSymDBUrl()).thenReturn("http://localhost:8126/symdb/v1/input"); - when(config.getDebuggerCodeOriginMaxUserFrames()).thenReturn(20); + config = mockConfig(); instrumentationListener = new MockInstrumentationListener(); probeStatusSink = mock(ProbeStatusSink.class); @@ -404,6 +397,19 @@ protected TestSnapshotListener installProbes( return listener; } + public static Config mockConfig() { + Config config = mock(Config.class); + when(config.isDebuggerEnabled()).thenReturn(true); + when(config.isDebuggerClassFileDumpEnabled()).thenReturn(true); + when(config.isDebuggerVerifyByteCode()).thenReturn(false); + when(config.getFinalDebuggerSnapshotUrl()) + .thenReturn("http://localhost:8126/debugger/v1/input"); + when(config.getFinalDebuggerSymDBUrl()).thenReturn("http://localhost:8126/symdb/v1/input"); + when(config.getDebuggerCodeOriginMaxUserFrames()).thenReturn(20); + + return config; + } + public static ProbeImplementation resolver( String encodedId, Collection logProbes, 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 2d3181ea380..383968070fd 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 @@ -1,15 +1,35 @@ package com.datadog.debugger.probe; +import static com.datadog.debugger.agent.CapturingTestBase.mockConfig; import static com.datadog.debugger.util.LogProbeTestHelper.parseTemplate; +import static java.lang.String.format; +import static java.lang.Thread.currentThread; +import static java.util.Collections.emptyList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import com.datadog.debugger.agent.DebuggerAgentHelper; +import com.datadog.debugger.probe.LogProbe.Builder; +import com.datadog.debugger.probe.LogProbe.LogStatus; +import com.datadog.debugger.probe.ProbeDefinition.DebugSessionStatus; +import com.datadog.debugger.sink.DebuggerSink; +import com.datadog.debugger.sink.ProbeStatusSink; import com.datadog.debugger.sink.Snapshot; +import datadog.trace.api.IdGenerationStrategy; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.EvaluationError; import datadog.trace.bootstrap.debugger.MethodLocation; import datadog.trace.bootstrap.debugger.ProbeId; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI; +import datadog.trace.bootstrap.instrumentation.api.ScopeSource; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.core.CoreTracer; +import java.util.UUID; import java.util.stream.Stream; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -21,6 +41,7 @@ public class LogProbeTest { private static final String LANGUAGE = "java"; private static final ProbeId PROBE_ID = new ProbeId("beae1807-f3b0-4ea8-a74f-826790c5e6f8", 0); + private static final String DEBUG_SESSION_ID = "TestSession"; @Test public void testCapture() { @@ -38,6 +59,69 @@ public void testSampling() { Assertions.assertEquals(0.25, snapshotProbe.getSampling().getEventsPerSecond(), 0.01); } + @Test + public void debugSessionActive() { + assertTrue( + fillSnapshot(DebugSessionStatus.ACTIVE), + "Session is active so snapshots should get filled."); + } + + @Test + public void debugSessionDisabled() { + Assertions.assertFalse( + fillSnapshot(DebugSessionStatus.DISABLED), + "Session is disabled so snapshots should not get filled."); + } + + @Test + public void noDebugSession() { + assertTrue( + fillSnapshot(DebugSessionStatus.NONE), + "With no debug sessions, snapshots should get filled."); + } + + private boolean fillSnapshot(DebugSessionStatus status) { + DebuggerAgentHelper.injectSink(new DebuggerSink(mockConfig(), mock(ProbeStatusSink.class))); + TracerAPI tracer = + CoreTracer.builder().idGenerationStrategy(IdGenerationStrategy.fromName("random")).build(); + AgentTracer.registerIfAbsent(tracer); + AgentSpan span = tracer.startSpan("log probe debug session testing", "test span"); + try (AgentScope scope = tracer.activateSpan(span, ScopeSource.MANUAL)) { + if (status == DebugSessionStatus.ACTIVE) { + span.setTag(Tags.PROPAGATED_DEBUG, DEBUG_SESSION_ID + ":1"); + } else if (status == DebugSessionStatus.DISABLED) { + span.setTag(Tags.PROPAGATED_DEBUG, DEBUG_SESSION_ID + ":0"); + } + + Builder builder = + createLog("I'm in a debug session").probeId(UUID.randomUUID().toString(), 0); + if (status != DebugSessionStatus.NONE) { + builder.tags(format("sessionId:%s", DEBUG_SESSION_ID)); + } + + LogProbe logProbe = builder.build(); + + CapturedContext entryContext = capturedContext(span, logProbe); + CapturedContext exitContext = capturedContext(span, logProbe); + logProbe.evaluate(entryContext, new LogStatus(logProbe), MethodLocation.ENTRY); + logProbe.evaluate(exitContext, new LogStatus(logProbe), MethodLocation.EXIT); + + return logProbe.fillSnapshot( + entryContext, exitContext, emptyList(), new Snapshot(currentThread(), logProbe, 3)); + } + } + + private static CapturedContext capturedContext(AgentSpan span, ProbeDefinition probeDefinition) { + CapturedContext context = new CapturedContext(); + context.evaluate( + probeDefinition.getProbeId().getEncodedId(), + probeDefinition, + "Log Probe test", + System.currentTimeMillis(), + MethodLocation.DEFAULT); + return context; + } + @Test public void log() { LogProbe logProbe = createLog(null).build(); From 88fdeb912b63b777b486421ef5938528f88bd8b5 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Thu, 16 Jan 2025 10:16:32 -0500 Subject: [PATCH 2/4] moving some bits around --- .../debugger/probe/DebugSessionStatus.java | 11 +++++ .../com/datadog/debugger/probe/LogProbe.java | 38 +++++++++++++++- .../debugger/probe/ProbeDefinition.java | 43 ------------------- .../datadog/debugger/probe/LogProbeTest.java | 1 - 4 files changed, 47 insertions(+), 46 deletions(-) create mode 100644 dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/DebugSessionStatus.java diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/DebugSessionStatus.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/DebugSessionStatus.java new file mode 100644 index 00000000000..3570f8de6e8 --- /dev/null +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/DebugSessionStatus.java @@ -0,0 +1,11 @@ +package com.datadog.debugger.probe; + +public enum DebugSessionStatus { + NONE, + ACTIVE, + DISABLED; + + public boolean isDisabled() { + return this == DISABLED; + } +} 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 e831f815e6a..41c5694d85b 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 @@ -32,6 +32,11 @@ import datadog.trace.bootstrap.debugger.ProbeImplementation; import datadog.trace.bootstrap.debugger.ProbeRateLimiter; import datadog.trace.bootstrap.debugger.util.TimeoutChecker; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI; +import datadog.trace.core.DDSpan; +import datadog.trace.core.DDSpanContext; import java.io.IOException; import java.lang.reflect.ParameterizedType; import java.time.Duration; @@ -39,6 +44,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -52,6 +58,30 @@ public class LogProbe extends ProbeDefinition implements Sampled { private static final Limits LIMITS = new Limits(1, 3, 8192, 5); private static final int LOG_MSG_LIMIT = 8192; + private static Map getDebugSessions() { + HashMap sessions = new HashMap<>(); + TracerAPI tracer = AgentTracer.get(); + if (tracer != null) { + AgentSpan span = tracer.activeSpan(); + if (span instanceof DDSpan) { + DDSpanContext context = (DDSpanContext) span.context(); + String debug = context.getPropagationTags().getDebugPropagation(); + if (debug != null) { + String[] entries = debug.split(","); + for (String entry : entries) { + if (!entry.contains(":")) { + sessions.put("*", entry); + } else { + String[] values = entry.split(":"); + sessions.put(values[0], values[1]); + } + } + } + } + } + return sessions; + } + /** Stores part of a templated message either a str or an expression */ public static class Segment { private final String str; @@ -681,6 +711,10 @@ public boolean hasCondition() { return probeCondition != null; } + protected String getDebugSessionId() { + return getTagMap().get("sessionId"); + } + @Override public CapturedContext.Status createStatus() { return new LogStatus(this); @@ -737,8 +771,8 @@ public DebugSessionStatus getDebugSessionStatus() { } private DebugSessionStatus debugSessionStatus() { - if (probeImplementation instanceof ProbeDefinition) { - ProbeDefinition definition = (ProbeDefinition) probeImplementation; + if (probeImplementation instanceof LogProbe) { + LogProbe definition = (LogProbe) probeImplementation; Map sessions = getDebugSessions(); String sessionId = definition.getDebugSessionId(); if (sessionId == null) { diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java index c4c99079669..6cbfe16e214 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java @@ -12,11 +12,6 @@ import datadog.trace.bootstrap.debugger.ProbeId; import datadog.trace.bootstrap.debugger.ProbeImplementation; import datadog.trace.bootstrap.debugger.ProbeLocation; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI; -import datadog.trace.core.DDSpan; -import datadog.trace.core.DDSpanContext; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -147,10 +142,6 @@ public ProbeLocation getLocation() { public void evaluate( CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {} - protected String getDebugSessionId() { - return getTagMap().get("sessionId"); - } - @Override public void commit( CapturedContext entryContext, @@ -181,16 +172,6 @@ public boolean isLineProbe() { return sourceLines != null && sourceLines.length > 0; } - public enum DebugSessionStatus { - NONE, - ACTIVE, - DISABLED; - - public boolean isDisabled() { - return this == DISABLED; - } - } - public abstract static class Builder { protected String language = LANGUAGE; protected ProbeId probeId; @@ -385,28 +366,4 @@ public void toJson(JsonWriter jsonWriter, Tag[] tags) throws IOException { jsonWriter.endArray(); } } - - public static Map getDebugSessions() { - HashMap sessions = new HashMap<>(); - TracerAPI tracer = AgentTracer.get(); - if (tracer != null) { - AgentSpan span = tracer.activeSpan(); - if (span instanceof DDSpan) { - DDSpanContext context = (DDSpanContext) span.context(); - String debug = context.getPropagationTags().getDebugPropagation(); - if (debug != null) { - String[] entries = debug.split(","); - for (String entry : entries) { - if (!entry.contains(":")) { - sessions.put("*", entry); - } else { - String[] values = entry.split(":"); - sessions.put(values[0], values[1]); - } - } - } - } - } - return sessions; - } } 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 383968070fd..eb08f000ec5 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 @@ -13,7 +13,6 @@ import com.datadog.debugger.agent.DebuggerAgentHelper; import com.datadog.debugger.probe.LogProbe.Builder; import com.datadog.debugger.probe.LogProbe.LogStatus; -import com.datadog.debugger.probe.ProbeDefinition.DebugSessionStatus; import com.datadog.debugger.sink.DebuggerSink; import com.datadog.debugger.sink.ProbeStatusSink; import com.datadog.debugger.sink.Snapshot; From 30b62bc511b210df1406c23d092ef49ca6564993 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Thu, 16 Jan 2025 14:55:44 -0500 Subject: [PATCH 3/4] fix the session id tag PR clean up --- .../com/datadog/debugger/probe/LogProbe.java | 2 +- .../debugger/probe/ProbeDefinition.java | 5 +---- .../datadog/debugger/probe/TriggerProbe.java | 20 +++++++++++++++---- .../datadog/debugger/probe/LogProbeTest.java | 2 +- 4 files changed, 19 insertions(+), 10 deletions(-) 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 41c5694d85b..30232a9b051 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 @@ -712,7 +712,7 @@ public boolean hasCondition() { } protected String getDebugSessionId() { - return getTagMap().get("sessionId"); + return getTagMap().get("session_id"); } @Override diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java index 6cbfe16e214..c4f9f0ae045 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java @@ -26,7 +26,7 @@ public abstract class ProbeDefinition implements ProbeImplementation { protected static final String LANGUAGE = "java"; protected final String language; - protected String id; + protected final String id; protected final int version; protected transient ProbeId probeId; protected final Tag[] tags; @@ -54,9 +54,6 @@ protected ProbeDefinition( @Override public String getId() { - if (id == null) { - id = probeId != null ? probeId.getId() : null; - } return id; } 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 9b1d077b993..bcdc2d13811 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 @@ -110,7 +110,7 @@ private void decorateTags() { AgentTracer.TracerAPI tracerAPI = AgentTracer.get(); AgentSpan agentSpan = tracerAPI.activeSpan().getLocalRootSpan(); - agentSpan.setTag(Tags.PROPAGATED_DEBUG, "1"); + agentSpan.setTag(Tags.PROPAGATED_DEBUG, sessionId + ":1"); agentSpan.setTag(format("_dd.ld.probe_id.%s", probeId.getId()), true); } @@ -148,8 +148,20 @@ public int hashCode() { @Override public String toString() { - return format( - "TriggerProbe{id='%s', where=%s, sampling=%s, probeCondition=%s}", - id, where, sampling, probeCondition); + return String.format( + "TriggerProbe{id='%s', sessionId='%s', evaluateAt=%s, language='%s', location=%s, probeCondition=%s, probeId=%s," + + " sampling=%s, tagMap=%s, tags=%s, version=%d, where=%s}", + id, + sessionId, + evaluateAt, + language, + location, + probeCondition, + probeId, + sampling, + tagMap, + Arrays.toString(tags), + version, + where); } } 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 eb08f000ec5..77847c79aed 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 @@ -95,7 +95,7 @@ private boolean fillSnapshot(DebugSessionStatus status) { Builder builder = createLog("I'm in a debug session").probeId(UUID.randomUUID().toString(), 0); if (status != DebugSessionStatus.NONE) { - builder.tags(format("sessionId:%s", DEBUG_SESSION_ID)); + builder.tags(format("session_id:%s", DEBUG_SESSION_ID)); } LogProbe logProbe = builder.build(); From db4aefa14b5dd185c24c054a7c95504b345ba419 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Fri, 17 Jan 2025 09:34:34 -0500 Subject: [PATCH 4/4] fix the session id tag PR clean up --- .../com/datadog/debugger/probe/LogProbe.java | 9 ++++-- .../datadog/debugger/probe/TriggerProbe.java | 9 ++++++ .../debugger/trigger/TriggerProbeTest.java | 31 +++++++++++++++---- 3 files changed, 40 insertions(+), 9 deletions(-) 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 30232a9b051..051fc9d7bff 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 @@ -465,8 +465,7 @@ public void evaluate( // sample when no condition associated sample(logStatus, methodLocation); } - logStatus.setCondition( - !logStatus.getDebugSessionStatus().isDisabled() && evaluateCondition(context, logStatus)); + logStatus.setCondition(evaluateCondition(context, logStatus)); CapturedContext.CapturedThrowable throwable = context.getCapturedThrowable(); if (logStatus.hasConditionErrors() && throwable != null) { logStatus.addError( @@ -755,7 +754,11 @@ public boolean isCapturing() { } public boolean shouldSend() { - return sampled && condition && !hasConditionErrors; + DebugSessionStatus status = getDebugSessionStatus(); + // an ACTIVE status overrides the sampling as the sampling decision was made by the trigger + // probe + return status == DebugSessionStatus.ACTIVE + || !status.isDisabled() && sampled && condition && !hasConditionErrors; } public boolean shouldReportError() { 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 bcdc2d13811..e29e691459e 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,6 +59,15 @@ public InstrumentationResult.Status instrument( .instrument(); } + public String getSessionId() { + return sessionId; + } + + public TriggerProbe setSessionId(String sessionId) { + this.sessionId = sessionId; + return this; + } + public Sampling getSampling() { return sampling; } 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 c3e529ada52..93d18843464 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 @@ -35,6 +35,7 @@ public class TriggerProbeTest extends CapturingTestBase { private static final ProbeId TRIGGER_PROBE_ID1 = new ProbeId("trigger probe 1", 0); + private static final String TRIGGER_PROBE_SESSION_ID = "trigger probe sessionID"; private TestTraceInterceptor traceInterceptor; @@ -61,7 +62,13 @@ public void cooldown() throws IOException, URISyntaxException { final String className = "com.datadog.debugger.TriggerProbe01"; TriggerProbe probe1 = createTriggerProbe( - TRIGGER_PROBE_ID1, className, "entry", "()", null, new Sampling(10, 10.0)); + TRIGGER_PROBE_ID1, + TRIGGER_PROBE_SESSION_ID, + className, + "entry", + "()", + null, + new Sampling(10, 10.0)); installProbes( Configuration.builder() .setService(SERVICE_NAME) @@ -75,6 +82,8 @@ public void cooldown() throws IOException, URISyntaxException { assertEquals(1, sampler.getCallCount()); List> allTraces = traceInterceptor.getAllTraces(); + assertEquals(runs, allTraces.size(), "actual traces: " + allTraces.size()); + long debugSessions = allTraces.stream() .map(span -> span.get(0)) @@ -82,9 +91,11 @@ public void cooldown() throws IOException, URISyntaxException { span -> { DDSpan ddSpan = (DDSpan) span; PropagationTags tags = ddSpan.context().getPropagationTags(); - return "1".equals(tags.getDebugPropagation()); + return (TRIGGER_PROBE_SESSION_ID + ":1").equals(tags.getDebugPropagation()); }) .count(); + assertEquals(1, debugSessions, "Should only have 1 debug session. found: " + debugSessions); + long tagged = allTraces.stream() .flatMap(Collection::stream) @@ -92,8 +103,6 @@ public void cooldown() throws IOException, URISyntaxException { span -> span.getTag(format("_dd.ld.probe_id.%s", TRIGGER_PROBE_ID1.getId())) != null) .count(); - assertEquals(runs, allTraces.size(), "actual traces: " + allTraces.size()); - assertEquals(1, debugSessions, "Should only have 1 debug session. found: " + debugSessions); assertEquals(1, tagged, "Should only have 1 tagged span. found: " + tagged); } finally { ProbeRateLimiter.setSamplerSupplier(null); @@ -108,7 +117,14 @@ public void sampling() throws IOException, URISyntaxException { final String className = "com.datadog.debugger.TriggerProbe01"; TriggerProbe probe1 = - createTriggerProbe(TRIGGER_PROBE_ID1, className, "entry", "()", null, new Sampling(10.0)); + createTriggerProbe( + TRIGGER_PROBE_ID1, + TRIGGER_PROBE_SESSION_ID, + className, + "entry", + "()", + null, + new Sampling(10.0)); Configuration config = Configuration.builder() .setService(SERVICE_NAME) @@ -133,6 +149,7 @@ public void conditions() throws IOException, URISyntaxException { TriggerProbe probe1 = createTriggerProbe( TRIGGER_PROBE_ID1, + TRIGGER_PROBE_SESSION_ID, className, "entry", "(int)", @@ -155,7 +172,7 @@ public void conditions() throws IOException, URISyntaxException { span -> { DDSpan ddSpan = (DDSpan) span; PropagationTags tags = ddSpan.context().getPropagationTags(); - return "1".equals(tags.getDebugPropagation()); + return (TRIGGER_PROBE_SESSION_ID + ":1").equals(tags.getDebugPropagation()); }) .count(); assertEquals(100, allTraces.size(), "actual traces: " + allTraces.size()); @@ -164,6 +181,7 @@ public void conditions() throws IOException, URISyntaxException { public static TriggerProbe createTriggerProbe( ProbeId id, + String sessionId, String typeName, String methodName, String signature, @@ -171,6 +189,7 @@ public static TriggerProbe createTriggerProbe( Sampling sampling, String... lines) { return new TriggerProbe(id, Where.of(typeName, methodName, signature, lines)) + .setSessionId(sessionId) .setProbeCondition(probeCondition) .setSampling(sampling); }