Skip to content

Commit e4d4017

Browse files
bric3devflow.devflow-routing-intake
andauthored
FF crashtracking extended info, and improve runtime args filtering (#11048)
feat(crashtracking): Simplify sys property filtering Precisely this matches any `key` rather that composite terms like `app-key`, which covers more ground. feat(crashtracking): Redact args after agentlib and javaagent feat(crashtracking): Align some filtering with JDK JEP-8372760 JDK JEP-8372760 (JFR In-Process Data Redaction) feat(crashtracking): Don't send dd properties they should be avialable via telemetry feat(crashtracking): Add flag to enable extended info in crash reports Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 4fa94c4 commit e4d4017

File tree

11 files changed

+131
-40
lines changed

11 files changed

+131
-40
lines changed

dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/ConfigManager.java

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public static class StoredConfig {
3535
final String reportUUID;
3636
final boolean agentless;
3737
final boolean sendToErrorTracking;
38+
final boolean extendedInfoEnabled;
3839

3940
StoredConfig(
4041
String reportUUID,
@@ -45,7 +46,8 @@ public static class StoredConfig {
4546
String processTags,
4647
String runtimeId,
4748
boolean agentless,
48-
boolean sendToErrorTracking) {
49+
boolean sendToErrorTracking,
50+
boolean extendedInfoEnabled) {
4951
this.service = service;
5052
this.env = env;
5153
this.version = version;
@@ -55,6 +57,11 @@ public static class StoredConfig {
5557
this.reportUUID = reportUUID;
5658
this.agentless = agentless;
5759
this.sendToErrorTracking = sendToErrorTracking;
60+
this.extendedInfoEnabled = extendedInfoEnabled;
61+
}
62+
63+
public CrashUploaderSettings toCrashUploaderSettings() {
64+
return new CrashUploaderSettings(extendedInfoEnabled);
5865
}
5966

6067
public static class Builder {
@@ -67,6 +74,7 @@ public static class Builder {
6774
String reportUUID;
6875
boolean agentless;
6976
boolean sendToErrorTracking;
77+
boolean extendedInfoEnabled;
7078

7179
public Builder(Config config) {
7280
// get sane defaults
@@ -77,6 +85,7 @@ public Builder(Config config) {
7785
this.reportUUID = RandomUtils.randomUUID().toString();
7886
this.agentless = config.isCrashTrackingAgentless();
7987
this.sendToErrorTracking = config.isCrashTrackingErrorsIntakeEnabled();
88+
this.extendedInfoEnabled = config.isCrashTrackingExtendedInfoEnabled();
8089
}
8190

8291
public Builder service(String service) {
@@ -119,6 +128,11 @@ public Builder agentless(boolean agentless) {
119128
return this;
120129
}
121130

131+
public Builder extendedInfoEnabled(boolean extendedInfoEnabled) {
132+
this.extendedInfoEnabled = extendedInfoEnabled;
133+
return this;
134+
}
135+
122136
// @VisibleForTesting
123137
Builder reportUUID(String reportUUID) {
124138
this.reportUUID = reportUUID;
@@ -135,7 +149,8 @@ public StoredConfig build() {
135149
processTags,
136150
runtimeId,
137151
agentless,
138-
sendToErrorTracking);
152+
sendToErrorTracking,
153+
extendedInfoEnabled);
139154
}
140155
}
141156
}
@@ -194,6 +209,8 @@ static void writeConfigToFile(Config config, Path cfgPath, String... additionalE
194209
writeEntry(bw, "java_home", SystemProperties.get("java.home"));
195210
writeEntry(bw, "agentless", Boolean.toString(config.isCrashTrackingAgentless()));
196211
writeEntry(bw, "upload_to_et", Boolean.toString(config.isCrashTrackingErrorsIntakeEnabled()));
212+
writeEntry(
213+
bw, "extended_info", Boolean.toString(config.isCrashTrackingExtendedInfoEnabled()));
197214

198215
Runtime.getRuntime()
199216
.addShutdownHook(
@@ -257,6 +274,9 @@ public static StoredConfig readConfig(Config config, Path scriptPath) {
257274
case "upload_to_et":
258275
cfgBuilder.sendToErrorTracking(Boolean.parseBoolean(value));
259276
break;
277+
case "extended_info":
278+
cfgBuilder.extendedInfoEnabled(Boolean.parseBoolean(value));
279+
break;
260280
default:
261281
// ignore
262282
break;

dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploader.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ public void onResponse(Call call, Response response) throws IOException {
113113

114114
private final Config config;
115115
private final ConfigManager.StoredConfig storedConfig;
116+
private final CrashUploaderSettings uploaderSettings;
116117

117118
private final HttpUrl telemetryUrl;
118119
private final HttpUrl errorTrackingUrl;
@@ -131,6 +132,7 @@ public CrashUploader(@Nonnull final ConfigManager.StoredConfig storedConfig) {
131132
@NonNull final Config config, @Nonnull final ConfigManager.StoredConfig storedConfig) {
132133
this.config = config;
133134
this.storedConfig = storedConfig;
135+
this.uploaderSettings = storedConfig.toCrashUploaderSettings();
134136
this.telemetryUrl = HttpUrl.get(config.getFinalCrashTrackingTelemetryUrl());
135137
this.errorTrackingUrl = HttpUrl.get(config.getFinalCrashTrackingErrorTrackingUrl());
136138
this.agentless = config.isCrashTrackingAgentless();
@@ -525,7 +527,7 @@ private RequestBody makeErrorTrackingRequestBody(@Nonnull CrashLog payload, bool
525527
}
526528
writer.name("type").value(payload.error.kind);
527529
writer.name("message").value(payload.error.message);
528-
if (payload.error.threadName != null) {
530+
if (uploaderSettings.isExtendedInfoEnabled() && payload.error.threadName != null) {
529531
writer.name("thread_name").value(payload.error.threadName);
530532
}
531533
writer.name("source_type").value("Crashtracking");
@@ -588,7 +590,8 @@ private RequestBody makeErrorTrackingRequestBody(@Nonnull CrashLog payload, bool
588590
}
589591
writer.endObject();
590592
}
591-
if (payload.experimental.runtimeArgs != null) {
593+
if (uploaderSettings.isExtendedInfoEnabled()
594+
&& payload.experimental.runtimeArgs != null) {
592595
writer.name("runtime_args");
593596
writer.beginArray();
594597
for (String arg : payload.experimental.runtimeArgs) {
@@ -599,7 +602,7 @@ private RequestBody makeErrorTrackingRequestBody(@Nonnull CrashLog payload, bool
599602
writer.endObject();
600603
}
601604
// files (e.g. /proc/self/maps or dynamic_libraries)
602-
if (payload.files != null) {
605+
if (uploaderSettings.isExtendedInfoEnabled() && payload.files != null) {
603606
writer.name("files");
604607
writer.beginObject();
605608
writer.name(payload.files.name);
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package datadog.crashtracking;
2+
3+
/** Immutable settings that control what data {@link CrashUploader} includes in uploaded reports. */
4+
public final class CrashUploaderSettings {
5+
final boolean extendedInfoEnabled;
6+
7+
CrashUploaderSettings(boolean extendedInfoEnabled) {
8+
this.extendedInfoEnabled = extendedInfoEnabled;
9+
}
10+
11+
boolean isExtendedInfoEnabled() {
12+
return extendedInfoEnabled;
13+
}
14+
}

dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/parsers/RuntimeArgs.java

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,14 @@
1414
* <li>pre-split J9 user arguments, such as individual {@code 2CIUSERARG} records
1515
* </ul>
1616
*
17-
* <p>Only a curated subset of arguments is retained for telemetry: {@code -Ddd.*}, {@code -Djdk.*},
18-
* {@code -Djava.*}, {@code -Dsun.*}, {@code -javaagent:}, {@code -agentlib:}, {@code -X*}, and
17+
* <p>Only a curated subset of arguments is retained for telemetry: {@code -Djdk.*}, {@code
18+
* -Djava.*}, {@code -Dsun.*}, {@code -javaagent:}, {@code -agentlib:}, {@code -X*}, and
1919
* module/native-access options.
2020
*/
2121
final class RuntimeArgs {
22+
// Aligned with JDK JEP-8372760 (JFR In-Process Data Redaction) default filter list.
2223
private static final String[] SECRET_PROPERTY_KEYWORDS = {
23-
"password",
24-
"passwd",
25-
"secret",
26-
"token",
27-
"apikey",
28-
"api-key",
29-
"accesskey",
30-
"access-key",
31-
"privatekey",
32-
"private-key",
33-
"credential"
24+
"auth", "password", "passwd", "pwd", "passphrase", "secret", "token", "key", "credential"
3425
};
3526
private static final String[] MODULE_OPTIONS = {
3627
"--add-modules",
@@ -72,11 +63,13 @@ private static List<String> filterArgs(List<String> args) {
7263
if (arg == null || arg.isEmpty()) {
7364
continue;
7465
}
75-
if (isAllowedSystemProperty(arg)
76-
|| arg.startsWith("-javaagent:")
77-
|| arg.startsWith("-agentlib:")
78-
|| arg.startsWith("-X")
79-
|| isModuleOrNativeAccessOption(arg)) {
66+
if (isAllowedSystemProperty(arg)) {
67+
filtered.add(arg);
68+
} else if (arg.startsWith("-javaagent:") || arg.startsWith("-agentlib:")) {
69+
// Redact options after '=' — only the jar path / library name is sent
70+
int eq = arg.indexOf('=', arg.indexOf(':') + 1);
71+
filtered.add(eq >= 0 ? arg.substring(0, eq) + "=REDACTED" : arg);
72+
} else if (arg.startsWith("-X") || isModuleOrNativeAccessOption(arg)) {
8073
filtered.add(arg);
8174
}
8275
}
@@ -96,7 +89,7 @@ private static boolean isAllowedSystemProperty(String arg) {
9689
if (hasSecretLikePropertyName(arg)) {
9790
return false;
9891
}
99-
if (arg.startsWith("-Ddd.") || arg.startsWith("-Djdk.") || arg.startsWith("-Dosgi.")) {
92+
if (arg.startsWith("-Djdk.") || arg.startsWith("-Dosgi.")) {
10093
return true;
10194
}
10295
// J9 lists them as vm args.

dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/ConfigManagerTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public void testConfigWriteAndRead() throws IOException {
2424
.thenReturn(new WellKnownTags("1234", "", "env", "service", "version", ""));
2525
when(config.isCrashTrackingAgentless()).thenReturn(false);
2626
when(config.isCrashTrackingErrorsIntakeEnabled()).thenReturn(true);
27+
when(config.isCrashTrackingExtendedInfoEnabled()).thenReturn(true);
2728
when(config.getMergedCrashTrackingTags()).thenReturn(Collections.singletonMap("key", "value"));
2829
File tmpFile = File.createTempFile("ConfigManagerTest", null);
2930
tmpFile.deleteOnExit();
@@ -41,6 +42,7 @@ public void testConfigWriteAndRead() throws IOException {
4142
deserialized.processTags);
4243
assertFalse(deserialized.agentless);
4344
assertTrue(deserialized.sendToErrorTracking);
45+
assertTrue(deserialized.extendedInfoEnabled);
4446
}
4547

4648
@Test
@@ -56,5 +58,6 @@ public void testStoredConfigDefaults() {
5658
assertEquals("env", storedConfig.env);
5759
assertFalse(storedConfig.agentless);
5860
assertFalse(storedConfig.sendToErrorTracking);
61+
assertFalse(storedConfig.extendedInfoEnabled);
5962
}
6063
}

dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/CrashUploaderTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ public void testErrorTrackingHappyPath(String log) throws Exception {
323323
.processTags("a:b")
324324
.runtimeId("1234")
325325
.tags(ConfigManager.getMergedTagsForSerialization(Config.get())) // take the real ones
326+
.extendedInfoEnabled(true)
326327
.build();
327328
// When
328329
uploader = new CrashUploader(config, crashConfig);
@@ -362,6 +363,28 @@ public void testErrorTrackingHappyPath(String log) throws Exception {
362363
.isEqualTo(mapper.writeValueAsString(expected));
363364
}
364365

366+
@Test
367+
public void testErrorTrackingExcludesExtendedInfoByDefault() throws Exception {
368+
// extendedInfoEnabled defaults to false — files, thread_name and runtime_args must not appear
369+
ConfigManager.StoredConfig crashConfig =
370+
new ConfigManager.StoredConfig.Builder(config)
371+
.reportUUID(SAMPLE_UUID)
372+
.tags(ConfigManager.getMergedTagsForSerialization(Config.get()))
373+
.build();
374+
375+
uploader = new CrashUploader(config, crashConfig);
376+
server.enqueue(new MockResponse().setResponseCode(200));
377+
uploader.remoteUpload(readFileAsString("sample-crash-for-telemetry.txt"), false, true);
378+
379+
final RecordedRequest recordedRequest = server.takeRequest(5, TimeUnit.SECONDS);
380+
final ObjectMapper mapper = new ObjectMapper();
381+
final JsonNode event = mapper.readTree(recordedRequest.getBody().readUtf8());
382+
383+
assertNull(event.get("files"));
384+
assertNull(event.at("/error/thread_name").textValue());
385+
assertTrue(event.at("/experimental/runtime_args").isMissingNode());
386+
}
387+
365388
@Test
366389
public void testErrorTrackingSerializesRuntimeArgs() throws Exception {
367390
ConfigManager.StoredConfig crashConfig =
@@ -370,6 +393,7 @@ public void testErrorTrackingSerializesRuntimeArgs() throws Exception {
370393
.processTags("a:b")
371394
.runtimeId("1234")
372395
.tags(ConfigManager.getMergedTagsForSerialization(Config.get()))
396+
.extendedInfoEnabled(true)
373397
.build();
374398

375399
uploader = new CrashUploader(config, crashConfig);

dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/parsers/HotspotCrashLogParserTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ public void testRuntimeArgsFilteringFromHotspotJvmArgs() throws Exception {
106106
assertTrue(
107107
crashLog.experimental.runtimeArgs.contains(
108108
"-javaagent:/opt/REDACT_THIS/datadog-apm-agent/dd-java-agent.jar"));
109-
assertTrue(crashLog.experimental.runtimeArgs.contains("-Ddd.profiling.enabled=true"));
110-
assertTrue(crashLog.experimental.runtimeArgs.contains("-Ddd.service=REDACT_THIS"));
109+
assertFalse(crashLog.experimental.runtimeArgs.contains("-Ddd.profiling.enabled=true"));
110+
assertFalse(crashLog.experimental.runtimeArgs.contains("-Ddd.service=REDACT_THIS"));
111111
assertTrue(
112112
crashLog.experimental.runtimeArgs.stream().anyMatch(arg -> arg.startsWith("--add-opens=")));
113113
assertFalse(

0 commit comments

Comments
 (0)