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 @@ -123,10 +123,9 @@ private long getRateLimitResetTime(okhttp3.Response response) {
return Long.parseLong(rateLimitHeader);
} catch (NumberFormatException e) {
log.error(
"Could not parse "
+ X_RATELIMIT_RESET_HTTP_HEADER
+ " header contents: "
+ rateLimitHeader,
"Could not parse {} header contents: {}",
X_RATELIMIT_RESET_HTTP_HEADER,
rateLimitHeader,
e);
return RATE_LIMIT_RESET_TIME_UNDEFINED;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,9 @@ static Map<String, String> getRemoteEnvironment(String url, String key, OkHttpCl
return adapter.fromJson(response.body().source());
} else {
logger.warn(
"Could not get remote CI environment (HTTP code "
+ response.code()
+ ")"
+ (response.body() != null ? ": " + response.body().string() : ""));
"Could not get remote CI environment (HTTP code {}) {}",
response.code(),
response.body() != null ? ": " + response.body().string() : "");
return Collections.emptyMap();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,21 @@ private static boolean copyCrashUploaderScript(
} catch (UnsupportedOperationException e) {
LOG.warn(
SEND_TELEMETRY,
"Unsupported permissions '"
+ RWXRWXRWX
+ "' for "
+ scriptDirectory
+ ". "
+ SETUP_FAILURE_MESSAGE);
"Unsupported permissions '" + RWXRWXRWX + "' for {}. " + SETUP_FAILURE_MESSAGE,
scriptDirectory);
return false;
} catch (FileAlreadyExistsException ignored) {
// can be safely ignored; if the folder exists we will just reuse it
if (!Files.isWritable(scriptDirectory)) {
LOG.warn(
SEND_TELEMETRY,
"Read only directory " + scriptDirectory + ". " + SETUP_FAILURE_MESSAGE);
SEND_TELEMETRY, "Read only directory {}. " + SETUP_FAILURE_MESSAGE, scriptDirectory);
return false;
}
} catch (IOException e) {
LOG.warn(
SEND_TELEMETRY,
"Failed to create writable crash tracking script folder "
+ scriptDirectory
+ ". "
+ SETUP_FAILURE_MESSAGE);
"Failed to create writable crash tracking script folder {}. " + SETUP_FAILURE_MESSAGE,
scriptDirectory);
return false;
}
try {
Expand All @@ -97,7 +90,8 @@ private static boolean copyCrashUploaderScript(
} catch (IOException e) {
LOG.warn(
SEND_TELEMETRY,
"Failed to copy crash tracking script " + scriptPath + ". " + SETUP_FAILURE_MESSAGE);
"Failed to copy crash tracking script {}. " + SETUP_FAILURE_MESSAGE,
scriptPath);
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,12 @@ static void writeConfig(Path scriptPath, String... entries) {
LOG.debug("Deleting config file: {}", cfgPath);
Files.deleteIfExists(cfgPath);
} catch (IOException e) {
LOG.warn(SEND_TELEMETRY, "Failed deleting config file: " + cfgPath, e);
LOG.warn(SEND_TELEMETRY, "Failed deleting config file: {}", cfgPath, e);
}
}));
LOG.debug("Config file written: {}", cfgPath);
} catch (IOException e) {
LOG.warn(SEND_TELEMETRY, "Failed writing config file: " + cfgPath);
LOG.warn(SEND_TELEMETRY, "Failed writing config file: {}", cfgPath);
try {
Files.deleteIfExists(cfgPath);
} catch (IOException ignored) {
Expand Down Expand Up @@ -349,10 +349,8 @@ private static void logInitializationError(String msg, Throwable t) {
} else {
LOG.warn(
SEND_TELEMETRY,
msg
+ " ["
+ t.getMessage()
+ "] (Change the logging level to debug to see the full stacktrace)");
msg + " [{}] (Change the logging level to debug to see the full stacktrace)",
t.getMessage());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ static void initialize(String onOutOfMemoryVal) {
if (scriptPath == null) {
LOG.error(
SEND_TELEMETRY,
"OOME notifier script value ("
+ onOutOfMemoryVal
+ ") does not follow the expected format: <path>/dd_oome_notifier.(sh|bat) %p. OOME tracking is disabled.");
"OOME notifier script value ({}) does not follow the expected format: <path>/dd_oome_notifier.(sh|bat) %p. OOME tracking is disabled.",
onOutOfMemoryVal);
return;
}
String agentJar = findAgentJar();
Expand Down Expand Up @@ -86,9 +85,8 @@ private static boolean copyOOMEscript(Path scriptPath) {
if (!Files.isWritable(scriptDirectory)) {
LOG.warn(
SEND_TELEMETRY,
"Read only directory "
+ scriptDirectory
+ ". OOME notification will not work properly.");
"Read only directory {}. OOME notification will not work properly.",
scriptDirectory);
return false;
}
} else {
Expand All @@ -97,22 +95,19 @@ private static boolean copyOOMEscript(Path scriptPath) {
} catch (UnsupportedOperationException e) {
LOG.warn(
SEND_TELEMETRY,
"Unsupported permissions '"
"Unsupported permissions '{"
+ RWXRWXRWX
+ "' for "
+ scriptDirectory
+ ". OOME notification will not work properly.");
+ "' for {}. OOME notification will not work properly.",
scriptDirectory);
return false;
} catch (FileAlreadyExistsException ignored) {
LOG.warn(
SEND_TELEMETRY, "Path " + scriptDirectory + " already exists and is not a directory.");
LOG.warn(SEND_TELEMETRY, "Path {} already exists and is not a directory.", scriptDirectory);
return false;
} catch (IOException e) {
LOG.warn(
SEND_TELEMETRY,
"Failed to create writable OOME script folder "
+ scriptDirectory
+ ". OOME notification will not work properly.");
"Failed to create writable OOME script folder {}. OOME notification will not work properly.",
scriptDirectory);
return false;
}

Expand All @@ -125,9 +120,8 @@ private static boolean copyOOMEscript(Path scriptPath) {
} catch (IOException e) {
LOG.warn(
SEND_TELEMETRY,
"Failed to copy OOME script "
+ scriptPath
+ ". OOME notification will not work properly.");
"Failed to copy OOME script {}. OOME notification will not work properly.",
scriptPath);
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ private void storeDebuggerDefinitions(ConfigurationComparer changes) {
public ProbeImplementation resolve(String encodedProbeId) {
ProbeDefinition definition = appliedDefinitions.get(encodedProbeId);
if (definition == null) {
LOGGER.warn(SEND_TELEMETRY, "Cannot resolve probe id=" + encodedProbeId);
LOGGER.warn(SEND_TELEMETRY, "Cannot resolve probe id={}", encodedProbeId);
Copy link
Member

@jpbempel jpbempel Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is on purpose I want the probe ID sent to telemetry to help troubleshoot when we have this case!
this is not sensitive information. nothing related to the customer, this is a technical id

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll roll back this one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, how concat and param are different here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, how concat and param are different here?

Only the format string "Cannot resolve probe id={}" is sent to the telemetry. Concatenation makes the otherwise hidden arguments available to the telemetry.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, we are hiding arguments to avoid sensitive data leakage? Correct?
If that true, I wish we have API for logging, that will support params and some flag managing what should be hidden... Something self-descriptive and better than concat... because you never know if this concat for purpose or just a human mistake...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is on purpose I want the probe ID sent to telemetry to help troubleshoot when we have this case!

The logs that we send to the intake here are not really for normal troubleshooting and they are supposed to be constant message templates only so they can be properly deduplicated to reduce load.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this could be addressed with an API change, but we currently use the standard Logger API backed by the TelemetryLogger, which reports errors to the backend.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is on purpose I want the probe ID sent to telemetry to help troubleshoot when we have this case!

The logs that we send to the intake here are not really for normal troubleshooting and they are supposed to be constant message templates only so they can be properly deduplicated to reduce load.

Got it! Rolling back the rollback.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those messages are cases, like exceptions.
If we cannot use them as telemetry for troubleshooting, they are useless!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. This should be allowed. Since the call is explicitly marked with SEND_TELEMETRY, we should permit sending the necessary additional information. To address concerns about cardinality, we should consider leveraging tags to capture this type of information.

}
return definition;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public Set<String> getThirdPartyLibraries(Config config) {
excludes.addAll(defaults.getPrefixes());
return excludes;
} catch (Exception e) {
LOGGER.error("Failed reading " + FILE_NAME + ". Treating all classes as third party.", e);
LOGGER.error("Failed reading {}. Treating all classes as third party.", FILE_NAME, e);
return getExcludeAll();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,9 @@ protected void flushIfNecessary() {
this.buffer.clear();
} else {
log.error(
"Could not submit eval metrics (HTTP code "
+ response.code()
+ ")"
+ (response.body() != null ? ": " + response.body().string() : ""));
"Could not submit eval metrics (HTTP code {}) {}",
response.code(),
response.body() != null ? response.body().string() : "");
}
} catch (Exception e) {
log.error("Could not submit eval metrics", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public static Controller build(ConfigProvider provider, ControllerContext contex
Class.forName("com.oracle.jrockit.jfr.Producer");
controllers.add(OracleJdkController.instance(provider));
} catch (Throwable t) {
log.debug(SEND_TELEMETRY, "Failed to load oracle profiler: " + t.getMessage(), t);
log.debug(SEND_TELEMETRY, "Failed to load oracle profiler: {}", t.getMessage(), t);
}
}
if (!isOracleJDK8) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ private static void unzipJar(Consumer<File> invoker, File file) throws IOExcepti
log.debug("Adding new jar: {}", temp.getAbsolutePath());
recursiveDependencySearch(invoker, temp);
if (!temp.delete()) {
log.error("Error deleting temp file:{}", temp.getAbsolutePath());
log.error("Error deleting temp file: {}", temp.getAbsolutePath());
}
} catch (Exception ex) {
log.error("Error unzipping file", ex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ private static Class<?> getJacocoClass(
return classLoader.loadClass(className);

} catch (Throwable throwable) {
log.error("Could not load Jacoco class: " + className, throwable);
log.error("Could not load Jacoco class: {}", className, throwable);
return null;
}
}
Expand All @@ -73,12 +73,10 @@ private static MethodHandle accessMethod(
return lookup.unreflect(method);
} catch (Throwable throwable) {
log.error(
"Could not find method "
+ methodName
+ " with arguments "
+ Arrays.toString(arguments)
+ " in class "
+ clazz.getName(),
"Could not find method {} with arguments {} in class {}",
methodName,
Arrays.toString(arguments),
clazz.getName(),
throwable);
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static BlockingException callRequestPathParamsCallback(
try {
return doCallRequestPathParamsCallback(reqCtx, params, origin);
} catch (Exception e) {
log.warn("Error calling " + origin, e);
log.warn("Error calling {}", origin, e);
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static BlockingException callRequestPathParamsCallback(
try {
return doCallRequestPathParamsCallback(reqCtx, params, origin);
} catch (Exception e) {
log.warn("Error calling " + origin, e);
log.warn("Error calling {}", origin, e);
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public final synchronized void start() throws Exception {
log.info("Started " + this);
started = true;
} catch (Exception e) {
log.warn("Problem starting " + this, e);
log.warn("Problem starting {}" + this, e);
throw e;
}
}
Expand All @@ -26,7 +26,7 @@ public final synchronized void stop() throws Exception {
log.info("Stopped " + this);
started = false;
} catch (Exception e) {
log.warn("Problem stopping " + this, e);
log.warn("Problem stopping {}" + this, e);
throw e;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,23 +81,21 @@ public GitInfo build(@Nullable String repositoryPath) {
String repoUrl = gitInfo.getRepositoryURL();
if (repoUrl == null || repoUrl.isEmpty()) {
log.error(
"Could not resolve git repository URL (can be provided via "
+ Strings.propertyNameToEnvironmentVariableName(DD_GIT_REPOSITORY_URL)
+ " env var or corresponding system property, "
+ GeneralConfig.TAGS
+ " config property or by embedding git metadata at build time)");
"Could not resolve git repository URL (can be provided via {} env var or corresponding system property, {} config property or by embedding git metadata at build time)",
Strings.propertyNameToEnvironmentVariableName(DD_GIT_REPOSITORY_URL),
GeneralConfig.TAGS);
}

String commitSha = gitInfo.getCommit().getSha();
if (!GitUtils.isValidCommitShaFull(commitSha)) {
log.error(
"Git commit SHA could not be resolved or is invalid: "
+ commitSha
+ " (can be provided via "
+ Strings.propertyNameToEnvironmentVariableName(DD_GIT_COMMIT_SHA)
+ " env var or corresponding system property, "
+ GeneralConfig.TAGS
+ " config property or by embedding git metadata at build time; must be a full-length SHA");
"Git commit SHA could not be resolved or is invalid: {}"
+ " (can be provided via {}"
+ " env var or corresponding system property, {}"
+ " config property or by embedding git metadata at build time; must be a full-length SHA",
commitSha,
Strings.propertyNameToEnvironmentVariableName(DD_GIT_COMMIT_SHA),
GeneralConfig.TAGS);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ private TempLocationManager() {
SEND_TELEMETRY,
"Base temp directory, as defined in '"
+ ProfilingConfig.PROFILING_TEMP_DIR
+ "' does not exist: "
+ configuredTempDir);
+ "' does not exist: {}",
configuredTempDir);
throw new IllegalStateException(
"Base temp directory, as defined in '"
+ ProfilingConfig.PROFILING_TEMP_DIR
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ private void callListenerRemoveTarget(PollingRateHinter hinter, ParsedConfigKey
listener.remove(configKey, hinter);
}
} catch (Exception ex) {
ratelimitedLogger.warn("Error handling configuration removal for " + configKey, ex);
ratelimitedLogger.warn("Error handling configuration removal for {}", configKey, ex);
}
cachedTargetFiles.remove(configKey);
configStates.remove(configKey);
Expand Down
Loading