Skip to content

Add to dd_tags before send traces to agent #8653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Add to dd_tags before send traces to agent #8653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dbabe9a
first commit test add to dd_tags before send traces to agent
jordan-wong Apr 1, 2025
a358fb3
Merge branch 'master' into Service-renaming-POC-tags
jordan-wong Apr 1, 2025
28e0568
add info to X-Datadog-Process-Tags
jordan-wong Apr 2, 2025
4635bef
change tag names, add tags only if they are known, add feature flag f…
jordan-wong Apr 4, 2025
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
80 changes: 80 additions & 0 deletions communication/src/main/java/datadog/communication/http/OkHttpUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.Proxy;
import java.net.URL;
import java.nio.ByteBuffer;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -209,13 +210,92 @@ public static Request.Builder prepareRequest(final HttpUrl url, Map<String, Stri
builder.addHeader(DATADOG_ENTITY_ID, entityId);
}

if (Config.get().isExperimentalProcessTagsEnabled()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a feature flag as requested @raphaelgavache

// custom X-Datadog-Process-Tags
String customTags = getCustomTags();
Copy link
Collaborator

Choose a reason for hiding this comment

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

prepareRequest is called each time something needs to be sent. It does not sound that the tags are changing since they will be immutable within a process. Can that computation be done once and reused?

String existingProcessTags = headers.getOrDefault("X-Datadog-Process-Tags", "");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have chances to already have a value for this? In this case should we just override what we calculated?

if (!existingProcessTags.isEmpty() && !existingProcessTags.endsWith(",")) {
Copy link
Collaborator

@amarziali amarziali Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

the second condition !endWith look always true if the first is true so it can be removed

existingProcessTags += ",";
}
if (customTags.length() > 0) {
headers.put("X-Datadog-Process-Tags", existingProcessTags + customTags);
}
}

for (Map.Entry<String, String> e : headers.entrySet()) {
builder.addHeader(e.getKey(), e.getValue());
}

return builder;
}

private static String getCustomTags() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This method heavily uses System.getProperty. In case a SecurityManager is used, this can throw a security exception if the access to them is not granted (see https://docs.oracle.com/javase/8/docs/api/java/lang/System.html#getProperty-java.lang.String-) . I can suggest to wrap that in a try-catch Throwable to avoid it to be thrown up in the call stack

String javaCommand = System.getProperty("sun.java.command", "unknown");
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no need to have a unknown default when the return value will be null if the property is missing

String[] parts = javaCommand.split(" ");
String mainTarget = parts.length > 0 ? parts[0] : "unknown";
System.out.println("javacommand is: " + javaCommand);
boolean isJar = mainTarget.endsWith(".jar");
String javaMainClass = isJar ? "unknown" : mainTarget;
String javaJarFile = "unknown";
String javaJarPath = "unknown";

if (isJar) {
javaJarFile = mainTarget;
try {
// Load it as a resource
URL jarUrl = new File(javaJarFile).toURI().toURL(); // Assumes relative path
File jarFile = new File(jarUrl.toURI());

if (jarFile.exists()) {
javaJarPath = jarFile.getAbsolutePath();
}
} catch (Exception e) {
System.out.println("Unable to get JAR file: " + e.getMessage());
}
} else {
try {
Class<?> mainClass = Class.forName(javaMainClass);
javaJarPath =
mainClass.getProtectionDomain().getCodeSource().getLocation().toURI().getPath();
} catch (Exception e) {
System.out.println("Unable to load main class or get JAR file: " + e.getMessage());
}
}

String jbossHome = System.getProperty("jboss.home.dir", "unknown");
String jbossMode = System.getProperty("jboss.server.mode", "unknown");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which jboss edition or version is setting this system property?

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'm testing with this sample app which is running quay.io/wildfly/wildfly:26.1.2.Final-jdk17

Is there a reason you ask this?

Copy link
Collaborator

@amarziali amarziali Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Usually it sets -D[Standalone] when running in standalone mode otherwise for domain mode you have something like -D[Server:server-one]

String jbossServerName = System.getProperty("jboss.server.name", "unknown");

StringBuilder customTags = new StringBuilder();
if (!"unknown".equals(javaMainClass)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if (javaMainClass != null) is a better choice here

if (customTags.length() > 0) customTags.append(",");
customTags.append("java_main_class:").append(javaMainClass);
}
if (!"unknown".equals(javaJarFile)) {
if (customTags.length() > 0) customTags.append(",");
customTags.append("java_jar_file:").append(javaJarFile);
}
if (!"unknown".equals(javaJarPath)) {
if (customTags.length() > 0) customTags.append(",");
customTags.append("java_jar_path:").append(javaJarPath);
}
if (!"unknown".equals(jbossHome)) {
if (customTags.length() > 0) customTags.append(",");
customTags.append("jboss_home:").append(jbossHome);
}
if (!"unknown".equals(jbossMode)) {
if (customTags.length() > 0) customTags.append(",");
customTags.append("jboss_mode:").append(jbossMode);
}
if (!"unknown".equals(jbossServerName)) {
if (customTags.length() > 0) customTags.append(",");
customTags.append("jboss_server_name:").append(jbossServerName);
}

System.out.println(customTags);
Copy link
Collaborator

@amarziali amarziali Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

those println sounds like leftover debug messages. If there is the need to keep diagnostic, they should be logged as debug otherwise they could be removed

return customTags.toString();
}

public static Request.Builder prepareRequest(
final HttpUrl url,
final Map<String, String> headers,
Expand Down
2 changes: 2 additions & 0 deletions dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ public final class GeneralConfig {
public static final String AGENTLESS_LOG_SUBMISSION_URL = "agentless.log.submission.url";
public static final String APM_TRACING_ENABLED = "apm.tracing.enabled";
public static final String JDK_SOCKET_ENABLED = "jdk.socket.enabled";
public static final String EXPERIMENTAL_PROCESS_TAGS_ENABLED =
"experimental.process.tags.enabled";
Comment on lines +98 to +99
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a case where we could take advantage of DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED(ref). The goal moving forward is the minimize overall configs that we want to add, and DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED is meant to be used as a catch-all for feature flags as much as possible.

@amarziali WDYT? We had a conversation of how this might be useful a while back. Wanted to see if you agree in this use case.


private GeneralConfig() {}
}
2 changes: 2 additions & 0 deletions dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ public Response sendSerializedTraces(final Payload payload) {
: "")
.put(payload.toRequest())
.build();
System.out.println("HELLO REQUEST SENT IS THIS:");
System.out.println(request.headers());
this.totalTraces += payload.traceCount();
this.receivedTraces += payload.traceCount();
try (final Recording recording = sendPayloadTimer.start();
Expand Down
8 changes: 8 additions & 0 deletions internal-api/src/main/java/datadog/trace/api/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ public static String getHostName() {
private final Set<String> experimentalFeaturesEnabled;

private final boolean jdkSocketEnabled;
private final boolean experimentalProcessTagsEnabled;

// Read order: System Properties -> Env Variables, [-> properties file], [-> default value]
private Config() {
Expand Down Expand Up @@ -2011,6 +2012,9 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment())

this.jdkSocketEnabled = configProvider.getBoolean(JDK_SOCKET_ENABLED, true);

this.experimentalProcessTagsEnabled =
configProvider.getBoolean(EXPERIMENTAL_PROCESS_TAGS_ENABLED, false);

log.debug("New instance: {}", this);
}

Expand Down Expand Up @@ -3617,6 +3621,10 @@ public boolean isJdkSocketEnabled() {
return jdkSocketEnabled;
}

public boolean isExperimentalProcessTagsEnabled() {
return experimentalProcessTagsEnabled;
}

/** @return A map of tags to be applied only to the local application root span. */
public Map<String, Object> getLocalRootSpanTags() {
final Map<String, String> runtimeTags = getRuntimeTags();
Expand Down
Loading