Skip to content

Commit 7341990

Browse files
dougqhdougqh
authored
Merge branch 'master' into datadog/trace-agent-sampling-feedback-optimization
2 parents 7d94941 + c58164b commit 7341990

31 files changed

Lines changed: 199 additions & 129 deletions

File tree

‎communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import datadog.remoteconfig.DefaultConfigurationPoller;
1212
import datadog.trace.api.Config;
1313
import datadog.trace.util.AgentTaskScheduler;
14+
import java.security.Security;
1415
import java.util.ArrayList;
1516
import java.util.List;
1617
import java.util.concurrent.TimeUnit;
@@ -59,6 +60,7 @@ public void createRemaining(Config config) {
5960
}
6061
}
6162

63+
/** Registers a callback to be called when remote communications resume. */
6264
public void whenReady(Runnable callback) {
6365
if (paused) {
6466
synchronized (pausedComponents) {
@@ -71,8 +73,15 @@ public void whenReady(Runnable callback) {
7173
callback.run(); // not paused, run immediately
7274
}
7375

76+
/** Resumes remote communications including any paused callbacks. */
7477
public void resume() {
7578
paused = false;
79+
// attempt discovery first to avoid potential race condition on IBM Java8
80+
if (null != featuresDiscovery) {
81+
featuresDiscovery.discoverIfOutdated();
82+
} else {
83+
Security.getProviders(); // fallback to preloading provider extensions
84+
}
7685
synchronized (pausedComponents) {
7786
for (Runnable callback : pausedComponents) {
7887
try {

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package datadog.trace.bootstrap;
22

33
import static datadog.trace.api.ConfigDefaults.DEFAULT_STARTUP_LOGS_ENABLED;
4-
import static datadog.trace.api.Platform.getRuntimeVendor;
54
import static datadog.trace.api.Platform.isJavaVersionAtLeast;
65
import static datadog.trace.api.Platform.isOracleJDK8;
76
import static datadog.trace.bootstrap.Library.WILDFLY;
@@ -329,7 +328,7 @@ public void run() {
329328
if (appUsingCustomJMXBuilder) {
330329
log.debug("Custom JMX builder detected. Delaying JMXFetch initialization.");
331330
registerMBeanServerBuilderCallback(new StartJmxCallback(jmxStartDelay));
332-
// one minute fail-safe in case nothing touches JMX and and callback isn't triggered
331+
// one minute fail-safe in case nothing touches JMX and callback isn't triggered
333332
scheduleJmxStart(60 + jmxStartDelay);
334333
} else if (appUsingCustomLogManager) {
335334
log.debug("Custom logger detected. Delaying JMXFetch initialization.");
@@ -339,20 +338,31 @@ public void run() {
339338
}
340339
}
341340

342-
boolean delayOkHttp = appUsingCustomLogManager && okHttpMayIndirectlyLoadJUL();
343-
344341
/*
345342
* Similar thing happens with DatadogTracer on (at least) zulu-8 because it uses OkHttp which indirectly loads JFR
346343
* events which in turn loads LogManager. This is not a problem on newer JDKs because there JFR uses different
347344
* logging facility. Likewise on IBM JDKs OkHttp may indirectly load 'IBMSASL' which in turn loads LogManager.
348345
*/
346+
boolean delayOkHttp = !ciVisibilityEnabled && okHttpMayIndirectlyLoadJUL();
347+
boolean waitForJUL = appUsingCustomLogManager && delayOkHttp;
348+
int okHttpDelayMillis;
349+
if (waitForJUL) {
350+
okHttpDelayMillis = 1_000;
351+
} else if (delayOkHttp) {
352+
okHttpDelayMillis = 100;
353+
} else {
354+
okHttpDelayMillis = 0;
355+
}
356+
349357
InstallDatadogTracerCallback installDatadogTracerCallback =
350-
new InstallDatadogTracerCallback(initTelemetry, inst, delayOkHttp);
351-
if (delayOkHttp) {
358+
new InstallDatadogTracerCallback(initTelemetry, inst, okHttpDelayMillis);
359+
if (waitForJUL) {
352360
log.debug("Custom logger detected. Delaying Datadog Tracer initialization.");
353361
registerLogManagerCallback(installDatadogTracerCallback);
362+
} else if (okHttpDelayMillis > 0) {
363+
installDatadogTracerCallback.run(); // complete on different thread (after premain)
354364
} else {
355-
installDatadogTracerCallback.execute();
365+
installDatadogTracerCallback.execute(); // complete on primordial thread in premain
356366
}
357367

358368
/*
@@ -362,7 +372,7 @@ public void run() {
362372
if (profilingEnabled && !isOracleJDK8()) {
363373
StaticEventLogger.begin("Profiling");
364374

365-
if (delayOkHttp) {
375+
if (waitForJUL) {
366376
log.debug("Custom logger detected. Delaying Profiling initialization.");
367377
registerLogManagerCallback(new StartProfilingAgentCallback(inst));
368378
} else {
@@ -499,18 +509,18 @@ protected static class InstallDatadogTracerCallback extends ClassLoadCallBack {
499509
private final Instrumentation instrumentation;
500510
private final Object sco;
501511
private final Class<?> scoClass;
502-
private final boolean delayOkHttp;
512+
private final int okHttpDelayMillis;
503513

504514
public InstallDatadogTracerCallback(
505515
InitializationTelemetry initTelemetry,
506516
Instrumentation instrumentation,
507-
boolean delayOkHttp) {
508-
this.delayOkHttp = delayOkHttp;
517+
int okHttpDelayMillis) {
518+
this.okHttpDelayMillis = okHttpDelayMillis;
509519
this.instrumentation = instrumentation;
510520
try {
511521
scoClass =
512522
AGENT_CLASSLOADER.loadClass("datadog.communication.ddagent.SharedCommunicationObjects");
513-
sco = scoClass.getConstructor(boolean.class).newInstance(delayOkHttp);
523+
sco = scoClass.getConstructor(boolean.class).newInstance(okHttpDelayMillis > 0);
514524
} catch (ClassNotFoundException
515525
| NoSuchMethodException
516526
| InstantiationException
@@ -530,7 +540,7 @@ public AgentThread agentThread() {
530540

531541
@Override
532542
public void execute() {
533-
if (delayOkHttp) {
543+
if (okHttpDelayMillis > 0) {
534544
resumeRemoteComponents();
535545
}
536546

@@ -550,7 +560,7 @@ private void resumeRemoteComponents() {
550560
try {
551561
// remote components were paused for custom log-manager/jmx-builder
552562
// add small delay before resuming remote I/O to help stabilization
553-
Thread.sleep(1_000);
563+
Thread.sleep(okHttpDelayMillis);
554564
scoClass.getMethod("resume").invoke(sco);
555565
} catch (InterruptedException ignore) {
556566
} catch (Throwable e) {
@@ -1339,15 +1349,25 @@ private static String ddGetEnv(final String sysProp) {
13391349
}
13401350

13411351
private static boolean okHttpMayIndirectlyLoadJUL() {
1342-
if ("IBM Corporation".equals(getRuntimeVendor())) {
1343-
return true; // IBM JDKs ship with 'IBMSASL' which will load JUL when OkHttp accesses TLS
1352+
if (isIBMSASLInstalled() || isACCPInstalled()) {
1353+
return true; // 'IBMSASL' and 'ACCP' crypto providers can load JUL when OkHttp accesses TLS
13441354
}
13451355
if (isJavaVersionAtLeast(9)) {
13461356
return false; // JDKs since 9 have reworked JFR to use a different logging facility, not JUL
13471357
}
13481358
return isJFRSupported(); // assume OkHttp will indirectly load JUL via its JFR events
13491359
}
13501360

1361+
private static boolean isIBMSASLInstalled() {
1362+
return ClassLoader.getSystemResource("com/ibm/security/sasl/IBMSASL.class") != null;
1363+
}
1364+
1365+
private static boolean isACCPInstalled() {
1366+
return ClassLoader.getSystemResource(
1367+
"com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.class")
1368+
!= null;
1369+
}
1370+
13511371
private static boolean isJFRSupported() {
13521372
// FIXME: this is quite a hack because there maybe jfr classes on classpath somehow that have
13531373
// nothing to do with JDK - but this should be safe because only thing this does is to delay

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,12 @@ TestSuiteImpl testSuiteStart(
3131
boolean isModified(TestSourceData testSourceData);
3232

3333
/**
34-
* Checks if a given test should be skipped with Intelligent Test Runner or not
34+
* Checks if a given test can be skipped with Intelligent Test Runner or not.
3535
*
3636
* @param test Test to be checked
3737
* @return {@code true} if the test can be skipped, {@code false} otherwise
3838
*/
39-
boolean shouldBeSkipped(TestIdentifier test);
40-
41-
/**
42-
* Checks if a given test can be skipped with Intelligent Test Runner or not. If the test is
43-
* considered skippable, the count of skippable tests is incremented.
44-
*
45-
* @param test Test to be checked
46-
* @return {@code true} if the test can be skipped, {@code false} otherwise
47-
*/
48-
boolean skip(TestIdentifier test);
39+
boolean isSkippable(TestIdentifier test);
4940

5041
@Nonnull
5142
TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource);

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import datadog.trace.civisibility.source.LinesResolver;
3838
import datadog.trace.civisibility.source.SourcePathResolver;
3939
import datadog.trace.civisibility.source.SourceResolutionException;
40+
import datadog.trace.civisibility.test.ExecutionResults;
4041
import java.lang.reflect.Method;
4142
import java.util.Collection;
4243
import java.util.Collections;
@@ -50,6 +51,7 @@ public class TestImpl implements DDTest {
5051
private static final Logger log = LoggerFactory.getLogger(TestImpl.class);
5152

5253
private final CiVisibilityMetricCollector metricCollector;
54+
private final ExecutionResults executionResults;
5355
private final TestFrameworkInstrumentation instrumentation;
5456
private final AgentSpan span;
5557
private final DDTraceId sessionId;
@@ -78,11 +80,13 @@ public TestImpl(
7880
LinesResolver linesResolver,
7981
Codeowners codeowners,
8082
CoverageStore.Factory coverageStoreFactory,
83+
ExecutionResults executionResults,
8184
Consumer<AgentSpan> onSpanFinish) {
8285
this.instrumentation = instrumentation;
8386
this.metricCollector = metricCollector;
8487
this.sessionId = moduleSpanContext.getTraceId();
8588
this.suiteId = suiteId;
89+
this.executionResults = executionResults;
8690
this.onSpanFinish = onSpanFinish;
8791

8892
this.identifier = new TestIdentifier(testSuiteName, testName, testParameters);
@@ -258,6 +262,10 @@ public void end(@Nullable Long endTime) {
258262
span.finish();
259263
}
260264

265+
if (InstrumentationBridge.ITR_SKIP_REASON.equals(span.getTag(Tags.TEST_SKIP_REASON))) {
266+
executionResults.incrementTestsSkippedByItr();
267+
}
268+
261269
metricCollector.add(
262270
CiVisibilityCountMetric.EVENT_FINISHED,
263271
1,

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import datadog.trace.civisibility.source.LinesResolver;
2323
import datadog.trace.civisibility.source.SourcePathResolver;
2424
import datadog.trace.civisibility.source.SourceResolutionException;
25+
import datadog.trace.civisibility.test.ExecutionResults;
2526
import datadog.trace.civisibility.utils.SpanUtils;
2627
import java.lang.reflect.Method;
2728
import java.util.Collection;
@@ -49,6 +50,7 @@ public class TestSuiteImpl implements DDTestSuite {
4950
private final Codeowners codeowners;
5051
private final LinesResolver linesResolver;
5152
private final CoverageStore.Factory coverageStoreFactory;
53+
private final ExecutionResults executionResults;
5254
private final boolean parallelized;
5355
private final Consumer<AgentSpan> onSpanFinish;
5456

@@ -69,6 +71,7 @@ public TestSuiteImpl(
6971
Codeowners codeowners,
7072
LinesResolver linesResolver,
7173
CoverageStore.Factory coverageStoreFactory,
74+
ExecutionResults executionResults,
7275
Consumer<AgentSpan> onSpanFinish) {
7376
this.moduleSpanContext = moduleSpanContext;
7477
this.moduleName = moduleName;
@@ -84,6 +87,7 @@ public TestSuiteImpl(
8487
this.codeowners = codeowners;
8588
this.linesResolver = linesResolver;
8689
this.coverageStoreFactory = coverageStoreFactory;
90+
this.executionResults = executionResults;
8791
this.onSpanFinish = onSpanFinish;
8892

8993
AgentTracer.SpanBuilder spanBuilder =
@@ -254,6 +258,7 @@ public TestImpl testStart(
254258
linesResolver,
255259
codeowners,
256260
coverageStoreFactory,
261+
executionResults,
257262
SpanUtils.propagateCiVisibilityTagsTo(span));
258263
}
259264
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import datadog.trace.civisibility.ipc.TestFramework;
2626
import datadog.trace.civisibility.source.LinesResolver;
2727
import datadog.trace.civisibility.source.SourcePathResolver;
28+
import datadog.trace.civisibility.test.ExecutionResults;
2829
import datadog.trace.civisibility.test.ExecutionStrategy;
2930
import java.util.Collection;
3031
import java.util.TreeSet;
@@ -46,6 +47,7 @@ public class ProxyTestModule implements TestFrameworkModule {
4647
private final AgentSpanContext parentProcessModuleContext;
4748
private final String moduleName;
4849
private final ExecutionStrategy executionStrategy;
50+
private final ExecutionResults executionResults;
4951
private final SignalClient.Factory signalClientFactory;
5052
private final ChildProcessCoverageReporter childProcessCoverageReporter;
5153
private final Config config;
@@ -73,6 +75,7 @@ public ProxyTestModule(
7375
this.parentProcessModuleContext = parentProcessModuleContext;
7476
this.moduleName = moduleName;
7577
this.executionStrategy = executionStrategy;
78+
this.executionResults = new ExecutionResults();
7679
this.signalClientFactory = signalClientFactory;
7780
this.childProcessCoverageReporter = childProcessCoverageReporter;
7881
this.config = config;
@@ -100,13 +103,8 @@ public boolean isModified(TestSourceData testSourceData) {
100103
}
101104

102105
@Override
103-
public boolean shouldBeSkipped(TestIdentifier test) {
104-
return executionStrategy.shouldBeSkipped(test);
105-
}
106-
107-
@Override
108-
public boolean skip(TestIdentifier test) {
109-
return executionStrategy.skip(test);
106+
public boolean isSkippable(TestIdentifier test) {
107+
return executionStrategy.isSkippable(test);
110108
}
111109

112110
@Override
@@ -143,7 +141,7 @@ private void sendModuleExecutionResult() {
143141
boolean earlyFlakeDetectionEnabled = earlyFlakeDetectionSettings.isEnabled();
144142
boolean earlyFlakeDetectionFaulty =
145143
earlyFlakeDetectionEnabled && executionStrategy.isEFDLimitReached();
146-
long testsSkippedTotal = executionStrategy.getTestsSkipped();
144+
long testsSkippedTotal = executionResults.getTestsSkippedByItr();
147145

148146
signalClient.send(
149147
new ModuleExecutionResult(
@@ -185,6 +183,7 @@ public TestSuiteImpl testSuiteStart(
185183
codeowners,
186184
linesResolver,
187185
coverageStoreFactory,
186+
executionResults,
188187
this::propagateTestFrameworkData);
189188
}
190189

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import datadog.trace.civisibility.domain.TestSuiteImpl;
2323
import datadog.trace.civisibility.source.LinesResolver;
2424
import datadog.trace.civisibility.source.SourcePathResolver;
25+
import datadog.trace.civisibility.test.ExecutionResults;
2526
import datadog.trace.civisibility.test.ExecutionStrategy;
2627
import datadog.trace.civisibility.utils.SpanUtils;
2728
import java.util.function.Consumer;
@@ -39,6 +40,7 @@ public class HeadlessTestModule extends AbstractTestModule implements TestFramew
3940

4041
private final CoverageStore.Factory coverageStoreFactory;
4142
private final ExecutionStrategy executionStrategy;
43+
private final ExecutionResults executionResults;
4244

4345
public HeadlessTestModule(
4446
AgentSpanContext sessionSpanContext,
@@ -67,6 +69,7 @@ public HeadlessTestModule(
6769
onSpanFinish);
6870
this.coverageStoreFactory = coverageStoreFactory;
6971
this.executionStrategy = executionStrategy;
72+
this.executionResults = new ExecutionResults();
7073
}
7174

7275
@Override
@@ -85,13 +88,8 @@ public boolean isModified(TestSourceData testSourceData) {
8588
}
8689

8790
@Override
88-
public boolean shouldBeSkipped(TestIdentifier test) {
89-
return executionStrategy.shouldBeSkipped(test);
90-
}
91-
92-
@Override
93-
public boolean skip(TestIdentifier test) {
94-
return executionStrategy.skip(test);
91+
public boolean isSkippable(TestIdentifier test) {
92+
return executionStrategy.isSkippable(test);
9593
}
9694

9795
@Override
@@ -111,7 +109,7 @@ public void end(@Nullable Long endTime) {
111109
setTag(Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, true);
112110
setTag(Tags.TEST_ITR_TESTS_SKIPPING_TYPE, "test");
113111

114-
long testsSkippedTotal = executionStrategy.getTestsSkipped();
112+
long testsSkippedTotal = executionResults.getTestsSkippedByItr();
115113
setTag(Tags.TEST_ITR_TESTS_SKIPPING_COUNT, testsSkippedTotal);
116114
if (testsSkippedTotal > 0) {
117115
setTag(DDTags.CI_ITR_TESTS_SKIPPED, true);
@@ -154,6 +152,7 @@ public TestSuiteImpl testSuiteStart(
154152
codeowners,
155153
linesResolver,
156154
coverageStoreFactory,
155+
executionResults,
157156
SpanUtils.propagateCiVisibilityTagsTo(span));
158157
}
159158
}

0 commit comments

Comments
 (0)