Skip to content

Commit d3fe671

Browse files
AlexeyKuznetsov-DDAlexeyKuznetsov-DD
authored
Merge branch 'master' into alexeyk/smoke-tests-start-agent-before-load
2 parents bf54895 + 2ec3a87 commit d3fe671

File tree

10 files changed

+505
-22
lines changed

10 files changed

+505
-22
lines changed

‎dd-java-agent/instrumentation/maven/maven-3.2.1/build.gradle‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ muzzle {
1111

1212
addTestSuiteForDir('latestDepTest', 'test')
1313

14-
tasks.named("latestDepTest", Test) {
15-
systemProperty 'test.isLatestDepTest', 'true'
16-
}
17-
1814
dependencies {
1915
compileOnly 'org.apache.maven:maven-embedder:3.2.1'
2016

‎dd-java-agent/instrumentation/maven/maven-3.2.1/src/test/groovy/MavenInstrumentationTest.groovy‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ import datadog.trace.api.config.CiVisibilityConfig
22
import datadog.trace.civisibility.CiVisibilityInstrumentationTest
33
import org.apache.maven.cli.MavenCli
44
import org.codehaus.plexus.util.FileUtils
5+
import org.slf4j.MDC
56
import spock.lang.TempDir
67

78
import java.nio.file.Path
89
import java.nio.file.Paths
910

1011
import static org.junit.jupiter.api.Assertions.assertEquals
11-
import static org.junit.jupiter.api.Assumptions.abort
1212

1313
class MavenInstrumentationTest extends CiVisibilityInstrumentationTest {
1414

@@ -19,6 +19,14 @@ class MavenInstrumentationTest extends CiVisibilityInstrumentationTest {
1919

2020
@Override
2121
def setup() {
22+
// Workaround for maven-surefire 3.5.5 bug (https://github.com/apache/maven-surefire/pull/3241):
23+
// ThreadedStreamConsumer$Pumper.run() calls MDC.setContextMap(MDC.getCopyOfContextMap()),
24+
// but LogbackMDCAdapter.getCopyOfContextMap() returns null when MDC is uninitialized,
25+
// and LogbackMDCAdapter.setContextMap(null) throws NPE via HashMap.putAll(null).
26+
// Pre-initializing MDC ensures getCopyOfContextMap() returns an empty map instead of null.
27+
MDC.put("_init", "true")
28+
MDC.remove("_init")
29+
2230
System.setProperty("maven.multiModuleProjectDirectory", projectFolder.toAbsolutePath().toString())
2331
givenMavenProjectFiles((String) specificationContext.currentIteration.dataVariables.testcaseName)
2432
givenMavenDependenciesAreLoaded()
@@ -32,10 +40,6 @@ class MavenInstrumentationTest extends CiVisibilityInstrumentationTest {
3240
}
3341

3442
def "test #testcaseName"() {
35-
if (skipLatest && Boolean.getBoolean("test.isLatestDepTest")) {
36-
abort("Skipping latest dep test")
37-
}
38-
3943
String workingDirectory = projectFolder.toString()
4044

4145
def exitCode = new MavenCli().doMain(args.toArray(new String[0]), workingDirectory, null, null)
@@ -44,15 +48,15 @@ class MavenInstrumentationTest extends CiVisibilityInstrumentationTest {
4448
assertSpansData(testcaseName)
4549

4650
where:
47-
testcaseName | args | expectedExitCode | skipLatest
48-
"test_maven_build_with_no_tests_generates_spans" | ["-B", "verify"] | 0 | false
49-
"test_maven_build_with_incorrect_command_generates_spans" | ["-B", "unknownPhase"] | 1 | false
50-
"test_maven_build_with_tests_generates_spans" | ["-B", "clean", "test"] | 0 | false
51-
"test_maven_build_with_failed_tests_generates_spans" | ["-B", "clean", "test"] | 1 | false
52-
"test_maven_build_with_tests_in_multiple_modules_generates_spans" | ["-B", "clean", "test"] | 1 | false
53-
"test_maven_build_with_tests_in_multiple_modules_run_in_parallel_generates_spans" | ["-B", "-T4", "clean", "test"] | 0 | false
54-
"test_maven_build_with_unit_and_integration_tests_generates_spans" | ["-B", "verify"] | 0 | true // temporary workaround to avoid failures with maven-failsafe-plugin 3.5.5
55-
"test_maven_build_with_no_fork_generates_spans" | ["-B", "clean", "test"] | 0 | false
51+
testcaseName | args | expectedExitCode
52+
"test_maven_build_with_no_tests_generates_spans" | ["-B", "verify"] | 0
53+
"test_maven_build_with_incorrect_command_generates_spans" | ["-B", "unknownPhase"] | 1
54+
"test_maven_build_with_tests_generates_spans" | ["-B", "clean", "test"] | 0
55+
"test_maven_build_with_failed_tests_generates_spans" | ["-B", "clean", "test"] | 1
56+
"test_maven_build_with_tests_in_multiple_modules_generates_spans" | ["-B", "clean", "test"] | 1
57+
"test_maven_build_with_tests_in_multiple_modules_run_in_parallel_generates_spans" | ["-B", "-T4", "clean", "test"] | 0
58+
"test_maven_build_with_unit_and_integration_tests_generates_spans" | ["-B", "verify"] | 0
59+
"test_maven_build_with_no_fork_generates_spans" | ["-B", "clean", "test"] | 0
5660
}
5761

5862
private void givenMavenProjectFiles(String projectFilesSources) {

‎dd-java-agent/instrumentation/spark/spark-common/build.gradle‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ configurations.configureEach {
1010
dependencies {
1111
compileOnly group: 'org.apache.spark', name: 'spark-core_2.12', version: '2.4.0'
1212
compileOnly group: 'org.apache.spark', name: 'spark-sql_2.12', version: '2.4.0'
13+
compileOnly group: 'org.apache.spark', name: 'spark-launcher_2.12', version: '2.4.0'
1314

1415
testFixturesImplementation group: 'com.datadoghq', name: 'sketches-java', version: '0.8.2'
1516
testFixturesImplementation group: 'com.google.protobuf', name: 'protobuf-java', version: '3.14.0'
@@ -21,7 +22,12 @@ dependencies {
2122
testFixturesCompileOnly group: 'org.apache.spark', name: 'spark-core_2.12', version: '2.4.0'
2223
testFixturesCompileOnly group: 'org.apache.spark', name: 'spark-sql_2.12', version: '2.4.0'
2324
testFixturesCompileOnly group: 'org.apache.spark', name: 'spark-yarn_2.12', version: '2.4.0'
25+
testFixturesCompileOnly group: 'org.apache.spark', name: 'spark-launcher_2.12', version: '2.4.0'
2426

2527
testFixturesCompileOnly(libs.bundles.groovy)
2628
testFixturesCompileOnly(libs.bundles.spock)
29+
30+
testImplementation project(':dd-java-agent:instrumentation-testing')
31+
testImplementation group: 'org.apache.spark', name: 'spark-launcher_2.12', version: '2.4.0'
2732
}
33+

‎dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/AbstractSparkInstrumentation.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ public static void exit(@Advice.Thrown Throwable throwable) {
106106
if (AbstractDatadogSparkListener.listener != null) {
107107
AbstractDatadogSparkListener.listener.finishApplication(
108108
System.currentTimeMillis(), throwable, 0, null);
109+
} else {
110+
SparkLauncherListener.finishSpanWithThrowable(throwable);
109111
}
110112
}
111113
}

‎dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkConfAllowList.java‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717
* @see <a href="https://spark.apache.org/docs/latest/configuration.html">Spark Configuration</a>
1818
*/
1919
class SparkConfAllowList {
20+
// Using values from
21+
// https://github.com/apache/spark/blob/v3.5.1/core/src/main/scala/org/apache/spark/internal/config/package.scala#L1150-L1158
22+
static final String DEFAULT_REDACTION_REGEX = "(?i)secret|password|token|access.key|api.key";
23+
24+
private static final Pattern DEFAULT_REDACTION_PATTERN = Pattern.compile(DEFAULT_REDACTION_REGEX);
25+
2026
/**
2127
* Job-specific parameters that can be used to control job execution or provide metadata about the
2228
* job being executed
@@ -80,11 +86,17 @@ public static boolean canCaptureJobParameter(String parameterName) {
8086
return allowedJobParams.contains(parameterName);
8187
}
8288

89+
/** Redact a value if the key or value matches the default redaction pattern. */
90+
public static String redactValue(String key, String value) {
91+
if (DEFAULT_REDACTION_PATTERN.matcher(key).find()
92+
|| DEFAULT_REDACTION_PATTERN.matcher(value).find()) {
93+
return "[redacted]";
94+
}
95+
return value;
96+
}
97+
8398
public static List<Map.Entry<String, String>> getRedactedSparkConf(SparkConf conf) {
84-
// Using values from
85-
// https://github.com/apache/spark/blob/v3.5.1/core/src/main/scala/org/apache/spark/internal/config/package.scala#L1150-L1158
86-
String redactionPattern =
87-
conf.get("spark.redaction.regex", "(?i)secret|password|token|access.key|api.key");
99+
String redactionPattern = conf.get("spark.redaction.regex", DEFAULT_REDACTION_REGEX);
88100
List<Map.Entry<String, String>> redacted = new ArrayList<>();
89101
Pattern pattern = Pattern.compile(redactionPattern);
90102

‎dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkLauncherInstrumentation.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package datadog.trace.instrumentation.spark;
2+
3+
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
4+
import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
5+
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
6+
7+
import com.google.auto.service.AutoService;
8+
import datadog.trace.agent.tooling.Instrumenter;
9+
import datadog.trace.agent.tooling.InstrumenterModule;
10+
import datadog.trace.api.InstrumenterConfig;
11+
import net.bytebuddy.asm.Advice;
12+
import org.apache.spark.launcher.SparkAppHandle;
13+
14+
@AutoService(InstrumenterModule.class)
15+
public class SparkLauncherInstrumentation extends InstrumenterModule.Tracing
16+
implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice {
17+
18+
public SparkLauncherInstrumentation() {
19+
super("spark-launcher");
20+
}
21+
22+
@Override
23+
protected boolean defaultEnabled() {
24+
return InstrumenterConfig.get().isDataJobsEnabled();
25+
}
26+
27+
@Override
28+
public String instrumentedType() {
29+
return "org.apache.spark.launcher.SparkLauncher";
30+
}
31+
32+
@Override
33+
public String[] helperClassNames() {
34+
return new String[] {
35+
packageName + ".SparkConfAllowList", packageName + ".SparkLauncherListener",
36+
};
37+
}
38+
39+
@Override
40+
public void methodAdvice(MethodTransformer transformer) {
41+
transformer.applyAdvice(
42+
isMethod()
43+
.and(named("startApplication"))
44+
.and(isDeclaredBy(named("org.apache.spark.launcher.SparkLauncher"))),
45+
SparkLauncherInstrumentation.class.getName() + "$StartApplicationAdvice");
46+
}
47+
48+
public static class StartApplicationAdvice {
49+
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
50+
public static void exit(
51+
@Advice.This Object launcher,
52+
@Advice.Return SparkAppHandle handle,
53+
@Advice.Thrown Throwable throwable) {
54+
SparkLauncherListener.createLauncherSpan(launcher);
55+
56+
if (throwable != null) {
57+
SparkLauncherListener.finishSpanWithThrowable(throwable);
58+
return;
59+
}
60+
61+
if (handle != null) {
62+
handle.addListener(new SparkLauncherListener());
63+
}
64+
}
65+
}
66+
}

0 commit comments

Comments
 (0)