Add peer tags, span kind and trace root flag to MetricKey bucket #9178
Conversation
This file contains hidden or 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 47 metrics, 12 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.53.0-SNAPSHOT~318a10eb4d, baseline=1.53.0-SNAPSHOT~a3836e611d
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.042 s) : 0, 1041604
Total [baseline] (8.544 s) : 0, 8543794
Agent [candidate] (1.044 s) : 0, 1043933
Total [candidate] (8.571 s) : 0, 8570735
section iast
Agent [baseline] (1.173 s) : 0, 1173219
Total [baseline] (9.331 s) : 0, 9331491
Agent [candidate] (1.179 s) : 0, 1178656
Total [candidate] (9.31 s) : 0, 9310282
gantt
title insecure-bank - break down per module: candidate=1.53.0-SNAPSHOT~318a10eb4d, baseline=1.53.0-SNAPSHOT~a3836e611d
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.418 ms) : 0, 1418
crashtracking [candidate] (1.428 ms) : 0, 1428
BytebuddyAgent [baseline] (729.162 ms) : 0, 729162
BytebuddyAgent [candidate] (732.349 ms) : 0, 732349
GlobalTracer [baseline] (241.03 ms) : 0, 241030
GlobalTracer [candidate] (241.898 ms) : 0, 241898
AppSec [baseline] (29.868 ms) : 0, 29868
AppSec [candidate] (30.028 ms) : 0, 30028
Debugger [baseline] (5.976 ms) : 0, 5976
Debugger [candidate] (6.018 ms) : 0, 6018
Remote Config [baseline] (643.299 µs) : 0, 643
Remote Config [candidate] (642.534 µs) : 0, 643
Telemetry [baseline] (12.652 ms) : 0, 12652
Telemetry [candidate] (10.647 ms) : 0, 10647
section iast
crashtracking [baseline] (1.428 ms) : 0, 1428
crashtracking [candidate] (1.417 ms) : 0, 1417
BytebuddyAgent [baseline] (847.607 ms) : 0, 847607
BytebuddyAgent [candidate] (850.617 ms) : 0, 850617
GlobalTracer [baseline] (231.955 ms) : 0, 231955
GlobalTracer [candidate] (233.164 ms) : 0, 233164
IAST [baseline] (31.357 ms) : 0, 31357
IAST [candidate] (28.402 ms) : 0, 28402
AppSec [baseline] (25.535 ms) : 0, 25535
AppSec [candidate] (26.876 ms) : 0, 26876
Debugger [baseline] (5.716 ms) : 0, 5716
Debugger [candidate] (8.389 ms) : 0, 8389
Remote Config [baseline] (574.877 µs) : 0, 575
Remote Config [candidate] (591.266 µs) : 0, 591
Telemetry [baseline] (8.219 ms) : 0, 8219
Telemetry [candidate] (8.319 ms) : 0, 8319
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.53.0-SNAPSHOT~318a10eb4d, baseline=1.53.0-SNAPSHOT~a3836e611d
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.043 s) : 0, 1043416
Total [baseline] (10.687 s) : 0, 10687208
Agent [candidate] (1.053 s) : 0, 1053026
Total [candidate] (10.787 s) : 0, 10787381
section appsec
Agent [baseline] (1.227 s) : 0, 1227055
Total [baseline] (10.828 s) : 0, 10827506
Agent [candidate] (1.219 s) : 0, 1218574
Total [candidate] (10.813 s) : 0, 10812808
section iast
Agent [baseline] (1.174 s) : 0, 1173732
Total [baseline] (10.859 s) : 0, 10858611
Agent [candidate] (1.177 s) : 0, 1177006
Total [candidate] (10.991 s) : 0, 10991032
section profiling
Agent [baseline] (1.193 s) : 0, 1192616
Total [baseline] (10.839 s) : 0, 10839489
Agent [candidate] (1.2 s) : 0, 1200485
Total [candidate] (10.862 s) : 0, 10861915
gantt
title petclinic - break down per module: candidate=1.53.0-SNAPSHOT~318a10eb4d, baseline=1.53.0-SNAPSHOT~a3836e611d
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.433 ms) : 0, 1433
crashtracking [candidate] (1.441 ms) : 0, 1441
BytebuddyAgent [baseline] (730.967 ms) : 0, 730967
BytebuddyAgent [candidate] (737.014 ms) : 0, 737014
GlobalTracer [baseline] (241.444 ms) : 0, 241444
GlobalTracer [candidate] (243.943 ms) : 0, 243943
AppSec [baseline] (29.93 ms) : 0, 29930
AppSec [candidate] (30.574 ms) : 0, 30574
Debugger [baseline] (6.004 ms) : 0, 6004
Debugger [candidate] (6.098 ms) : 0, 6098
Remote Config [baseline] (648.761 µs) : 0, 649
Remote Config [candidate] (663.014 µs) : 0, 663
Telemetry [baseline] (12.065 ms) : 0, 12065
Telemetry [candidate] (12.18 ms) : 0, 12180
section appsec
crashtracking [baseline] (1.444 ms) : 0, 1444
crashtracking [candidate] (1.427 ms) : 0, 1427
BytebuddyAgent [baseline] (758.306 ms) : 0, 758306
BytebuddyAgent [candidate] (753.051 ms) : 0, 753051
GlobalTracer [baseline] (236.305 ms) : 0, 236305
GlobalTracer [candidate] (234.642 ms) : 0, 234642
IAST [baseline] (23.726 ms) : 0, 23726
IAST [candidate] (23.411 ms) : 0, 23411
AppSec [baseline] (169.026 ms) : 0, 169026
AppSec [candidate] (168.835 ms) : 0, 168835
Debugger [baseline] (8.032 ms) : 0, 8032
Debugger [candidate] (7.221 ms) : 0, 7221
Remote Config [baseline] (635.005 µs) : 0, 635
Remote Config [candidate] (628.911 µs) : 0, 629
Telemetry [baseline] (8.413 ms) : 0, 8413
Telemetry [candidate] (8.348 ms) : 0, 8348
section iast
crashtracking [baseline] (1.426 ms) : 0, 1426
crashtracking [candidate] (1.423 ms) : 0, 1423
BytebuddyAgent [baseline] (846.951 ms) : 0, 846951
BytebuddyAgent [candidate] (849.211 ms) : 0, 849211
GlobalTracer [baseline] (232.438 ms) : 0, 232438
GlobalTracer [candidate] (233.182 ms) : 0, 233182
IAST [baseline] (28.41 ms) : 0, 28410
IAST [candidate] (29.277 ms) : 0, 29277
AppSec [baseline] (27.293 ms) : 0, 27293
AppSec [candidate] (28.261 ms) : 0, 28261
Debugger [baseline] (7.433 ms) : 0, 7433
Debugger [candidate] (5.782 ms) : 0, 5782
Remote Config [baseline] (582.147 µs) : 0, 582
Remote Config [candidate] (578.341 µs) : 0, 578
Telemetry [baseline] (8.249 ms) : 0, 8249
Telemetry [candidate] (8.288 ms) : 0, 8288
section profiling
crashtracking [baseline] (1.39 ms) : 0, 1390
crashtracking [candidate] (1.423 ms) : 0, 1423
BytebuddyAgent [baseline] (759.639 ms) : 0, 759639
BytebuddyAgent [candidate] (766.174 ms) : 0, 766174
GlobalTracer [baseline] (221.912 ms) : 0, 221912
GlobalTracer [candidate] (222.59 ms) : 0, 222590
AppSec [baseline] (29.867 ms) : 0, 29867
AppSec [candidate] (30.187 ms) : 0, 30187
Debugger [baseline] (6.239 ms) : 0, 6239
Debugger [candidate] (6.319 ms) : 0, 6319
Remote Config [baseline] (710.076 µs) : 0, 710
Remote Config [candidate] (672.998 µs) : 0, 673
Telemetry [baseline] (16.183 ms) : 0, 16183
Telemetry [candidate] (15.313 ms) : 0, 15313
ProfilingAgent [baseline] (107.231 ms) : 0, 107231
ProfilingAgent [candidate] (107.982 ms) : 0, 107982
Profiling [baseline] (107.874 ms) : 0, 107874
Profiling [candidate] (108.62 ms) : 0, 108620
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 2 performance regressions! Performance is the same for 9 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.53.0-SNAPSHOT~318a10eb4d, baseline=1.53.0-SNAPSHOT~a3836e611d
dateFormat X
axisFormat %s
section baseline
no_agent (4.404 ms) : 4354, 4454
. : milestone, 4404,
iast (9.341 ms) : 9183, 9499
. : milestone, 9341,
iast_FULL (13.827 ms) : 13556, 14098
. : milestone, 13827,
iast_GLOBAL (10.343 ms) : 10160, 10526
. : milestone, 10343,
profiling (8.446 ms) : 8306, 8587
. : milestone, 8446,
tracing (7.626 ms) : 7513, 7739
. : milestone, 7626,
section candidate
no_agent (4.382 ms) : 4332, 4432
. : milestone, 4382,
iast (9.279 ms) : 9128, 9429
. : milestone, 9279,
iast_FULL (13.657 ms) : 13388, 13925
. : milestone, 13657,
iast_GLOBAL (10.686 ms) : 10496, 10876
. : milestone, 10686,
profiling (8.589 ms) : 8460, 8718
. : milestone, 8589,
tracing (7.455 ms) : 7351, 7559
. : milestone, 7455,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.53.0-SNAPSHOT~318a10eb4d, baseline=1.53.0-SNAPSHOT~a3836e611d
dateFormat X
axisFormat %s
section baseline
no_agent (35.424 ms) : 35144, 35704
. : milestone, 35424,
appsec (47.082 ms) : 46662, 47501
. : milestone, 47082,
code_origins (45.423 ms) : 45034, 45813
. : milestone, 45423,
iast (45.764 ms) : 45367, 46160
. : milestone, 45764,
profiling (48.515 ms) : 48039, 48992
. : milestone, 48515,
tracing (43.28 ms) : 42908, 43653
. : milestone, 43280,
section candidate
no_agent (37.63 ms) : 37330, 37931
. : milestone, 37630,
appsec (47.075 ms) : 46653, 47496
. : milestone, 47075,
code_origins (44.295 ms) : 43925, 44665
. : milestone, 44295,
iast (46.012 ms) : 45624, 46401
. : milestone, 46012,
profiling (47.006 ms) : 46549, 47464
. : milestone, 47006,
tracing (45.483 ms) : 45094, 45873
. : milestone, 45483,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 2 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.53.0-SNAPSHOT~318a10eb4d, baseline=1.53.0-SNAPSHOT~a3836e611d
dateFormat X
axisFormat %s
section baseline
no_agent (15.009 s) : 15009000, 15009000
. : milestone, 15009000,
appsec (15.051 s) : 15051000, 15051000
. : milestone, 15051000,
iast (18.95 s) : 18950000, 18950000
. : milestone, 18950000,
iast_GLOBAL (18.147 s) : 18147000, 18147000
. : milestone, 18147000,
profiling (15.146 s) : 15146000, 15146000
. : milestone, 15146000,
tracing (14.997 s) : 14997000, 14997000
. : milestone, 14997000,
section candidate
no_agent (14.928 s) : 14928000, 14928000
. : milestone, 14928000,
appsec (14.813 s) : 14813000, 14813000
. : milestone, 14813000,
iast (18.532 s) : 18532000, 18532000
. : milestone, 18532000,
iast_GLOBAL (17.934 s) : 17934000, 17934000
. : milestone, 17934000,
profiling (15.211 s) : 15211000, 15211000
. : milestone, 15211000,
tracing (15.205 s) : 15205000, 15205000
. : milestone, 15205000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.53.0-SNAPSHOT~318a10eb4d, baseline=1.53.0-SNAPSHOT~a3836e611d
dateFormat X
axisFormat %s
section baseline
no_agent (1.477 ms) : 1465, 1488
. : milestone, 1477,
appsec (3.675 ms) : 3453, 3896
. : milestone, 3675,
iast (2.193 ms) : 2130, 2255
. : milestone, 2193,
iast_GLOBAL (2.245 ms) : 2182, 2308
. : milestone, 2245,
profiling (2.481 ms) : 2313, 2649
. : milestone, 2481,
tracing (2.016 ms) : 1968, 2065
. : milestone, 2016,
section candidate
no_agent (1.474 ms) : 1463, 1486
. : milestone, 1474,
appsec (3.65 ms) : 3433, 3867
. : milestone, 3650,
iast (2.199 ms) : 2136, 2262
. : milestone, 2199,
iast_GLOBAL (2.245 ms) : 2182, 2309
. : milestone, 2245,
profiling (2.068 ms) : 2016, 2120
. : milestone, 2068,
tracing (2.024 ms) : 1975, 2073
. : milestone, 2024,
|
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
8c4a17b to
3e3c914
Compare
bric3
commented
Jul 16, 2025
Comment on lines
-115
to
-117
| 1 * writer.add(new MetricKey("resource", "service", "operation", "type", HTTP_OK, false), _) >> { MetricKey key, AggregateMetric value -> | ||
| value.getHitCount() == 1 && value.getTopLevelCount() == 1 && value.getDuration() == 100 | ||
| } |
There was a problem hiding this comment.
1 * writer.add(…, _) >> { MetricKey key, AggregateMetric value -> some assertions }FYI, these spock code are incorrect, it tries to verify the AggregateMetric, but actually nothing is properly verified, because >> indicates a stub, which is incorrect since it appears in the then section with a verification.
Instead, the verification code should be written this way, with an argument constraint :
1 * writer.add(…, { AggregateMetric value -> some assertions })
amarziali
reviewed
Jul 16, 2025
| for (String peerTag : features.peerTags()) { | ||
| Object value = span.getTag(peerTag); | ||
| if (value != null) { | ||
| peerTags.add(peerTag + ":" + TraceUtils.normalizeTag(value.toString())); |
There was a problem hiding this comment.
I would avoid creating a lot of new strings here. Instead we can return a map view of the tracer tags (a view returning only entries whose key is in peerTags) and write directly key then : then normalize(value) on the messagepack writer. It will avoid create a string here.
There was a problem hiding this comment.
A simplified version is just to hold key, value here (so that we can use them for the hashtag) but not concat the key:value
There was a problem hiding this comment.
Thanks, we're thinking alike!
I had the same thought about String creation. I tried a StringBuilder approach, but without notable benefit in the end on the serialization side, especially since UTF8BytesString actually creates a full String anyway.
The issue with keeping a map view, is it requires to create UTF8BytesString each time this key is serialized. I'll try to come up with a solution that balances both.
There was a problem hiding this comment.
Yes as an idea that can split in a couple of things.
First the need not to concatenate strings. For this you can just store the key in a list of UTF8ByteString (since they peerTags are quite stable they can be converted once anyway) and value in of List<String> and in SerializingMetricWriter just write first the key then : than the normalised value.
I don't think that the values can be efficiently cached since we don't know the cardinality in advance. Converting in UTF8 each time should be fine for now
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
3e3c914 to
48a0722
Compare
amarziali
reviewed
Jul 17, 2025
| StringBuilder peerTagBuilder = new StringBuilder(); | ||
| for (Map.Entry<String, String> peerTag : peerTags.entrySet()) { | ||
| peerTagBuilder.setLength(0); | ||
| String toWrite = peerTagBuilder |
There was a problem hiding this comment.
I'm wondering if we can do something to cache them with a DDCache (like 1024 entries assuming that it will be enough for peer tags)
There was a problem hiding this comment.
Indeed, that's something on my mind, but there some challenges there that I'd rather work in another smaller PR.
amarziali
approved these changes
Jul 17, 2025
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
48a0722 to
2d089ec
Compare
6 tasks
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
2d089ec to
696ab1c
Compare
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
3 times, most recently
from
b9006af to
b7aa90d
Compare
bric3
commented
Jul 30, 2025
dd-trace-core/src/test/groovy/datadog/trace/common/metrics/FootprintForkedTest.groovy
Outdated
Show resolved
Hide resolved
|
System tests are failing, need to wait on DataDog/system-tests#5002 |
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
c394992 to
b53a739
Compare
|
Code coverage: total 59.03%, base diff 1.72%, patch 100.00% (view details) This comment will be updated automatically if new data arrives.🔗 Commit SHA: 318a10e | Docs | Was this helpful? Give us feedback! |
amarziali
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
a5d58e2 to
9154951
Compare
… bucket # Conflicts: # dd-trace-core/src/test/groovy/datadog/trace/common/metrics/FootprintForkedTest.groovy
Also delays the creation of the Utf8ByteString at serialization time. Note that `writer.writeUTF8` emits a header corresponding to the length of the value being written, calling this method again will count as an entry in the array. A possible idea was to use a cache to store the computation of the peerTags, however `Utf8ByteString` is not concatenable / appendable, which is necessary to have the proper encoding. Creating a new "temporary" `Utf8ByteString`, was replaced by a direct call to `String: :getBytes`.
* Add jmh for metrics aggregation * Cache peer tags to avoid too many strings/utf8 conversions * Use span kind cache * Fix tests
amarziali
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
9154951 to
318a10e
Compare
PerfectSlayer
approved these changes
Aug 12, 2025
Comment on lines
+65
to
+77
| private static final DDCache< | ||
| String, Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>> | ||
| PEER_TAGS_CACHE = | ||
| DDCaches.newFixedSizeCache( | ||
| 64); // it can be unbounded since those values are returned by the agent and should be | ||
| // under control. 64 entries is enough in this case to contain all the peer tags. | ||
| private static final Function< | ||
| String, Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>> | ||
| PEER_TAGS_CACHE_ADDER = | ||
| key -> | ||
| Pair.of( | ||
| DDCaches.newFixedSizeCache(512), | ||
| value -> UTF8BytesString.create(key + ":" + value)); |
There was a problem hiding this comment.
🎯 suggestion: Can't we make a dedicated type for the cache and creator rather than using a Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>? It feels hard to read.
Because it always end up calling cache.computeIfAbsent(key, creator), so the whole Pair<...> thing can be simplified as a functional interface like: Function<String, UTF8BytesString>.
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
This fills the gap regarding peer tags, by declaring the in the
MetricKey.Motivation
Additional Notes
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: [PROJ-IDENT]