-
Notifications
You must be signed in to change notification settings - Fork 300
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.
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
base: master
Are you sure you want to change the base?
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 67 metrics, 4 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.48.0-SNAPSHOT~342dd2bd15, baseline=1.49.0-SNAPSHOT~33fc3c9a9b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.061 s) : 0, 1061390
Total [baseline] (10.536 s) : 0, 10535529
Agent [candidate] (1.055 s) : 0, 1055172
Total [candidate] (10.492 s) : 0, 10492376
section appsec
Agent [baseline] (1.213 s) : 0, 1212893
Total [baseline] (10.821 s) : 0, 10821400
Agent [candidate] (1.196 s) : 0, 1195677
Total [candidate] (10.755 s) : 0, 10754710
section iast
Agent [baseline] (1.185 s) : 0, 1185050
Total [baseline] (11.086 s) : 0, 11086091
Agent [candidate] (1.192 s) : 0, 1192202
Total [candidate] (11.06 s) : 0, 11059502
section profiling
Agent [baseline] (1.292 s) : 0, 1292411
Total [baseline] (10.975 s) : 0, 10975455
Agent [candidate] (1.283 s) : 0, 1282975
Total [candidate] (10.927 s) : 0, 10926519
gantt
title petclinic - break down per module: candidate=1.48.0-SNAPSHOT~342dd2bd15, baseline=1.49.0-SNAPSHOT~33fc3c9a9b
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (724.311 ms) : 0, 724311
BytebuddyAgent [candidate] (720.595 ms) : 0, 720595
GlobalTracer [baseline] (240.448 ms) : 0, 240448
GlobalTracer [candidate] (240.081 ms) : 0, 240081
AppSec [baseline] (54.918 ms) : 0, 54918
AppSec [candidate] (54.436 ms) : 0, 54436
Debugger [baseline] (4.443 ms) : 0, 4443
Debugger [candidate] (4.412 ms) : 0, 4412
Remote Config [baseline] (711.8 µs) : 0, 712
Remote Config [candidate] (691.954 µs) : 0, 692
Telemetry [baseline] (15.762 ms) : 0, 15762
Telemetry [candidate] (13.734 ms) : 0, 13734
section appsec
BytebuddyAgent [baseline] (751.197 ms) : 0, 751197
BytebuddyAgent [candidate] (738.183 ms) : 0, 738183
GlobalTracer [baseline] (238.989 ms) : 0, 238989
GlobalTracer [candidate] (236.476 ms) : 0, 236476
AppSec [baseline] (177.316 ms) : 0, 177316
AppSec [candidate] (176.631 ms) : 0, 176631
Debugger [baseline] (4.294 ms) : 0, 4294
Debugger [candidate] (4.288 ms) : 0, 4288
Remote Config [baseline] (657.226 µs) : 0, 657
Remote Config [candidate] (633.774 µs) : 0, 634
Telemetry [baseline] (8.338 ms) : 0, 8338
Telemetry [candidate] (8.262 ms) : 0, 8262
IAST [baseline] (22.128 ms) : 0, 22128
IAST [candidate] (21.452 ms) : 0, 21452
section iast
BytebuddyAgent [baseline] (841.653 ms) : 0, 841653
BytebuddyAgent [candidate] (846.04 ms) : 0, 846040
GlobalTracer [baseline] (230.071 ms) : 0, 230071
GlobalTracer [candidate] (232.063 ms) : 0, 232063
AppSec [baseline] (56.399 ms) : 0, 56399
AppSec [candidate] (56.717 ms) : 0, 56717
Debugger [baseline] (4.14 ms) : 0, 4140
Debugger [candidate] (4.218 ms) : 0, 4218
Remote Config [baseline] (598.899 µs) : 0, 599
Remote Config [candidate] (638.625 µs) : 0, 639
Telemetry [baseline] (8.669 ms) : 0, 8669
Telemetry [candidate] (8.923 ms) : 0, 8923
IAST [baseline] (22.681 ms) : 0, 22681
IAST [candidate] (23.035 ms) : 0, 23035
section profiling
BytebuddyAgent [baseline] (713.392 ms) : 0, 713392
BytebuddyAgent [candidate] (711.409 ms) : 0, 711409
GlobalTracer [baseline] (359.413 ms) : 0, 359413
GlobalTracer [candidate] (352.369 ms) : 0, 352369
AppSec [baseline] (54.196 ms) : 0, 54196
AppSec [candidate] (54.226 ms) : 0, 54226
Debugger [baseline] (4.304 ms) : 0, 4304
Debugger [candidate] (4.369 ms) : 0, 4369
Remote Config [baseline] (693.445 µs) : 0, 693
Remote Config [candidate] (731.698 µs) : 0, 732
Telemetry [baseline] (8.951 ms) : 0, 8951
Telemetry [candidate] (9.147 ms) : 0, 9147
ProfilingAgent [baseline] (103.438 ms) : 0, 103438
ProfilingAgent [candidate] (105.018 ms) : 0, 105018
Profiling [baseline] (103.464 ms) : 0, 103464
Profiling [candidate] (105.044 ms) : 0, 105044
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.48.0-SNAPSHOT~342dd2bd15, baseline=1.49.0-SNAPSHOT~33fc3c9a9b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.057 s) : 0, 1056700
Total [baseline] (8.681 s) : 0, 8680858
Agent [candidate] (1.05 s) : 0, 1050061
Total [candidate] (8.68 s) : 0, 8680217
section iast
Agent [baseline] (1.198 s) : 0, 1197964
Total [baseline] (9.281 s) : 0, 9281289
Agent [candidate] (1.188 s) : 0, 1188195
Total [candidate] (9.267 s) : 0, 9267046
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.184 s) : 0, 1184454
Total [baseline] (9.238 s) : 0, 9238148
Agent [candidate] (1.183 s) : 0, 1183131
Total [candidate] (9.253 s) : 0, 9253212
section iast_TELEMETRY_OFF
Agent [baseline] (1.18 s) : 0, 1179618
Total [baseline] (9.228 s) : 0, 9228190
Agent [candidate] (1.182 s) : 0, 1182357
Total [candidate] (9.286 s) : 0, 9285781
gantt
title insecure-bank - break down per module: candidate=1.48.0-SNAPSHOT~342dd2bd15, baseline=1.49.0-SNAPSHOT~33fc3c9a9b
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (723.946 ms) : 0, 723946
BytebuddyAgent [candidate] (717.905 ms) : 0, 717905
GlobalTracer [baseline] (240.039 ms) : 0, 240039
GlobalTracer [candidate] (239.355 ms) : 0, 239355
AppSec [baseline] (54.63 ms) : 0, 54630
AppSec [candidate] (54.447 ms) : 0, 54447
Debugger [baseline] (4.438 ms) : 0, 4438
Debugger [candidate] (4.427 ms) : 0, 4427
Remote Config [baseline] (700.525 µs) : 0, 701
Remote Config [candidate] (687.616 µs) : 0, 688
Telemetry [baseline] (12.167 ms) : 0, 12167
Telemetry [candidate] (12.837 ms) : 0, 12837
section iast
BytebuddyAgent [baseline] (853.344 ms) : 0, 853344
BytebuddyAgent [candidate] (844.728 ms) : 0, 844728
GlobalTracer [baseline] (230.541 ms) : 0, 230541
GlobalTracer [candidate] (230.911 ms) : 0, 230911
AppSec [baseline] (55.85 ms) : 0, 55850
AppSec [candidate] (55.607 ms) : 0, 55607
Debugger [baseline] (4.157 ms) : 0, 4157
Debugger [candidate] (4.138 ms) : 0, 4138
Remote Config [baseline] (601.58 µs) : 0, 602
Remote Config [candidate] (643.37 µs) : 0, 643
Telemetry [baseline] (8.772 ms) : 0, 8772
Telemetry [candidate] (8.691 ms) : 0, 8691
IAST [baseline] (23.755 ms) : 0, 23755
IAST [candidate] (22.893 ms) : 0, 22893
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (841.528 ms) : 0, 841528
BytebuddyAgent [candidate] (839.403 ms) : 0, 839403
GlobalTracer [baseline] (229.932 ms) : 0, 229932
GlobalTracer [candidate] (230.594 ms) : 0, 230594
AppSec [baseline] (55.963 ms) : 0, 55963
AppSec [candidate] (56.209 ms) : 0, 56209
Debugger [baseline] (4.119 ms) : 0, 4119
Debugger [candidate] (4.174 ms) : 0, 4174
Remote Config [baseline] (599.742 µs) : 0, 600
Remote Config [candidate] (624.351 µs) : 0, 624
Telemetry [baseline] (8.706 ms) : 0, 8706
Telemetry [candidate] (8.836 ms) : 0, 8836
IAST [baseline] (22.822 ms) : 0, 22822
IAST [candidate] (22.782 ms) : 0, 22782
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (838.53 ms) : 0, 838530
BytebuddyAgent [candidate] (840.445 ms) : 0, 840445
GlobalTracer [baseline] (229.165 ms) : 0, 229165
GlobalTracer [candidate] (229.88 ms) : 0, 229880
AppSec [baseline] (55.85 ms) : 0, 55850
AppSec [candidate] (55.77 ms) : 0, 55770
Debugger [baseline] (4.059 ms) : 0, 4059
Debugger [candidate] (4.168 ms) : 0, 4168
Remote Config [baseline] (589.768 µs) : 0, 590
Remote Config [candidate] (621.742 µs) : 0, 622
Telemetry [baseline] (8.523 ms) : 0, 8523
Telemetry [candidate] (8.768 ms) : 0, 8768
IAST [baseline] (22.153 ms) : 0, 22153
IAST [candidate] (22.259 ms) : 0, 22259
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 13 metrics, 17 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.48.0-SNAPSHOT~342dd2bd15, baseline=1.49.0-SNAPSHOT~33fc3c9a9b
dateFormat X
axisFormat %s
section baseline
no_agent (381.568 µs) : 362, 401
. : milestone, 382,
iast (521.131 µs) : 499, 543
. : milestone, 521,
iast_FULL (740.64 µs) : 719, 763
. : milestone, 741,
iast_GLOBAL (564.398 µs) : 542, 586
. : milestone, 564,
iast_HARDCODED_SECRET_DISABLED (519.939 µs) : 498, 542
. : milestone, 520,
iast_INACTIVE (466.764 µs) : 445, 488
. : milestone, 467,
iast_TELEMETRY_OFF (506.341 µs) : 484, 528
. : milestone, 506,
tracing (463.237 µs) : 442, 484
. : milestone, 463,
section candidate
no_agent (383.284 µs) : 364, 403
. : milestone, 383,
iast (518.332 µs) : 497, 540
. : milestone, 518,
iast_FULL (733.531 µs) : 711, 756
. : milestone, 734,
iast_GLOBAL (566.715 µs) : 545, 589
. : milestone, 567,
iast_HARDCODED_SECRET_DISABLED (513.022 µs) : 491, 535
. : milestone, 513,
iast_INACTIVE (475.938 µs) : 454, 498
. : milestone, 476,
iast_TELEMETRY_OFF (503.164 µs) : 482, 525
. : milestone, 503,
tracing (473.057 µs) : 451, 495
. : milestone, 473,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.48.0-SNAPSHOT~342dd2bd15, baseline=1.49.0-SNAPSHOT~33fc3c9a9b
dateFormat X
axisFormat %s
section baseline
no_agent (1.36 ms) : 1339, 1381
. : milestone, 1360,
appsec (1.753 ms) : 1729, 1777
. : milestone, 1753,
appsec_no_iast (1.746 ms) : 1721, 1770
. : milestone, 1746,
code_origins (1.696 ms) : 1669, 1723
. : milestone, 1696,
iast (1.508 ms) : 1484, 1532
. : milestone, 1508,
profiling (1.544 ms) : 1519, 1568
. : milestone, 1544,
tracing (1.509 ms) : 1484, 1534
. : milestone, 1509,
section candidate
no_agent (1.37 ms) : 1350, 1390
. : milestone, 1370,
appsec (1.741 ms) : 1717, 1766
. : milestone, 1741,
appsec_no_iast (1.747 ms) : 1723, 1771
. : milestone, 1747,
code_origins (1.705 ms) : 1678, 1731
. : milestone, 1705,
iast (1.52 ms) : 1496, 1544
. : milestone, 1520,
profiling (1.515 ms) : 1492, 1538
. : milestone, 1515,
tracing (1.485 ms) : 1460, 1511
. : milestone, 1485,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.48.0-SNAPSHOT~342dd2bd15, baseline=1.49.0-SNAPSHOT~33fc3c9a9b
dateFormat X
axisFormat %s
section baseline
no_agent (15.379 s) : 15379000, 15379000
. : milestone, 15379000,
appsec (14.935 s) : 14935000, 14935000
. : milestone, 14935000,
iast (18.406 s) : 18406000, 18406000
. : milestone, 18406000,
iast_GLOBAL (18.147 s) : 18147000, 18147000
. : milestone, 18147000,
profiling (15.297 s) : 15297000, 15297000
. : milestone, 15297000,
tracing (14.86 s) : 14860000, 14860000
. : milestone, 14860000,
section candidate
no_agent (15.31 s) : 15310000, 15310000
. : milestone, 15310000,
appsec (15.012 s) : 15012000, 15012000
. : milestone, 15012000,
iast (18.326 s) : 18326000, 18326000
. : milestone, 18326000,
iast_GLOBAL (17.72 s) : 17720000, 17720000
. : milestone, 17720000,
profiling (15.142 s) : 15142000, 15142000
. : milestone, 15142000,
tracing (14.873 s) : 14873000, 14873000
. : milestone, 14873000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.48.0-SNAPSHOT~342dd2bd15, baseline=1.49.0-SNAPSHOT~33fc3c9a9b
dateFormat X
axisFormat %s
section baseline
no_agent (1.472 ms) : 1460, 1483
. : milestone, 1472,
appsec (2.367 ms) : 2320, 2414
. : milestone, 2367,
iast (2.145 ms) : 2086, 2204
. : milestone, 2145,
iast_GLOBAL (2.183 ms) : 2124, 2242
. : milestone, 2183,
profiling (1.992 ms) : 1946, 2039
. : milestone, 1992,
tracing (1.975 ms) : 1930, 2021
. : milestone, 1975,
section candidate
no_agent (1.469 ms) : 1458, 1480
. : milestone, 1469,
appsec (2.364 ms) : 2317, 2410
. : milestone, 2364,
iast (2.146 ms) : 2087, 2204
. : milestone, 2146,
iast_GLOBAL (2.192 ms) : 2133, 2251
. : milestone, 2192,
profiling (2.006 ms) : 1958, 2054
. : milestone, 2006,
tracing (1.978 ms) : 1933, 2023
. : milestone, 1978,
|
communication/src/main/java/datadog/communication/http/OkHttpUtils.java
Outdated
Show resolved
Hide resolved
communication/src/main/java/datadog/communication/http/OkHttpUtils.java
Outdated
Show resolved
Hide resolved
…or experimental tags
@@ -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()) { |
There was a problem hiding this comment.
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
public static final String EXPERIMENTAL_PROCESS_TAGS_ENABLED = | ||
"experimental.process.tags.enabled"; |
There was a problem hiding this comment.
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.
} | ||
|
||
String jbossHome = System.getProperty("jboss.home.dir", "unknown"); | ||
String jbossMode = System.getProperty("jboss.server.mode", "unknown"); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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]
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
for (Map.Entry<String, String> e : headers.entrySet()) { | ||
builder.addHeader(e.getKey(), e.getValue()); | ||
} | ||
|
||
return builder; | ||
} | ||
|
||
private static String getCustomTags() { | ||
String javaCommand = System.getProperty("sun.java.command", "unknown"); |
There was a problem hiding this comment.
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 jbossServerName = System.getProperty("jboss.server.name", "unknown"); | ||
|
||
StringBuilder customTags = new StringBuilder(); | ||
if (!"unknown".equals(javaMainClass)) { |
There was a problem hiding this comment.
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
customTags.append("jboss_server_name:").append(jbossServerName); | ||
} | ||
|
||
System.out.println(customTags); |
There was a problem hiding this comment.
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
@@ -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()) { | |||
// custom X-Datadog-Process-Tags | |||
String customTags = getCustomTags(); |
There was a problem hiding this comment.
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?
// custom X-Datadog-Process-Tags | ||
String customTags = getCustomTags(); | ||
String existingProcessTags = headers.getOrDefault("X-Datadog-Process-Tags", ""); | ||
if (!existingProcessTags.isEmpty() && !existingProcessTags.endsWith(",")) { |
There was a problem hiding this comment.
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
if (Config.get().isExperimentalProcessTagsEnabled()) { | ||
// custom X-Datadog-Process-Tags | ||
String customTags = getCustomTags(); | ||
String existingProcessTags = headers.getOrDefault("X-Datadog-Process-Tags", ""); |
There was a problem hiding this comment.
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?
for (Map.Entry<String, String> e : headers.entrySet()) { | ||
builder.addHeader(e.getKey(), e.getValue()); | ||
} | ||
|
||
return builder; | ||
} | ||
|
||
private static String getCustomTags() { |
There was a problem hiding this comment.
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
What Does This Do
Adds custom
X-Datadog-Process-Tags
HTML header to the request to the Datadog Agent. Used for testing for Service Renaming initiative.EXPERIMENTAL_PROCESS_TAGS_ENABLED=EXPERIMENTAL_PROCESS_TAGS_ENABLED
Motivation
Service renaming needs process info sent in each payload to the Datadog Agent. for now this is locked behind an experimental feature flag.
Additional Notes
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any usefull labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]