Add new parser for DD_TAGS and prioritizing DD_SERVICE
#8296
+150
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 53 metrics, 10 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.46.0-SNAPSHOT~32e9861d17, baseline=1.47.0-SNAPSHOT~312af335ab
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.044 s) : 0, 1044459
Total [baseline] (10.478 s) : 0, 10477684
Agent [candidate] (1.04 s) : 0, 1040443
Total [candidate] (10.472 s) : 0, 10471806
section appsec
Agent [baseline] (1.188 s) : 0, 1188083
Total [baseline] (10.795 s) : 0, 10794781
Agent [candidate] (1.184 s) : 0, 1184498
Total [candidate] (10.801 s) : 0, 10801031
section iast
Agent [baseline] (1.172 s) : 0, 1172425
Total [baseline] (10.944 s) : 0, 10943913
Agent [candidate] (1.168 s) : 0, 1167501
Total [candidate] (10.921 s) : 0, 10920603
section profiling
Agent [baseline] (1.264 s) : 0, 1263639
Total [baseline] (10.862 s) : 0, 10861995
Agent [candidate] (1.258 s) : 0, 1258325
Total [candidate] (10.887 s) : 0, 10887278
gantt
title petclinic - break down per module: candidate=1.46.0-SNAPSHOT~32e9861d17, baseline=1.47.0-SNAPSHOT~312af335ab
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (715.628 ms) : 0, 715628
BytebuddyAgent [candidate] (715.427 ms) : 0, 715427
GlobalTracer [baseline] (243.706 ms) : 0, 243706
GlobalTracer [candidate] (240.489 ms) : 0, 240489
AppSec [baseline] (55.105 ms) : 0, 55105
AppSec [candidate] (55.633 ms) : 0, 55633
Remote Config [baseline] (723.644 µs) : 0, 724
Remote Config [candidate] (714.425 µs) : 0, 714
Telemetry [baseline] (14.097 ms) : 0, 14097
Telemetry [candidate] (12.896 ms) : 0, 12896
section appsec
BytebuddyAgent [baseline] (735.224 ms) : 0, 735224
BytebuddyAgent [candidate] (734.648 ms) : 0, 734648
GlobalTracer [baseline] (241.521 ms) : 0, 241521
GlobalTracer [candidate] (238.012 ms) : 0, 238012
AppSec [baseline] (176.406 ms) : 0, 176406
AppSec [candidate] (176.694 ms) : 0, 176694
Remote Config [baseline] (652.099 µs) : 0, 652
Remote Config [candidate] (654.618 µs) : 0, 655
Telemetry [baseline] (8.272 ms) : 0, 8272
Telemetry [candidate] (8.303 ms) : 0, 8303
IAST [baseline] (21.565 ms) : 0, 21565
IAST [candidate] (21.83 ms) : 0, 21830
section iast
BytebuddyAgent [baseline] (834.096 ms) : 0, 834096
BytebuddyAgent [candidate] (832.146 ms) : 0, 832146
GlobalTracer [baseline] (234.685 ms) : 0, 234685
GlobalTracer [candidate] (230.973 ms) : 0, 230973
AppSec [baseline] (52.012 ms) : 0, 52012
AppSec [candidate] (55.712 ms) : 0, 55712
Remote Config [baseline] (609.613 µs) : 0, 610
Remote Config [candidate] (618.269 µs) : 0, 618
Telemetry [baseline] (8.627 ms) : 0, 8627
Telemetry [candidate] (8.755 ms) : 0, 8755
IAST [baseline] (27.147 ms) : 0, 27147
IAST [candidate] (24.032 ms) : 0, 24032
section profiling
ProfilingAgent [baseline] (95.896 ms) : 0, 95896
ProfilingAgent [candidate] (95.75 ms) : 0, 95750
BytebuddyAgent [baseline] (707.235 ms) : 0, 707235
BytebuddyAgent [candidate] (705.873 ms) : 0, 705873
GlobalTracer [baseline] (353.276 ms) : 0, 353276
GlobalTracer [candidate] (350.276 ms) : 0, 350276
AppSec [baseline] (55.377 ms) : 0, 55377
AppSec [candidate] (54.475 ms) : 0, 54475
Remote Config [baseline] (708.18 µs) : 0, 708
Remote Config [candidate] (716.314 µs) : 0, 716
Telemetry [baseline] (8.951 ms) : 0, 8951
Telemetry [candidate] (8.903 ms) : 0, 8903
Profiling [baseline] (95.923 ms) : 0, 95923
Profiling [candidate] (95.778 ms) : 0, 95778
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.46.0-SNAPSHOT~32e9861d17, baseline=1.47.0-SNAPSHOT~312af335ab
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.05 s) : 0, 1049675
Total [baseline] (8.649 s) : 0, 8649329
Agent [candidate] (1.039 s) : 0, 1039206
Total [candidate] (8.637 s) : 0, 8637247
section iast
Agent [baseline] (1.172 s) : 0, 1172201
Total [baseline] (9.239 s) : 0, 9239467
Agent [candidate] (1.17 s) : 0, 1169548
Total [candidate] (9.256 s) : 0, 9255710
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.181 s) : 0, 1181308
Total [baseline] (9.196 s) : 0, 9195948
Agent [candidate] (1.176 s) : 0, 1176311
Total [candidate] (9.219 s) : 0, 9219046
section iast_TELEMETRY_OFF
Agent [baseline] (1.171 s) : 0, 1170910
Total [baseline] (9.26 s) : 0, 9259709
Agent [candidate] (1.168 s) : 0, 1167569
Total [candidate] (9.265 s) : 0, 9265355
gantt
title insecure-bank - break down per module: candidate=1.46.0-SNAPSHOT~32e9861d17, baseline=1.47.0-SNAPSHOT~312af335ab
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (719.957 ms) : 0, 719957
BytebuddyAgent [candidate] (714.154 ms) : 0, 714154
GlobalTracer [baseline] (244.728 ms) : 0, 244728
GlobalTracer [candidate] (239.313 ms) : 0, 239313
AppSec [baseline] (55.905 ms) : 0, 55905
AppSec [candidate] (56.926 ms) : 0, 56926
Remote Config [baseline] (733.006 µs) : 0, 733
Remote Config [candidate] (709.602 µs) : 0, 710
Telemetry [baseline] (13.032 ms) : 0, 13032
Telemetry [candidate] (12.893 ms) : 0, 12893
section iast
BytebuddyAgent [baseline] (834.618 ms) : 0, 834618
BytebuddyAgent [candidate] (834.02 ms) : 0, 834020
GlobalTracer [baseline] (233.936 ms) : 0, 233936
GlobalTracer [candidate] (230.816 ms) : 0, 230816
IAST [baseline] (24.124 ms) : 0, 24124
IAST [candidate] (24.283 ms) : 0, 24283
AppSec [baseline] (54.982 ms) : 0, 54982
AppSec [candidate] (55.721 ms) : 0, 55721
Remote Config [baseline] (606.208 µs) : 0, 606
Remote Config [candidate] (628.055 µs) : 0, 628
Telemetry [baseline] (8.667 ms) : 0, 8667
Telemetry [candidate] (8.819 ms) : 0, 8819
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (841.049 ms) : 0, 841049
BytebuddyAgent [candidate] (839.493 ms) : 0, 839493
GlobalTracer [baseline] (235.578 ms) : 0, 235578
GlobalTracer [candidate] (231.83 ms) : 0, 231830
IAST [baseline] (25.985 ms) : 0, 25985
IAST [candidate] (25.165 ms) : 0, 25165
AppSec [baseline] (54.061 ms) : 0, 54061
AppSec [candidate] (54.921 ms) : 0, 54921
Remote Config [baseline] (618.037 µs) : 0, 618
Remote Config [candidate] (626.928 µs) : 0, 627
Telemetry [baseline] (8.695 ms) : 0, 8695
Telemetry [candidate] (8.77 ms) : 0, 8770
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (832.525 ms) : 0, 832525
BytebuddyAgent [candidate] (832.373 ms) : 0, 832373
GlobalTracer [baseline] (234.657 ms) : 0, 234657
GlobalTracer [candidate] (231.331 ms) : 0, 231331
IAST [baseline] (25.318 ms) : 0, 25318
IAST [candidate] (26.086 ms) : 0, 26086
AppSec [baseline] (53.979 ms) : 0, 53979
AppSec [candidate] (53.212 ms) : 0, 53212
Remote Config [baseline] (620.415 µs) : 0, 620
Remote Config [candidate] (631.042 µs) : 0, 631
Telemetry [baseline] (8.613 ms) : 0, 8613
Telemetry [candidate] (8.672 ms) : 0, 8672
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 17 unstable metrics. Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.46.0-SNAPSHOT~32e9861d17, baseline=1.47.0-SNAPSHOT~312af335ab
dateFormat X
axisFormat %s
section baseline
no_agent (1.377 ms) : 1357, 1398
. : milestone, 1377,
appsec (1.754 ms) : 1730, 1777
. : milestone, 1754,
appsec_no_iast (1.749 ms) : 1726, 1772
. : milestone, 1749,
iast (1.548 ms) : 1524, 1573
. : milestone, 1548,
profiling (1.528 ms) : 1503, 1552
. : milestone, 1528,
tracing (1.494 ms) : 1469, 1519
. : milestone, 1494,
section candidate
no_agent (1.348 ms) : 1329, 1367
. : milestone, 1348,
appsec (1.748 ms) : 1724, 1772
. : milestone, 1748,
appsec_no_iast (1.748 ms) : 1722, 1773
. : milestone, 1748,
iast (1.527 ms) : 1504, 1551
. : milestone, 1527,
profiling (1.569 ms) : 1544, 1594
. : milestone, 1569,
tracing (1.521 ms) : 1497, 1545
. : milestone, 1521,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.46.0-SNAPSHOT~32e9861d17, baseline=1.47.0-SNAPSHOT~312af335ab
dateFormat X
axisFormat %s
section baseline
no_agent (383.748 µs) : 364, 403
. : milestone, 384,
iast (516.652 µs) : 495, 539
. : milestone, 517,
iast_FULL (753.274 µs) : 731, 775
. : milestone, 753,
iast_GLOBAL (565.597 µs) : 543, 588
. : milestone, 566,
iast_HARDCODED_SECRET_DISABLED (514.077 µs) : 492, 536
. : milestone, 514,
iast_INACTIVE (472.52 µs) : 450, 495
. : milestone, 473,
iast_TELEMETRY_OFF (501.38 µs) : 479, 524
. : milestone, 501,
tracing (462.398 µs) : 441, 484
. : milestone, 462,
section candidate
no_agent (387.748 µs) : 368, 407
. : milestone, 388,
iast (522.271 µs) : 499, 545
. : milestone, 522,
iast_FULL (750.044 µs) : 727, 773
. : milestone, 750,
iast_GLOBAL (564.534 µs) : 542, 587
. : milestone, 565,
iast_HARDCODED_SECRET_DISABLED (517.204 µs) : 495, 539
. : milestone, 517,
iast_INACTIVE (467.369 µs) : 446, 489
. : milestone, 467,
iast_TELEMETRY_OFF (507.883 µs) : 484, 531
. : milestone, 508,
tracing (467.058 µs) : 445, 489
. : milestone, 467,
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.46.0-SNAPSHOT~32e9861d17, baseline=1.47.0-SNAPSHOT~312af335ab
dateFormat X
axisFormat %s
section baseline
no_agent (15.357 s) : 15357000, 15357000
. : milestone, 15357000,
appsec (14.875 s) : 14875000, 14875000
. : milestone, 14875000,
iast (18.327 s) : 18327000, 18327000
. : milestone, 18327000,
iast_GLOBAL (17.907 s) : 17907000, 17907000
. : milestone, 17907000,
profiling (15.017 s) : 15017000, 15017000
. : milestone, 15017000,
tracing (15.194 s) : 15194000, 15194000
. : milestone, 15194000,
section candidate
no_agent (14.72 s) : 14720000, 14720000
. : milestone, 14720000,
appsec (15.069 s) : 15069000, 15069000
. : milestone, 15069000,
iast (18.916 s) : 18916000, 18916000
. : milestone, 18916000,
iast_GLOBAL (18.215 s) : 18215000, 18215000
. : milestone, 18215000,
profiling (15.088 s) : 15088000, 15088000
. : milestone, 15088000,
tracing (15.037 s) : 15037000, 15037000
. : milestone, 15037000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.46.0-SNAPSHOT~32e9861d17, baseline=1.47.0-SNAPSHOT~312af335ab
dateFormat X
axisFormat %s
section baseline
no_agent (1.474 ms) : 1462, 1486
. : milestone, 1474,
appsec (2.36 ms) : 2316, 2403
. : milestone, 2360,
iast (2.116 ms) : 2061, 2171
. : milestone, 2116,
iast_GLOBAL (2.15 ms) : 2095, 2205
. : milestone, 2150,
profiling (1.962 ms) : 1918, 2005
. : milestone, 1962,
tracing (1.947 ms) : 1905, 1989
. : milestone, 1947,
section candidate
no_agent (1.471 ms) : 1459, 1482
. : milestone, 1471,
appsec (2.362 ms) : 2318, 2405
. : milestone, 2362,
iast (2.113 ms) : 2058, 2168
. : milestone, 2113,
iast_GLOBAL (2.147 ms) : 2092, 2202
. : milestone, 2147,
profiling (1.964 ms) : 1921, 2007
. : milestone, 1964,
tracing (1.953 ms) : 1911, 1995
. : milestone, 1953,
|
mhlidd
changed the title
DD_TAGS and prioritizing DD_SERVICE
mhlidd
force-pushed
the
mhlidd/dd_tags
branch
from
50cad0a to
5dbe976
Compare
amarziali
reviewed
Feb 3, 2025
amarziali
reviewed
Feb 3, 2025
| @@ -68,7 +68,12 @@ static List<String> parseList(final String str, final String separator) { | |||
|
|
|||
| @Nonnull | |||
| static Map<String, String> parseMap(final String str, final String settingName) { | |||
| return parseMap(str, settingName, ':'); | |||
| if (settingName.equals( | |||
| "trace.tags")) { // if we are parsing dd.tags, use the tags specific parser | |||
There was a problem hiding this comment.
I think putting tag name conditional logic in this method is not durable. Instead, what about just calling parseTraceTagsMap from the config class where those tags are extracted?
There was a problem hiding this comment.
This would involve creating a new getMergedMap to let trace.tags take precedence over tags. This seems a little repetitive to me, but I agree that the conditional logic used currently isn't the best. I'm wondering if there is an alternate solution for this? WDYT?
amarziali
reviewed
Feb 3, 2025
| argSeparatorInd = str.indexOf(sep); | ||
| if (argSeparatorInd != -1) { | ||
| argSeparator = sep; | ||
| break; |
There was a problem hiding this comment.
If I understood well the code I think you should retain the separator that comes first (the smaller indexof) and you should'n break here. For instance if you have mykeys abc,d the separator used is space but if the method is called with argSeparator = [',', ' '] This code will detect the comma as the separator while it does not look so.
There was a problem hiding this comment.
The intended behavior is that if a comma exists at all within the input, we would want to split by comma, regardless of which separator comes first. That is why we iterate through an ordered list of separators that are ordered by priority of which separator we'd rather use.
There was a problem hiding this comment.
it looks confusing since the space is allowed and nothing prevent to a value to contain a comma
There was a problem hiding this comment.
Hmm I'm not sure what you mean here. This loop here is solely to determine whether we want to separate by comma or separate by space. When we iterate through the input, we take each substring (separated by the argSeparator) and then check for K/V then, meaning that the potential value would stop at the argSeparator. Let me know if that makes sense or if you have more questions!
There was a problem hiding this comment.
If you run that code like this: loadTraceTagsMap(map, "k1=v1 k2=v2,v3", '=', List.of(',', ' ')); the result is {k1=v1 k2=v2, v3=}
The separator used is , but the first one would have been a space.
The comment in the code says: // find the first instance of the first possible separator.
I perhaps misunderstood, but the first possible would have been a space and not a comma.
amarziali
reviewed
Feb 3, 2025
internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy
Outdated
Show resolved
Hide resolved
amarziali
reviewed
Feb 3, 2025
internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy
Show resolved
Hide resolved
amarziali
reviewed
Feb 7, 2025
| private static void loadTraceTagsMap( | ||
| Map<String, String> map, | ||
| String str, | ||
| String settingName, |
There was a problem hiding this comment.
that argument does not look used in that method
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Does This Do
Creates a new parser to parse
DD_TAGSby prioritizing comma separated K/V pairs, and falling back to space separated K/V pairs. See this reference doc for how it is already implemented in dd-trace-go.Additionally, if both
DD_SERVICEand theservicekey inDD_TAGSare set, this PR prioritizesDD_SERVICEas the service name. This effectively is the same as not settingserviceinDD_TAGS. However, if a customer still wanted to manually override the service name withsetTag, the service name would still be updated by theTagInterceptor.Motivation
Our goal is to make the implementation of configuration variables consistent for all languages as part of the config consistency effort listed in the following RFC.
Additional Notes
Tested with additional and existing unit tests, as well as parametric and weblog tests.
Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: APMAPI-1039