From f701ba9d759538d54ad534235d9c20ec2cf21565 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Fri, 24 Jan 2025 19:35:52 -0500 Subject: [PATCH 1/4] Use two budgets depending on type (#8283) --- .../bootstrap/debugger/DebuggerContext.java | 6 ++ .../com/datadog/debugger/probe/LogProbe.java | 26 +++++--- .../datadog/debugger/sink/SnapshotSink.java | 4 -- .../datadog/debugger/probe/LogProbeTest.java | 61 ++++++++++++++++--- 4 files changed, 78 insertions(+), 19 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 9d1c793ea8e..5acd251b78a 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 @@ -39,6 +39,12 @@ public String tag() { public String tag() { return "cause:debug session disabled"; } + }, + BUDGET { + @Override + public String tag() { + return "cause:budget_exceeded"; + } }; public abstract String tag(); 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 f07f17cc558..95ba3426135 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 @@ -1,6 +1,7 @@ package com.datadog.debugger.probe; import static com.datadog.debugger.probe.LogProbe.Capture.toLimits; +import static java.lang.String.format; import com.datadog.debugger.agent.DebuggerAgent; import com.datadog.debugger.agent.Generated; @@ -62,7 +63,8 @@ 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; - public static final int PROBE_BUDGET = 10; + public static final int CAPTURING_PROBE_BUDGET = 10; + public static final int NON_CAPTURING_PROBE_BUDGET = 1000; /** Stores part of a templated message either a str or an expression */ public static class Segment { @@ -575,14 +577,17 @@ public void commit( CapturedContext exitContext, List caughtExceptions) { Snapshot snapshot = createSnapshot(); - boolean shouldCommit = - inBudget() && fillSnapshot(entryContext, exitContext, caughtExceptions, snapshot); + boolean shouldCommit = fillSnapshot(entryContext, exitContext, caughtExceptions, snapshot); DebuggerSink sink = DebuggerAgent.getSink(); if (shouldCommit) { - commitSnapshot(snapshot, sink); incrementBudget(); - if (snapshotProcessor != null) { - snapshotProcessor.accept(snapshot); + if (inBudget()) { + commitSnapshot(snapshot, sink); + if (snapshotProcessor != null) { + snapshotProcessor.accept(snapshot); + } + } else { + sink.skipSnapshot(id, DebuggerContext.SkipCause.BUDGET); } } else { sink.skipSnapshot(id, DebuggerContext.SkipCause.CONDITION); @@ -866,7 +871,9 @@ public String toString() { private boolean inBudget() { AtomicInteger budgetLevel = getBudgetLevel(); - return budgetLevel == null || budgetLevel.get() < PROBE_BUDGET; + return budgetLevel == null + || budgetLevel.get() + <= (captureSnapshot ? CAPTURING_PROBE_BUDGET : NON_CAPTURING_PROBE_BUDGET); } private AtomicInteger getBudgetLevel() { @@ -881,6 +888,11 @@ private void incrementBudget() { AtomicInteger budgetLevel = getBudgetLevel(); if (budgetLevel != null) { budgetLevel.incrementAndGet(); + TracerAPI tracer = AgentTracer.get(); + AgentSpan span = tracer != null ? tracer.activeSpan() : null; + if (span != null) { + span.getLocalRootSpan().setTag(format("_dd.ld.probe_id.%s", id), budgetLevel.get()); + } } } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/SnapshotSink.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/SnapshotSink.java index 168ef43a19e..8b47882c8fa 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/SnapshotSink.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/SnapshotSink.java @@ -53,10 +53,6 @@ public SnapshotSink(Config config, String tags, BatchUploader snapshotUploader) this.snapshotUploader = snapshotUploader; } - public BlockingQueue getLowRateSnapshots() { - return lowRateSnapshots; - } - public void start() { if (started.compareAndSet(false, true)) { highRateScheduled = 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 19019ffa3e6..5c048a47fd8 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 @@ -16,6 +16,7 @@ import com.datadog.debugger.sink.DebuggerSink; import com.datadog.debugger.sink.ProbeStatusSink; import com.datadog.debugger.sink.Snapshot; +import datadog.trace.api.Config; import datadog.trace.api.IdGenerationStrategy; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.EvaluationError; @@ -80,20 +81,28 @@ public void noDebugSession() { @Test public void budgets() { - DebuggerSink sink = new DebuggerSink(getConfig(), mock(ProbeStatusSink.class)); + BudgetSink sink = new BudgetSink(getConfig(), mock(ProbeStatusSink.class)); DebuggerAgentHelper.injectSink(sink); - assertEquals(0, sink.getSnapshotSink().getLowRateSnapshots().size()); + TracerAPI tracer = CoreTracer.builder().idGenerationStrategy(IdGenerationStrategy.fromName("random")).build(); AgentTracer.registerIfAbsent(tracer); int runs = 100; for (int i = 0; i < runs; i++) { - runTrace(tracer); + runTrace(tracer, true); + } + assertEquals(runs * LogProbe.CAPTURING_PROBE_BUDGET, sink.captures); + + sink = new BudgetSink(getConfig(), mock(ProbeStatusSink.class)); + DebuggerAgentHelper.injectSink(sink); + runs = 1010; + for (int i = 0; i < runs; i++) { + runTrace(tracer, false); } - assertEquals(runs * LogProbe.PROBE_BUDGET, sink.getSnapshotSink().getLowRateSnapshots().size()); + assertEquals(runs * LogProbe.NON_CAPTURING_PROBE_BUDGET, sink.highRate); } - private void runTrace(TracerAPI tracer) { + private void runTrace(TracerAPI tracer, boolean captureSnapshot) { AgentSpan span = tracer.startSpan("budget testing", "test span"); span.setTag(Tags.PROPAGATED_DEBUG, "12345:1"); try (AgentScope scope = tracer.activateSpan(span, ScopeSource.MANUAL)) { @@ -102,17 +111,23 @@ private void runTrace(TracerAPI tracer) { createLog("I'm in a debug session") .probeId(ProbeId.newId()) .tags("session_id:12345") - .captureSnapshot(true) + .captureSnapshot(captureSnapshot) .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); - for (int i = 0; i < 20; i++) { + int budget = + logProbe.isCaptureSnapshot() + ? LogProbe.CAPTURING_PROBE_BUDGET + : LogProbe.NON_CAPTURING_PROBE_BUDGET; + int runs = budget + 20; + + for (int i = 0; i < runs; i++) { logProbe.commit(entryContext, exitContext, emptyList()); } + assertEquals(runs, span.getLocalRootSpan().getTag(format("_dd.ld.probe_id.%s", logProbe.id))); } } @@ -293,4 +308,34 @@ private Builder createLog(String template) { .probeId(PROBE_ID) .template(template, parseTemplate(template)); } + + private static class BudgetSink extends DebuggerSink { + + public int captures; + public int highRate; + + public BudgetSink(Config config, ProbeStatusSink probeStatusSink) { + super(config, probeStatusSink); + } + + @Override + public void addHighRateSnapshot(Snapshot snapshot) { + highRate++; + } + + @Override + public void addSnapshot(Snapshot snapshot) { + captures++; + } + + @Override + public void start() { + super.start(); + } + + @Override + public void stop() { + super.stop(); + } + } } From 19da739a7c4a0d0c9c7e7f0550531aef9e5f45fd Mon Sep 17 00:00:00 2001 From: "Santiago M. Mola" Date: Mon, 27 Jan 2025 10:15:16 +0100 Subject: [PATCH 2/4] Remove redundant setJavaVersion calls in gradle (#8278) --- dd-java-agent/instrumentation/java-lang/build.gradle | 5 ----- dd-java-agent/instrumentation/java-net/build.gradle | 5 ----- dd-java-agent/instrumentation/javax-naming/build.gradle | 6 ------ 3 files changed, 16 deletions(-) diff --git a/dd-java-agent/instrumentation/java-lang/build.gradle b/dd-java-agent/instrumentation/java-lang/build.gradle index 8258d36ba37..74bdd7ed385 100644 --- a/dd-java-agent/instrumentation/java-lang/build.gradle +++ b/dd-java-agent/instrumentation/java-lang/build.gradle @@ -12,8 +12,3 @@ addTestSuiteForDir('latestDepTest', 'test') dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') } - -tasks.compileTestJava.configure { - setJavaVersion(it, 8) -} - diff --git a/dd-java-agent/instrumentation/java-net/build.gradle b/dd-java-agent/instrumentation/java-net/build.gradle index 8258d36ba37..74bdd7ed385 100644 --- a/dd-java-agent/instrumentation/java-net/build.gradle +++ b/dd-java-agent/instrumentation/java-net/build.gradle @@ -12,8 +12,3 @@ addTestSuiteForDir('latestDepTest', 'test') dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') } - -tasks.compileTestJava.configure { - setJavaVersion(it, 8) -} - diff --git a/dd-java-agent/instrumentation/javax-naming/build.gradle b/dd-java-agent/instrumentation/javax-naming/build.gradle index 167588643d4..74bdd7ed385 100644 --- a/dd-java-agent/instrumentation/javax-naming/build.gradle +++ b/dd-java-agent/instrumentation/javax-naming/build.gradle @@ -12,9 +12,3 @@ addTestSuiteForDir('latestDepTest', 'test') dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') } - - -tasks.compileTestJava.configure { - setJavaVersion(it, 8) -} - From a85ec5f3c17f1c6f0d112e59d06214c25acf0052 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko <121111529+nikita-tkachenko-datadog@users.noreply.github.com> Date: Mon, 27 Jan 2025 14:38:11 +0100 Subject: [PATCH 3/4] Remove skip and shouldBeSkipped methods from TestEventsHandler in favor of isSkippable (#8286) --- .../domain/TestFrameworkModule.java | 13 ++----------- .../trace/civisibility/domain/TestImpl.java | 8 ++++++++ .../civisibility/domain/TestSuiteImpl.java | 5 +++++ .../domain/buildsystem/ProxyTestModule.java | 15 +++++++-------- .../domain/headless/HeadlessTestModule.java | 15 +++++++-------- .../domain/manualapi/ManualApiTestModule.java | 3 +++ .../events/NoOpTestEventsHandler.java | 7 +------ .../events/TestEventsHandlerImpl.java | 17 ++++++----------- .../civisibility/test/ExecutionResults.java | 16 ++++++++++++++++ .../civisibility/test/ExecutionStrategy.java | 17 +---------------- .../civisibility/domain/TestImplTest.groovy | 3 +++ .../domain/TestSuiteImplTest.groovy | 3 +++ .../JUnit4CucumberItrInstrumentation.java | 2 +- .../junit4/JUnit4ItrInstrumentation.java | 2 +- .../JUnit5CucumberItrInstrumentation.java | 2 +- .../junit5/JUnit5SpockItrInstrumentation.java | 9 +++------ ...5SpockParameterizedRetryInstrumentation.java | 1 + .../junit5/JUnit5ItrInstrumentation.java | 2 +- .../karate/KarateTracingHook.java | 2 +- .../instrumentation/scalatest/RunContext.java | 4 ++-- .../testng/TestNGItrInstrumentation.java | 2 +- .../civisibility/events/TestEventsHandler.java | 6 ++---- 22 files changed, 76 insertions(+), 78 deletions(-) create mode 100644 dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionResults.java diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java index 62e1e48aec9..137ca240f19 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java @@ -31,21 +31,12 @@ TestSuiteImpl testSuiteStart( boolean isModified(TestSourceData testSourceData); /** - * Checks if a given test should be skipped with Intelligent Test Runner or not + * Checks if a given test can be skipped with Intelligent Test Runner or not. * * @param test Test to be checked * @return {@code true} if the test can be skipped, {@code false} otherwise */ - boolean shouldBeSkipped(TestIdentifier test); - - /** - * Checks if a given test can be skipped with Intelligent Test Runner or not. If the test is - * considered skippable, the count of skippable tests is incremented. - * - * @param test Test to be checked - * @return {@code true} if the test can be skipped, {@code false} otherwise - */ - boolean skip(TestIdentifier test); + boolean isSkippable(TestIdentifier test); @Nonnull TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java index 46248ff19b1..115b140b5a8 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java @@ -37,6 +37,7 @@ import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; import datadog.trace.civisibility.source.SourceResolutionException; +import datadog.trace.civisibility.test.ExecutionResults; import java.lang.reflect.Method; import java.util.Collection; import java.util.Collections; @@ -50,6 +51,7 @@ public class TestImpl implements DDTest { private static final Logger log = LoggerFactory.getLogger(TestImpl.class); private final CiVisibilityMetricCollector metricCollector; + private final ExecutionResults executionResults; private final TestFrameworkInstrumentation instrumentation; private final AgentSpan span; private final DDTraceId sessionId; @@ -78,11 +80,13 @@ public TestImpl( LinesResolver linesResolver, Codeowners codeowners, CoverageStore.Factory coverageStoreFactory, + ExecutionResults executionResults, Consumer onSpanFinish) { this.instrumentation = instrumentation; this.metricCollector = metricCollector; this.sessionId = moduleSpanContext.getTraceId(); this.suiteId = suiteId; + this.executionResults = executionResults; this.onSpanFinish = onSpanFinish; this.identifier = new TestIdentifier(testSuiteName, testName, testParameters); @@ -258,6 +262,10 @@ public void end(@Nullable Long endTime) { span.finish(); } + if (InstrumentationBridge.ITR_SKIP_REASON.equals(span.getTag(Tags.TEST_SKIP_REASON))) { + executionResults.incrementTestsSkippedByItr(); + } + metricCollector.add( CiVisibilityCountMetric.EVENT_FINISHED, 1, diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java index 3318f799e1a..024f77f0b4d 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java @@ -22,6 +22,7 @@ import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; import datadog.trace.civisibility.source.SourceResolutionException; +import datadog.trace.civisibility.test.ExecutionResults; import datadog.trace.civisibility.utils.SpanUtils; import java.lang.reflect.Method; import java.util.Collection; @@ -49,6 +50,7 @@ public class TestSuiteImpl implements DDTestSuite { private final Codeowners codeowners; private final LinesResolver linesResolver; private final CoverageStore.Factory coverageStoreFactory; + private final ExecutionResults executionResults; private final boolean parallelized; private final Consumer onSpanFinish; @@ -69,6 +71,7 @@ public TestSuiteImpl( Codeowners codeowners, LinesResolver linesResolver, CoverageStore.Factory coverageStoreFactory, + ExecutionResults executionResults, Consumer onSpanFinish) { this.moduleSpanContext = moduleSpanContext; this.moduleName = moduleName; @@ -84,6 +87,7 @@ public TestSuiteImpl( this.codeowners = codeowners; this.linesResolver = linesResolver; this.coverageStoreFactory = coverageStoreFactory; + this.executionResults = executionResults; this.onSpanFinish = onSpanFinish; AgentTracer.SpanBuilder spanBuilder = @@ -254,6 +258,7 @@ public TestImpl testStart( linesResolver, codeowners, coverageStoreFactory, + executionResults, SpanUtils.propagateCiVisibilityTagsTo(span)); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java index 3e903a07c51..d086c27244d 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java @@ -25,6 +25,7 @@ import datadog.trace.civisibility.ipc.TestFramework; import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; +import datadog.trace.civisibility.test.ExecutionResults; import datadog.trace.civisibility.test.ExecutionStrategy; import java.util.Collection; import java.util.TreeSet; @@ -46,6 +47,7 @@ public class ProxyTestModule implements TestFrameworkModule { private final AgentSpanContext parentProcessModuleContext; private final String moduleName; private final ExecutionStrategy executionStrategy; + private final ExecutionResults executionResults; private final SignalClient.Factory signalClientFactory; private final ChildProcessCoverageReporter childProcessCoverageReporter; private final Config config; @@ -73,6 +75,7 @@ public ProxyTestModule( this.parentProcessModuleContext = parentProcessModuleContext; this.moduleName = moduleName; this.executionStrategy = executionStrategy; + this.executionResults = new ExecutionResults(); this.signalClientFactory = signalClientFactory; this.childProcessCoverageReporter = childProcessCoverageReporter; this.config = config; @@ -100,13 +103,8 @@ public boolean isModified(TestSourceData testSourceData) { } @Override - public boolean shouldBeSkipped(TestIdentifier test) { - return executionStrategy.shouldBeSkipped(test); - } - - @Override - public boolean skip(TestIdentifier test) { - return executionStrategy.skip(test); + public boolean isSkippable(TestIdentifier test) { + return executionStrategy.isSkippable(test); } @Override @@ -143,7 +141,7 @@ private void sendModuleExecutionResult() { boolean earlyFlakeDetectionEnabled = earlyFlakeDetectionSettings.isEnabled(); boolean earlyFlakeDetectionFaulty = earlyFlakeDetectionEnabled && executionStrategy.isEFDLimitReached(); - long testsSkippedTotal = executionStrategy.getTestsSkipped(); + long testsSkippedTotal = executionResults.getTestsSkippedByItr(); signalClient.send( new ModuleExecutionResult( @@ -185,6 +183,7 @@ public TestSuiteImpl testSuiteStart( codeowners, linesResolver, coverageStoreFactory, + executionResults, this::propagateTestFrameworkData); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java index 00833d65b7e..7d5df44490b 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java @@ -22,6 +22,7 @@ import datadog.trace.civisibility.domain.TestSuiteImpl; import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; +import datadog.trace.civisibility.test.ExecutionResults; import datadog.trace.civisibility.test.ExecutionStrategy; import datadog.trace.civisibility.utils.SpanUtils; import java.util.function.Consumer; @@ -39,6 +40,7 @@ public class HeadlessTestModule extends AbstractTestModule implements TestFramew private final CoverageStore.Factory coverageStoreFactory; private final ExecutionStrategy executionStrategy; + private final ExecutionResults executionResults; public HeadlessTestModule( AgentSpanContext sessionSpanContext, @@ -67,6 +69,7 @@ public HeadlessTestModule( onSpanFinish); this.coverageStoreFactory = coverageStoreFactory; this.executionStrategy = executionStrategy; + this.executionResults = new ExecutionResults(); } @Override @@ -85,13 +88,8 @@ public boolean isModified(TestSourceData testSourceData) { } @Override - public boolean shouldBeSkipped(TestIdentifier test) { - return executionStrategy.shouldBeSkipped(test); - } - - @Override - public boolean skip(TestIdentifier test) { - return executionStrategy.skip(test); + public boolean isSkippable(TestIdentifier test) { + return executionStrategy.isSkippable(test); } @Override @@ -111,7 +109,7 @@ public void end(@Nullable Long endTime) { setTag(Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, true); setTag(Tags.TEST_ITR_TESTS_SKIPPING_TYPE, "test"); - long testsSkippedTotal = executionStrategy.getTestsSkipped(); + long testsSkippedTotal = executionResults.getTestsSkippedByItr(); setTag(Tags.TEST_ITR_TESTS_SKIPPING_COUNT, testsSkippedTotal); if (testsSkippedTotal > 0) { setTag(DDTags.CI_ITR_TESTS_SKIPPED, true); @@ -154,6 +152,7 @@ public TestSuiteImpl testSuiteStart( codeowners, linesResolver, coverageStoreFactory, + executionResults, SpanUtils.propagateCiVisibilityTagsTo(span)); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java index 5c5df7d18c1..4d9d3f54e68 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java @@ -14,6 +14,7 @@ import datadog.trace.civisibility.domain.TestSuiteImpl; import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; +import datadog.trace.civisibility.test.ExecutionResults; import datadog.trace.civisibility.utils.SpanUtils; import java.util.function.Consumer; import javax.annotation.Nullable; @@ -25,6 +26,7 @@ public class ManualApiTestModule extends AbstractTestModule implements DDTestModule { private final CoverageStore.Factory coverageStoreFactory; + private final ExecutionResults executionResults = new ExecutionResults(); public ManualApiTestModule( AgentSpanContext sessionSpanContext, @@ -76,6 +78,7 @@ public TestSuiteImpl testSuiteStart( codeowners, linesResolver, coverageStoreFactory, + executionResults, SpanUtils.propagateCiVisibilityTagsTo(span)); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java index 30903cf1170..f5187516dad 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java @@ -91,12 +91,7 @@ public void onTestIgnore( } @Override - public boolean skip(TestIdentifier test) { - return false; - } - - @Override - public boolean shouldBeSkipped(TestIdentifier test) { + public boolean isSkippable(TestIdentifier test) { return false; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java index 10c6f9b9274..787bc882ae4 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java @@ -187,7 +187,7 @@ public void onTestStart( test.setTag(Tags.TEST_ITR_UNSKIPPABLE, true); metricCollector.add(CiVisibilityCountMetric.ITR_UNSKIPPABLE, 1, EventType.TEST); - if (testModule.shouldBeSkipped(thisTest)) { + if (testModule.isSkippable(thisTest)) { test.setTag(Tags.TEST_ITR_FORCED_RUN, true); metricCollector.add(CiVisibilityCountMetric.ITR_FORCED_RUN, 1, EventType.TEST); } @@ -260,16 +260,6 @@ public void onTestIgnore( onTestFinish(testDescriptor, null); } - @Override - public boolean skip(TestIdentifier test) { - return testModule.skip(test); - } - - @Override - public boolean shouldBeSkipped(TestIdentifier test) { - return testModule.shouldBeSkipped(test); - } - @Override @Nonnull public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource) { @@ -286,6 +276,11 @@ public boolean isFlaky(TestIdentifier test) { return testModule.isFlaky(test); } + @Override + public boolean isSkippable(TestIdentifier test) { + return testModule.isSkippable(test); + } + @Override public void close() { testModule.end(null); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionResults.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionResults.java new file mode 100644 index 00000000000..44e70b78f68 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionResults.java @@ -0,0 +1,16 @@ +package datadog.trace.civisibility.test; + +import java.util.concurrent.atomic.LongAdder; + +public class ExecutionResults { + + private final LongAdder testsSkippedByItr = new LongAdder(); + + public void incrementTestsSkippedByItr() { + testsSkippedByItr.increment(); + } + + public long getTestsSkippedByItr() { + return testsSkippedByItr.sum(); + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java index ae3fcee1792..39e781b1469 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java @@ -16,7 +16,6 @@ import java.util.Collection; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.LongAdder; import javax.annotation.Nonnull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -25,7 +24,6 @@ public class ExecutionStrategy { private static final Logger LOGGER = LoggerFactory.getLogger(ExecutionStrategy.class); - private final LongAdder testsSkipped = new LongAdder(); private final AtomicInteger earlyFlakeDetectionsUsed = new AtomicInteger(0); private final AtomicInteger autoRetriesUsed = new AtomicInteger(0); @@ -50,10 +48,6 @@ public ExecutionSettings getExecutionSettings() { return executionSettings; } - public long getTestsSkipped() { - return testsSkipped.sum(); - } - public boolean isNew(TestIdentifier test) { Collection knownTests = executionSettings.getKnownTests(); return knownTests != null && !knownTests.contains(test.withoutParameters()); @@ -64,7 +58,7 @@ public boolean isFlaky(TestIdentifier test) { return flakyTests != null && flakyTests.contains(test.withoutParameters()); } - public boolean shouldBeSkipped(TestIdentifier test) { + public boolean isSkippable(TestIdentifier test) { if (test == null) { return false; } @@ -78,15 +72,6 @@ public boolean shouldBeSkipped(TestIdentifier test) { && testMetadata.isMissingLineCodeCoverage()); } - public boolean skip(TestIdentifier test) { - if (shouldBeSkipped(test)) { - testsSkipped.increment(); - return true; - } else { - return false; - } - } - @Nonnull public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource) { if (test == null) { diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy index 80026ece26d..6b7cf95617b 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy @@ -16,6 +16,7 @@ import datadog.trace.civisibility.decorator.TestDecoratorImpl import datadog.trace.civisibility.source.LinesResolver import datadog.trace.civisibility.source.NoOpSourcePathResolver import datadog.trace.civisibility.telemetry.CiVisibilityMetricCollectorImpl +import datadog.trace.civisibility.test.ExecutionResults import datadog.trace.civisibility.utils.SpanUtils class TestImplTest extends SpanWriterTest { @@ -97,6 +98,7 @@ class TestImplTest extends SpanWriterTest { def testFramework = TestFrameworkInstrumentation.OTHER def config = Config.get() def metricCollector = Stub(CiVisibilityMetricCollectorImpl) + def executionResults = Stub(ExecutionResults) def testDecorator = new TestDecoratorImpl("component", "session-name", "test-command", [:]) def linesResolver = Stub(LinesResolver) @@ -123,6 +125,7 @@ class TestImplTest extends SpanWriterTest { linesResolver, codeowners, coverageStoreFactory, + executionResults, SpanUtils.DO_NOT_PROPAGATE_CI_VISIBILITY_TAGS ) } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy index 78d53f7364b..2acfcc0dc79 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy @@ -14,6 +14,7 @@ import datadog.trace.civisibility.decorator.TestDecoratorImpl import datadog.trace.civisibility.source.LinesResolver import datadog.trace.civisibility.source.SourcePathResolver import datadog.trace.civisibility.telemetry.CiVisibilityMetricCollectorImpl +import datadog.trace.civisibility.test.ExecutionResults import datadog.trace.civisibility.utils.SpanUtils class TestSuiteImplTest extends SpanWriterTest { @@ -53,6 +54,7 @@ class TestSuiteImplTest extends SpanWriterTest { def testFramework = TestFrameworkInstrumentation.OTHER def config = Config.get() def metricCollector = Stub(CiVisibilityMetricCollectorImpl) + def executionResults = Stub(ExecutionResults) def testDecorator = new TestDecoratorImpl("component", "session-name", "test-command", [:]) def linesResolver = Stub(LinesResolver) @@ -82,6 +84,7 @@ class TestSuiteImplTest extends SpanWriterTest { codeowners, linesResolver, coverageStoreFactory, + executionResults, SpanUtils.DO_NOT_PROPAGATE_CI_VISIBILITY_TAGS ) } diff --git a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberItrInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberItrInstrumentation.java index 507ba3df32b..c6dce0fa24e 100644 --- a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberItrInstrumentation.java @@ -86,7 +86,7 @@ public static Boolean run( } TestIdentifier test = CucumberUtils.toTestIdentifier(description); - if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(test)) { + if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { notifier.fireTestAssumptionFailed( new Failure( description, diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4ItrInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4ItrInstrumentation.java index 78a7e575477..0755f08fbf2 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4ItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4ItrInstrumentation.java @@ -99,7 +99,7 @@ public static Boolean runChild( } TestIdentifier test = JUnit4Utils.toTestIdentifier(description); - if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(test)) { + if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { Description skippedDescription = JUnit4Utils.getSkippedDescription(description); notifier.fireTestIgnored(skippedDescription); return Boolean.FALSE; diff --git a/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberItrInstrumentation.java b/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberItrInstrumentation.java index 8491e1617af..c39fa1c179e 100644 --- a/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberItrInstrumentation.java @@ -101,7 +101,7 @@ public static void shouldBeSkipped( } TestIdentifier test = CucumberUtils.toTestIdentifier(testDescriptor); - if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(test)) { + if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON); } } diff --git a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockItrInstrumentation.java b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockItrInstrumentation.java index f12efa51f98..e3f962b2bac 100644 --- a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/JUnit5SpockItrInstrumentation.java @@ -80,6 +80,7 @@ public static void beforeSkipCheck() { CallDepthThreadLocalMap.incrementCallDepth(SpockNode.class); } + @SuppressWarnings("bytebuddy-exception-suppression") @SuppressFBWarnings( value = "UC_USELESS_OBJECT", justification = "skipResult is the return value of the instrumented method") @@ -117,21 +118,17 @@ public static void shouldBeSkipped( TestIdentifier featureIdentifier = SpockUtils.toTestIdentifier(feature); if (featureIdentifier == null - || !TestEventsHandlerHolder.TEST_EVENTS_HANDLER.shouldBeSkipped(featureIdentifier)) { + || !TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(featureIdentifier)) { return; } } - // all children are skippable - for (TestDescriptor feature : features) { - TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(SpockUtils.toTestIdentifier(feature)); - } skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON); } else { // individual test case TestIdentifier test = SpockUtils.toTestIdentifier(spockNode); - if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(test)) { + if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON); } } diff --git a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/retry/JUnit5SpockParameterizedRetryInstrumentation.java b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/retry/JUnit5SpockParameterizedRetryInstrumentation.java index 1a3f54851df..a6ad4ea135d 100644 --- a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/retry/JUnit5SpockParameterizedRetryInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/retry/JUnit5SpockParameterizedRetryInstrumentation.java @@ -59,6 +59,7 @@ public void methodAdvice(MethodTransformer transformer) { public static class SpockParameterizedRetryAdvice { + @SuppressWarnings("bytebuddy-exception-suppression") @SuppressFBWarnings( value = "UC_USELESS_OBJECT", justification = "executionListener is a field in the instrumented class") diff --git a/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5ItrInstrumentation.java b/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5ItrInstrumentation.java index b3211c22883..731b48fcb9d 100644 --- a/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5ItrInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnit5ItrInstrumentation.java @@ -97,7 +97,7 @@ public static void shouldBeSkipped( } TestIdentifier test = JUnitPlatformUtils.toTestIdentifier(testDescriptor); - if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(test)) { + if (test != null && TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) { skipResult = Node.SkipResult.skip(InstrumentationBridge.ITR_SKIP_REASON); } } diff --git a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java index 6a221b7b4f8..030d736d0ff 100644 --- a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java +++ b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java @@ -115,7 +115,7 @@ public boolean beforeScenario(ScenarioRuntime sr) { if (Config.get().isCiVisibilityTestSkippingEnabled() && !categories.contains(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) { TestIdentifier skippableTest = KarateUtils.toTestIdentifier(scenario); - if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(skippableTest)) { + if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(skippableTest)) { TestEventsHandlerHolder.TEST_EVENTS_HANDLER.onTestIgnore( suiteDescriptor, testDescriptor, diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java index 64400fa7617..4b0da0601b9 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java @@ -87,7 +87,7 @@ private Tuple2 skip( unskippableTests.add(test); return testNameAndSkipStatus; - } else if (eventHandler.skip(test)) { + } else if (eventHandler.isSkippable(test)) { skippedTests.add(test); return new Tuple2<>(testName, true); @@ -100,7 +100,7 @@ public boolean skip(TestIdentifier test, Map> tags) { if (isUnskippable(test, tags)) { unskippableTests.add(test); return false; - } else if (eventHandler.skip(test)) { + } else if (eventHandler.isSkippable(test)) { skippedTests.add(test); return true; } else { diff --git a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGItrInstrumentation.java b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGItrInstrumentation.java index df33279256e..1bba703c618 100644 --- a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGItrInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TestNGItrInstrumentation.java @@ -69,7 +69,7 @@ public static void invokeMethod( } TestIdentifier skippableTest = TestNGUtils.toTestIdentifier(method, instance, parameters); - if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skip(skippableTest)) { + if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(skippableTest)) { throw new SkipException(InstrumentationBridge.ITR_SKIP_REASON); } } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java b/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java index 2d2903457fd..5a30079dadb 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java @@ -83,10 +83,6 @@ void onTestIgnore( @Nonnull TestSourceData testSourceData, @Nullable String reason); - boolean skip(TestIdentifier test); - - boolean shouldBeSkipped(TestIdentifier test); - @Nonnull TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData source); @@ -94,6 +90,8 @@ void onTestIgnore( boolean isFlaky(TestIdentifier test); + boolean isSkippable(TestIdentifier test); + @Override void close(); From c58164b25db0304cc1e89a95cf2dff49440d3a27 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Mon, 27 Jan 2025 13:51:09 +0000 Subject: [PATCH 4/4] Probe for existence of IBMSASL or ACCP security providers. (#8276) If these providers exist in the running JDK then complete registration of other products on a different thread after premain to avoid eagerly loading 'java.util.logging' before main. A longer delay is added when we know a custom logging manager will be installed. Includes a fix for a race condition on IBM Java 8 where resuming Appsec/IAST could affect initial remote discovery. --- .../ddagent/SharedCommunicationObjects.java | 9 ++++ .../java/datadog/trace/bootstrap/Agent.java | 52 +++++++++++++------ 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java b/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java index 0309f5a38f1..894a9809e44 100644 --- a/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java +++ b/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java @@ -11,6 +11,7 @@ import datadog.remoteconfig.DefaultConfigurationPoller; import datadog.trace.api.Config; import datadog.trace.util.AgentTaskScheduler; +import java.security.Security; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -59,6 +60,7 @@ public void createRemaining(Config config) { } } + /** Registers a callback to be called when remote communications resume. */ public void whenReady(Runnable callback) { if (paused) { synchronized (pausedComponents) { @@ -71,8 +73,15 @@ public void whenReady(Runnable callback) { callback.run(); // not paused, run immediately } + /** Resumes remote communications including any paused callbacks. */ public void resume() { paused = false; + // attempt discovery first to avoid potential race condition on IBM Java8 + if (null != featuresDiscovery) { + featuresDiscovery.discoverIfOutdated(); + } else { + Security.getProviders(); // fallback to preloading provider extensions + } synchronized (pausedComponents) { for (Runnable callback : pausedComponents) { try { diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index f3d124cff7c..04361d159d6 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -1,7 +1,6 @@ package datadog.trace.bootstrap; import static datadog.trace.api.ConfigDefaults.DEFAULT_STARTUP_LOGS_ENABLED; -import static datadog.trace.api.Platform.getRuntimeVendor; import static datadog.trace.api.Platform.isJavaVersionAtLeast; import static datadog.trace.api.Platform.isOracleJDK8; import static datadog.trace.bootstrap.Library.WILDFLY; @@ -329,7 +328,7 @@ public void run() { if (appUsingCustomJMXBuilder) { log.debug("Custom JMX builder detected. Delaying JMXFetch initialization."); registerMBeanServerBuilderCallback(new StartJmxCallback(jmxStartDelay)); - // one minute fail-safe in case nothing touches JMX and and callback isn't triggered + // one minute fail-safe in case nothing touches JMX and callback isn't triggered scheduleJmxStart(60 + jmxStartDelay); } else if (appUsingCustomLogManager) { log.debug("Custom logger detected. Delaying JMXFetch initialization."); @@ -339,20 +338,31 @@ public void run() { } } - boolean delayOkHttp = appUsingCustomLogManager && okHttpMayIndirectlyLoadJUL(); - /* * Similar thing happens with DatadogTracer on (at least) zulu-8 because it uses OkHttp which indirectly loads JFR * events which in turn loads LogManager. This is not a problem on newer JDKs because there JFR uses different * logging facility. Likewise on IBM JDKs OkHttp may indirectly load 'IBMSASL' which in turn loads LogManager. */ + boolean delayOkHttp = !ciVisibilityEnabled && okHttpMayIndirectlyLoadJUL(); + boolean waitForJUL = appUsingCustomLogManager && delayOkHttp; + int okHttpDelayMillis; + if (waitForJUL) { + okHttpDelayMillis = 1_000; + } else if (delayOkHttp) { + okHttpDelayMillis = 100; + } else { + okHttpDelayMillis = 0; + } + InstallDatadogTracerCallback installDatadogTracerCallback = - new InstallDatadogTracerCallback(initTelemetry, inst, delayOkHttp); - if (delayOkHttp) { + new InstallDatadogTracerCallback(initTelemetry, inst, okHttpDelayMillis); + if (waitForJUL) { log.debug("Custom logger detected. Delaying Datadog Tracer initialization."); registerLogManagerCallback(installDatadogTracerCallback); + } else if (okHttpDelayMillis > 0) { + installDatadogTracerCallback.run(); // complete on different thread (after premain) } else { - installDatadogTracerCallback.execute(); + installDatadogTracerCallback.execute(); // complete on primordial thread in premain } /* @@ -362,7 +372,7 @@ public void run() { if (profilingEnabled && !isOracleJDK8()) { StaticEventLogger.begin("Profiling"); - if (delayOkHttp) { + if (waitForJUL) { log.debug("Custom logger detected. Delaying Profiling initialization."); registerLogManagerCallback(new StartProfilingAgentCallback(inst)); } else { @@ -499,18 +509,18 @@ protected static class InstallDatadogTracerCallback extends ClassLoadCallBack { private final Instrumentation instrumentation; private final Object sco; private final Class scoClass; - private final boolean delayOkHttp; + private final int okHttpDelayMillis; public InstallDatadogTracerCallback( InitializationTelemetry initTelemetry, Instrumentation instrumentation, - boolean delayOkHttp) { - this.delayOkHttp = delayOkHttp; + int okHttpDelayMillis) { + this.okHttpDelayMillis = okHttpDelayMillis; this.instrumentation = instrumentation; try { scoClass = AGENT_CLASSLOADER.loadClass("datadog.communication.ddagent.SharedCommunicationObjects"); - sco = scoClass.getConstructor(boolean.class).newInstance(delayOkHttp); + sco = scoClass.getConstructor(boolean.class).newInstance(okHttpDelayMillis > 0); } catch (ClassNotFoundException | NoSuchMethodException | InstantiationException @@ -530,7 +540,7 @@ public AgentThread agentThread() { @Override public void execute() { - if (delayOkHttp) { + if (okHttpDelayMillis > 0) { resumeRemoteComponents(); } @@ -550,7 +560,7 @@ private void resumeRemoteComponents() { try { // remote components were paused for custom log-manager/jmx-builder // add small delay before resuming remote I/O to help stabilization - Thread.sleep(1_000); + Thread.sleep(okHttpDelayMillis); scoClass.getMethod("resume").invoke(sco); } catch (InterruptedException ignore) { } catch (Throwable e) { @@ -1339,8 +1349,8 @@ private static String ddGetEnv(final String sysProp) { } private static boolean okHttpMayIndirectlyLoadJUL() { - if ("IBM Corporation".equals(getRuntimeVendor())) { - return true; // IBM JDKs ship with 'IBMSASL' which will load JUL when OkHttp accesses TLS + if (isIBMSASLInstalled() || isACCPInstalled()) { + return true; // 'IBMSASL' and 'ACCP' crypto providers can load JUL when OkHttp accesses TLS } if (isJavaVersionAtLeast(9)) { return false; // JDKs since 9 have reworked JFR to use a different logging facility, not JUL @@ -1348,6 +1358,16 @@ private static boolean okHttpMayIndirectlyLoadJUL() { return isJFRSupported(); // assume OkHttp will indirectly load JUL via its JFR events } + private static boolean isIBMSASLInstalled() { + return ClassLoader.getSystemResource("com/ibm/security/sasl/IBMSASL.class") != null; + } + + private static boolean isACCPInstalled() { + return ClassLoader.getSystemResource( + "com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.class") + != null; + } + private static boolean isJFRSupported() { // FIXME: this is quite a hack because there maybe jfr classes on classpath somehow that have // nothing to do with JDK - but this should be safe because only thing this does is to delay