Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public static class StoredConfig {
final String reportUUID;
final boolean agentless;
final boolean sendToErrorTracking;
final boolean extendedInfoEnabled;

StoredConfig(
String reportUUID,
Expand All @@ -45,7 +46,8 @@ public static class StoredConfig {
String processTags,
String runtimeId,
boolean agentless,
boolean sendToErrorTracking) {
boolean sendToErrorTracking,
boolean extendedInfoEnabled) {
this.service = service;
this.env = env;
this.version = version;
Expand All @@ -55,6 +57,11 @@ public static class StoredConfig {
this.reportUUID = reportUUID;
this.agentless = agentless;
this.sendToErrorTracking = sendToErrorTracking;
this.extendedInfoEnabled = extendedInfoEnabled;
}

public CrashUploaderSettings toCrashUploaderSettings() {
return new CrashUploaderSettings(extendedInfoEnabled);
}

public static class Builder {
Expand All @@ -67,6 +74,7 @@ public static class Builder {
String reportUUID;
boolean agentless;
boolean sendToErrorTracking;
boolean extendedInfoEnabled;

public Builder(Config config) {
// get sane defaults
Expand All @@ -77,6 +85,7 @@ public Builder(Config config) {
this.reportUUID = RandomUtils.randomUUID().toString();
this.agentless = config.isCrashTrackingAgentless();
this.sendToErrorTracking = config.isCrashTrackingErrorsIntakeEnabled();
this.extendedInfoEnabled = config.isCrashTrackingExtendedInfoEnabled();
}

public Builder service(String service) {
Expand Down Expand Up @@ -119,6 +128,11 @@ public Builder agentless(boolean agentless) {
return this;
}

public Builder extendedInfoEnabled(boolean extendedInfoEnabled) {
this.extendedInfoEnabled = extendedInfoEnabled;
return this;
}

// @VisibleForTesting
Builder reportUUID(String reportUUID) {
this.reportUUID = reportUUID;
Expand All @@ -135,7 +149,8 @@ public StoredConfig build() {
processTags,
runtimeId,
agentless,
sendToErrorTracking);
sendToErrorTracking,
extendedInfoEnabled);
}
}
}
Expand Down Expand Up @@ -194,6 +209,8 @@ static void writeConfigToFile(Config config, Path cfgPath, String... additionalE
writeEntry(bw, "java_home", SystemProperties.get("java.home"));
writeEntry(bw, "agentless", Boolean.toString(config.isCrashTrackingAgentless()));
writeEntry(bw, "upload_to_et", Boolean.toString(config.isCrashTrackingErrorsIntakeEnabled()));
writeEntry(
bw, "extended_info", Boolean.toString(config.isCrashTrackingExtendedInfoEnabled()));

Runtime.getRuntime()
.addShutdownHook(
Expand Down Expand Up @@ -257,6 +274,9 @@ public static StoredConfig readConfig(Config config, Path scriptPath) {
case "upload_to_et":
cfgBuilder.sendToErrorTracking(Boolean.parseBoolean(value));
break;
case "extended_info":
cfgBuilder.extendedInfoEnabled(Boolean.parseBoolean(value));
break;
default:
// ignore
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ public void onResponse(Call call, Response response) throws IOException {

private final Config config;
private final ConfigManager.StoredConfig storedConfig;
private final CrashUploaderSettings uploaderSettings;

private final HttpUrl telemetryUrl;
private final HttpUrl errorTrackingUrl;
Expand All @@ -131,6 +132,7 @@ public CrashUploader(@Nonnull final ConfigManager.StoredConfig storedConfig) {
@NonNull final Config config, @Nonnull final ConfigManager.StoredConfig storedConfig) {
this.config = config;
this.storedConfig = storedConfig;
this.uploaderSettings = storedConfig.toCrashUploaderSettings();
this.telemetryUrl = HttpUrl.get(config.getFinalCrashTrackingTelemetryUrl());
this.errorTrackingUrl = HttpUrl.get(config.getFinalCrashTrackingErrorTrackingUrl());
this.agentless = config.isCrashTrackingAgentless();
Expand Down Expand Up @@ -525,7 +527,7 @@ private RequestBody makeErrorTrackingRequestBody(@Nonnull CrashLog payload, bool
}
writer.name("type").value(payload.error.kind);
writer.name("message").value(payload.error.message);
if (payload.error.threadName != null) {
if (uploaderSettings.isExtendedInfoEnabled() && payload.error.threadName != null) {
writer.name("thread_name").value(payload.error.threadName);
}
writer.name("source_type").value("Crashtracking");
Expand Down Expand Up @@ -588,7 +590,8 @@ private RequestBody makeErrorTrackingRequestBody(@Nonnull CrashLog payload, bool
}
writer.endObject();
}
if (payload.experimental.runtimeArgs != null) {
if (uploaderSettings.isExtendedInfoEnabled()
&& payload.experimental.runtimeArgs != null) {
writer.name("runtime_args");
writer.beginArray();
for (String arg : payload.experimental.runtimeArgs) {
Expand All @@ -599,7 +602,7 @@ private RequestBody makeErrorTrackingRequestBody(@Nonnull CrashLog payload, bool
writer.endObject();
}
// files (e.g. /proc/self/maps or dynamic_libraries)
if (payload.files != null) {
if (uploaderSettings.isExtendedInfoEnabled() && payload.files != null) {
writer.name("files");
writer.beginObject();
writer.name(payload.files.name);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package datadog.crashtracking;

/** Immutable settings that control what data {@link CrashUploader} includes in uploaded reports. */
public final class CrashUploaderSettings {
final boolean extendedInfoEnabled;

CrashUploaderSettings(boolean extendedInfoEnabled) {
this.extendedInfoEnabled = extendedInfoEnabled;
}

boolean isExtendedInfoEnabled() {
return extendedInfoEnabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,14 @@
* <li>pre-split J9 user arguments, such as individual {@code 2CIUSERARG} records
* </ul>
*
* <p>Only a curated subset of arguments is retained for telemetry: {@code -Ddd.*}, {@code -Djdk.*},
* {@code -Djava.*}, {@code -Dsun.*}, {@code -javaagent:}, {@code -agentlib:}, {@code -X*}, and
* <p>Only a curated subset of arguments is retained for telemetry: {@code -Djdk.*}, {@code
* -Djava.*}, {@code -Dsun.*}, {@code -javaagent:}, {@code -agentlib:}, {@code -X*}, and
* module/native-access options.
*/
final class RuntimeArgs {
// Aligned with JDK JEP-8372760 (JFR In-Process Data Redaction) default filter list.
private static final String[] SECRET_PROPERTY_KEYWORDS = {
"password",
"passwd",
"secret",
"token",
"apikey",
"api-key",
"accesskey",
"access-key",
"privatekey",
"private-key",
"credential"
"auth", "password", "passwd", "pwd", "passphrase", "secret", "token", "key", "credential"
};
private static final String[] MODULE_OPTIONS = {
"--add-modules",
Expand Down Expand Up @@ -72,11 +63,13 @@ private static List<String> filterArgs(List<String> args) {
if (arg == null || arg.isEmpty()) {
continue;
}
if (isAllowedSystemProperty(arg)
|| arg.startsWith("-javaagent:")
|| arg.startsWith("-agentlib:")
|| arg.startsWith("-X")
|| isModuleOrNativeAccessOption(arg)) {
if (isAllowedSystemProperty(arg)) {
filtered.add(arg);
} else if (arg.startsWith("-javaagent:") || arg.startsWith("-agentlib:")) {
// Redact options after '=' — only the jar path / library name is sent
int eq = arg.indexOf('=', arg.indexOf(':') + 1);
filtered.add(eq >= 0 ? arg.substring(0, eq) + "=REDACTED" : arg);
} else if (arg.startsWith("-X") || isModuleOrNativeAccessOption(arg)) {
filtered.add(arg);
}
}
Expand All @@ -96,7 +89,7 @@ private static boolean isAllowedSystemProperty(String arg) {
if (hasSecretLikePropertyName(arg)) {
return false;
}
if (arg.startsWith("-Ddd.") || arg.startsWith("-Djdk.") || arg.startsWith("-Dosgi.")) {
if (arg.startsWith("-Djdk.") || arg.startsWith("-Dosgi.")) {
return true;
}
// J9 lists them as vm args.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public void testConfigWriteAndRead() throws IOException {
.thenReturn(new WellKnownTags("1234", "", "env", "service", "version", ""));
when(config.isCrashTrackingAgentless()).thenReturn(false);
when(config.isCrashTrackingErrorsIntakeEnabled()).thenReturn(true);
when(config.isCrashTrackingExtendedInfoEnabled()).thenReturn(true);
when(config.getMergedCrashTrackingTags()).thenReturn(Collections.singletonMap("key", "value"));
File tmpFile = File.createTempFile("ConfigManagerTest", null);
tmpFile.deleteOnExit();
Expand All @@ -41,6 +42,7 @@ public void testConfigWriteAndRead() throws IOException {
deserialized.processTags);
assertFalse(deserialized.agentless);
assertTrue(deserialized.sendToErrorTracking);
assertTrue(deserialized.extendedInfoEnabled);
}

@Test
Expand All @@ -56,5 +58,6 @@ public void testStoredConfigDefaults() {
assertEquals("env", storedConfig.env);
assertFalse(storedConfig.agentless);
assertFalse(storedConfig.sendToErrorTracking);
assertFalse(storedConfig.extendedInfoEnabled);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ public void testErrorTrackingHappyPath(String log) throws Exception {
.processTags("a:b")
.runtimeId("1234")
.tags(ConfigManager.getMergedTagsForSerialization(Config.get())) // take the real ones
.extendedInfoEnabled(true)
.build();
// When
uploader = new CrashUploader(config, crashConfig);
Expand Down Expand Up @@ -362,6 +363,28 @@ public void testErrorTrackingHappyPath(String log) throws Exception {
.isEqualTo(mapper.writeValueAsString(expected));
}

@Test
public void testErrorTrackingExcludesExtendedInfoByDefault() throws Exception {
// extendedInfoEnabled defaults to false — files, thread_name and runtime_args must not appear
ConfigManager.StoredConfig crashConfig =
new ConfigManager.StoredConfig.Builder(config)
.reportUUID(SAMPLE_UUID)
.tags(ConfigManager.getMergedTagsForSerialization(Config.get()))
.build();

uploader = new CrashUploader(config, crashConfig);
server.enqueue(new MockResponse().setResponseCode(200));
uploader.remoteUpload(readFileAsString("sample-crash-for-telemetry.txt"), false, true);

final RecordedRequest recordedRequest = server.takeRequest(5, TimeUnit.SECONDS);
final ObjectMapper mapper = new ObjectMapper();
final JsonNode event = mapper.readTree(recordedRequest.getBody().readUtf8());

assertNull(event.get("files"));
assertNull(event.at("/error/thread_name").textValue());
assertTrue(event.at("/experimental/runtime_args").isMissingNode());
}

@Test
public void testErrorTrackingSerializesRuntimeArgs() throws Exception {
ConfigManager.StoredConfig crashConfig =
Expand All @@ -370,6 +393,7 @@ public void testErrorTrackingSerializesRuntimeArgs() throws Exception {
.processTags("a:b")
.runtimeId("1234")
.tags(ConfigManager.getMergedTagsForSerialization(Config.get()))
.extendedInfoEnabled(true)
.build();

uploader = new CrashUploader(config, crashConfig);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ public void testRuntimeArgsFilteringFromHotspotJvmArgs() throws Exception {
assertTrue(
crashLog.experimental.runtimeArgs.contains(
"-javaagent:/opt/REDACT_THIS/datadog-apm-agent/dd-java-agent.jar"));
assertTrue(crashLog.experimental.runtimeArgs.contains("-Ddd.profiling.enabled=true"));
assertTrue(crashLog.experimental.runtimeArgs.contains("-Ddd.service=REDACT_THIS"));
assertFalse(crashLog.experimental.runtimeArgs.contains("-Ddd.profiling.enabled=true"));
assertFalse(crashLog.experimental.runtimeArgs.contains("-Ddd.service=REDACT_THIS"));
assertTrue(
crashLog.experimental.runtimeArgs.stream().anyMatch(arg -> arg.startsWith("--add-opens=")));
assertFalse(
Expand Down
Loading
Loading