-
Notifications
You must be signed in to change notification settings - Fork 322
Inferred Routes and Resource Renaming: new APM trace metrics tag #9553
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
|
Hi! 👋 Looks like you updated a Git Submodule.
|
746bf88 to
b3676e3
Compare
|
Hi! 👋 Looks like you updated a Git Submodule.
|
|
🎯 Code Coverage 🔗 Commit SHA: f10792d | Docs | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 58 metrics, 7 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.54.0-SNAPSHOT~f10792d75b, baseline=1.55.0-SNAPSHOT~47f955b5dc
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.016 s) : 0, 1016188
Total [baseline] (10.615 s) : 0, 10615069
Agent [candidate] (1.015 s) : 0, 1014710
Total [candidate] (10.668 s) : 0, 10667677
section appsec
Agent [baseline] (1.194 s) : 0, 1193540
Total [baseline] (11.031 s) : 0, 11031434
Agent [candidate] (1.195 s) : 0, 1194630
Total [candidate] (11.054 s) : 0, 11053514
section iast
Agent [baseline] (1.168 s) : 0, 1167731
Total [baseline] (11.113 s) : 0, 11112520
Agent [candidate] (1.162 s) : 0, 1162361
Total [candidate] (11.017 s) : 0, 11016507
section profiling
Agent [baseline] (1.167 s) : 0, 1167237
Total [baseline] (10.992 s) : 0, 10991760
Agent [candidate] (1.163 s) : 0, 1162728
Total [candidate] (11.059 s) : 0, 11059346
gantt
title petclinic - break down per module: candidate=1.54.0-SNAPSHOT~f10792d75b, baseline=1.55.0-SNAPSHOT~47f955b5dc
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.469 ms) : 0, 1469
crashtracking [candidate] (1.473 ms) : 0, 1473
BytebuddyAgent [baseline] (692.947 ms) : 0, 692947
BytebuddyAgent [candidate] (691.656 ms) : 0, 691656
GlobalTracer [baseline] (241.77 ms) : 0, 241770
GlobalTracer [candidate] (241.619 ms) : 0, 241619
AppSec [baseline] (32.371 ms) : 0, 32371
AppSec [candidate] (32.365 ms) : 0, 32365
Debugger [baseline] (6.358 ms) : 0, 6358
Debugger [candidate] (6.384 ms) : 0, 6384
Remote Config [baseline] (713.347 µs) : 0, 713
Remote Config [candidate] (693.558 µs) : 0, 694
Telemetry [baseline] (9.283 ms) : 0, 9283
Telemetry [candidate] (9.24 ms) : 0, 9240
Flare Poller [baseline] (10.082 ms) : 0, 10082
Flare Poller [candidate] (10.179 ms) : 0, 10179
section appsec
crashtracking [baseline] (1.482 ms) : 0, 1482
crashtracking [candidate] (1.464 ms) : 0, 1464
BytebuddyAgent [baseline] (717.182 ms) : 0, 717182
BytebuddyAgent [candidate] (718.284 ms) : 0, 718284
GlobalTracer [baseline] (234.425 ms) : 0, 234425
GlobalTracer [candidate] (234.737 ms) : 0, 234737
AppSec [baseline] (175.396 ms) : 0, 175396
AppSec [candidate] (174.524 ms) : 0, 174524
Debugger [baseline] (6.138 ms) : 0, 6138
Debugger [candidate] (6.103 ms) : 0, 6103
Remote Config [baseline] (635.52 µs) : 0, 636
Remote Config [candidate] (627.03 µs) : 0, 627
Telemetry [baseline] (8.516 ms) : 0, 8516
Telemetry [candidate] (9.172 ms) : 0, 9172
Flare Poller [baseline] (3.954 ms) : 0, 3954
Flare Poller [candidate] (3.908 ms) : 0, 3908
IAST [baseline] (24.754 ms) : 0, 24754
IAST [candidate] (24.703 ms) : 0, 24703
section iast
crashtracking [baseline] (1.493 ms) : 0, 1493
crashtracking [candidate] (1.483 ms) : 0, 1483
BytebuddyAgent [baseline] (827.627 ms) : 0, 827627
BytebuddyAgent [candidate] (823.561 ms) : 0, 823561
GlobalTracer [baseline] (234.264 ms) : 0, 234264
GlobalTracer [candidate] (233.393 ms) : 0, 233393
AppSec [baseline] (35.543 ms) : 0, 35543
AppSec [candidate] (35.496 ms) : 0, 35496
Debugger [baseline] (6.203 ms) : 0, 6203
Debugger [candidate] (6.249 ms) : 0, 6249
Remote Config [baseline] (628.595 µs) : 0, 629
Remote Config [candidate] (623.919 µs) : 0, 624
Telemetry [baseline] (8.876 ms) : 0, 8876
Telemetry [candidate] (8.806 ms) : 0, 8806
Flare Poller [baseline] (4.319 ms) : 0, 4319
Flare Poller [candidate] (4.418 ms) : 0, 4418
IAST [baseline] (27.105 ms) : 0, 27105
IAST [candidate] (26.648 ms) : 0, 26648
section profiling
crashtracking [baseline] (1.453 ms) : 0, 1453
crashtracking [candidate] (1.441 ms) : 0, 1441
BytebuddyAgent [baseline] (724.418 ms) : 0, 724418
BytebuddyAgent [candidate] (721.272 ms) : 0, 721272
GlobalTracer [baseline] (218.792 ms) : 0, 218792
GlobalTracer [candidate] (217.769 ms) : 0, 217769
AppSec [baseline] (32.645 ms) : 0, 32645
AppSec [candidate] (32.29 ms) : 0, 32290
Debugger [baseline] (7.34 ms) : 0, 7340
Debugger [candidate] (6.479 ms) : 0, 6479
Remote Config [baseline] (747.64 µs) : 0, 748
Remote Config [candidate] (803.403 µs) : 0, 803
Telemetry [baseline] (15.567 ms) : 0, 15567
Telemetry [candidate] (16.124 ms) : 0, 16124
Flare Poller [baseline] (4.221 ms) : 0, 4221
Flare Poller [candidate] (4.184 ms) : 0, 4184
ProfilingAgent [baseline] (108.785 ms) : 0, 108785
ProfilingAgent [candidate] (109.322 ms) : 0, 109322
Profiling [baseline] (109.646 ms) : 0, 109646
Profiling [candidate] (110.311 ms) : 0, 110311
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.54.0-SNAPSHOT~f10792d75b, baseline=1.55.0-SNAPSHOT~47f955b5dc
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.015 s) : 0, 1015213
Total [baseline] (8.66 s) : 0, 8660253
Agent [candidate] (1.016 s) : 0, 1016496
Total [candidate] (8.714 s) : 0, 8713690
section iast
Agent [baseline] (1.151 s) : 0, 1150504
Total [baseline] (9.309 s) : 0, 9308920
Agent [candidate] (1.16 s) : 0, 1159895
Total [candidate] (9.295 s) : 0, 9294955
gantt
title insecure-bank - break down per module: candidate=1.54.0-SNAPSHOT~f10792d75b, baseline=1.55.0-SNAPSHOT~47f955b5dc
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.477 ms) : 0, 1477
crashtracking [candidate] (1.468 ms) : 0, 1468
BytebuddyAgent [baseline] (694.069 ms) : 0, 694069
BytebuddyAgent [candidate] (693.258 ms) : 0, 693258
GlobalTracer [baseline] (241.781 ms) : 0, 241781
GlobalTracer [candidate] (241.77 ms) : 0, 241770
AppSec [baseline] (32.269 ms) : 0, 32269
AppSec [candidate] (32.365 ms) : 0, 32365
Debugger [baseline] (6.381 ms) : 0, 6381
Debugger [candidate] (6.42 ms) : 0, 6420
Remote Config [baseline] (713.485 µs) : 0, 713
Remote Config [candidate] (705.802 µs) : 0, 706
Telemetry [baseline] (9.41 ms) : 0, 9410
Telemetry [candidate] (9.208 ms) : 0, 9208
Flare Poller [baseline] (7.84 ms) : 0, 7840
Flare Poller [candidate] (10.146 ms) : 0, 10146
section iast
crashtracking [baseline] (1.486 ms) : 0, 1486
crashtracking [candidate] (1.512 ms) : 0, 1512
BytebuddyAgent [baseline] (814.404 ms) : 0, 814404
BytebuddyAgent [candidate] (821.951 ms) : 0, 821951
GlobalTracer [baseline] (231.555 ms) : 0, 231555
GlobalTracer [candidate] (232.793 ms) : 0, 232793
AppSec [baseline] (34.351 ms) : 0, 34351
AppSec [candidate] (35.619 ms) : 0, 35619
Debugger [baseline] (7.14 ms) : 0, 7140
Debugger [candidate] (6.168 ms) : 0, 6168
Remote Config [baseline] (624.792 µs) : 0, 625
Remote Config [candidate] (620.505 µs) : 0, 621
Telemetry [baseline] (8.705 ms) : 0, 8705
Telemetry [candidate] (8.793 ms) : 0, 8793
Flare Poller [baseline] (4.207 ms) : 0, 4207
Flare Poller [candidate] (4.246 ms) : 0, 4246
IAST [baseline] (26.549 ms) : 0, 26549
IAST [candidate] (26.6 ms) : 0, 26600
LoadParameters
See matching parameters
SummaryFound 5 performance improvements and 2 performance regressions! Performance is the same for 5 metrics, 12 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~f10792d75b, baseline=1.55.0-SNAPSHOT~47f955b5dc
dateFormat X
axisFormat %s
section baseline
no_agent (37.385 ms) : 37086, 37683
. : milestone, 37385,
appsec (47.012 ms) : 46606, 47418
. : milestone, 47012,
code_origins (43.976 ms) : 43599, 44353
. : milestone, 43976,
iast (44.127 ms) : 43769, 44485
. : milestone, 44127,
profiling (47.775 ms) : 47298, 48253
. : milestone, 47775,
tracing (44.026 ms) : 43641, 44411
. : milestone, 44026,
section candidate
no_agent (36.158 ms) : 35860, 36456
. : milestone, 36158,
appsec (49.01 ms) : 48580, 49440
. : milestone, 49010,
code_origins (42.48 ms) : 42131, 42829
. : milestone, 42480,
iast (44.332 ms) : 43956, 44709
. : milestone, 44332,
profiling (48.312 ms) : 47874, 48750
. : milestone, 48312,
tracing (45.188 ms) : 44807, 45569
. : milestone, 45188,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~f10792d75b, baseline=1.55.0-SNAPSHOT~47f955b5dc
dateFormat X
axisFormat %s
section baseline
no_agent (4.226 ms) : 4174, 4278
. : milestone, 4226,
iast (9.278 ms) : 9115, 9440
. : milestone, 9278,
iast_FULL (14.656 ms) : 14362, 14950
. : milestone, 14656,
iast_GLOBAL (11.191 ms) : 10989, 11393
. : milestone, 11191,
profiling (9.41 ms) : 9260, 9560
. : milestone, 9410,
tracing (7.886 ms) : 7769, 8002
. : milestone, 7886,
section candidate
no_agent (4.361 ms) : 4304, 4417
. : milestone, 4361,
iast (9.6 ms) : 9438, 9761
. : milestone, 9600,
iast_FULL (13.523 ms) : 13257, 13789
. : milestone, 13523,
iast_GLOBAL (10.547 ms) : 10361, 10733
. : milestone, 10547,
profiling (8.804 ms) : 8666, 8942
. : milestone, 8804,
tracing (8.532 ms) : 8407, 8658
. : milestone, 8532,
DacapoParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 0 unstable metrics.
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~f10792d75b, baseline=1.55.0-SNAPSHOT~47f955b5dc
dateFormat X
axisFormat %s
section baseline
no_agent (1.485 ms) : 1473, 1497
. : milestone, 1485,
appsec (3.762 ms) : 3542, 3982
. : milestone, 3762,
iast (2.219 ms) : 2156, 2283
. : milestone, 2219,
iast_GLOBAL (2.268 ms) : 2204, 2332
. : milestone, 2268,
profiling (2.089 ms) : 2036, 2141
. : milestone, 2089,
tracing (2.037 ms) : 1988, 2087
. : milestone, 2037,
section candidate
no_agent (1.482 ms) : 1470, 1494
. : milestone, 1482,
appsec (2.474 ms) : 2424, 2525
. : milestone, 2474,
iast (2.217 ms) : 2153, 2280
. : milestone, 2217,
iast_GLOBAL (2.275 ms) : 2210, 2339
. : milestone, 2275,
profiling (2.081 ms) : 2028, 2133
. : milestone, 2081,
tracing (2.034 ms) : 1984, 2083
. : milestone, 2034,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~f10792d75b, baseline=1.55.0-SNAPSHOT~47f955b5dc
dateFormat X
axisFormat %s
section baseline
no_agent (14.842 s) : 14842000, 14842000
. : milestone, 14842000,
appsec (15.263 s) : 15263000, 15263000
. : milestone, 15263000,
iast (18.242 s) : 18242000, 18242000
. : milestone, 18242000,
iast_GLOBAL (18.032 s) : 18032000, 18032000
. : milestone, 18032000,
profiling (15.163 s) : 15163000, 15163000
. : milestone, 15163000,
tracing (15.214 s) : 15214000, 15214000
. : milestone, 15214000,
section candidate
no_agent (15.509 s) : 15509000, 15509000
. : milestone, 15509000,
appsec (15.047 s) : 15047000, 15047000
. : milestone, 15047000,
iast (18.682 s) : 18682000, 18682000
. : milestone, 18682000,
iast_GLOBAL (18.276 s) : 18276000, 18276000
. : milestone, 18276000,
profiling (15.143 s) : 15143000, 15143000
. : milestone, 15143000,
tracing (15.177 s) : 15177000, 15177000
. : milestone, 15177000,
|
b3676e3 to
7b7ad49
Compare
|
Hi! 👋 Looks like you updated a Git Submodule.
|
7b7ad49 to
f0b3d92
Compare
|
Hi! 👋 Looks like you updated a Git Submodule.
|
1 similar comment
|
Hi! 👋 Looks like you updated a Git Submodule.
|
7506773 to
c4dcdb5
Compare
|
Hi! 👋 Looks like you updated a Git Submodule.
|
Signed-off-by: sezen.leblay <sezen.leblay@datadoghq.com>
c4dcdb5 to
5ff30b6
Compare
Signed-off-by: sezen.leblay <sezen.leblay@datadoghq.com>
amarziali
left a comment
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 had a first pass. There are some comments to address especially the one concerning the design. In particular, the tagging engine should be moved away from the metrics since concerning mostly the span (tracing). Moving to tagPostProcessor is a first step.
I will do another pass once the first set of comments are resolved
dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricKey.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/HttpEndpointTagging.java
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/common/metrics/HttpEndpointTagging.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/HttpEndpointTagging.java
Show resolved
Hide resolved
dd-trace-core/src/test/groovy/datadog/trace/core/tagprocessor/HttpEndpointTaggingTest.groovy
Show resolved
Hide resolved
dd-trace-core/src/test/java/datadog/trace/common/metrics/MetricKeyTest.java
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/common/metrics/HttpEndpointTagging.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/common/metrics/HttpEndpointTagging.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/common/metrics/HttpEndpointTagging.java
Outdated
Show resolved
Hide resolved
eb38ab8 to
b3ffd63
Compare
b3ffd63 to
3f778ee
Compare
Is it possible to increase this coverage with more tests? :) |
amarziali
left a comment
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 left few more comments. Code wise: tag access in the postprocessor can be done directly using the provided map. Also, the postProcessor can just not be instantiated if not enabled in the config.
An important point about the bucket aggregation: the HTTP method is already available today as a tag and this can cause changes in the behaviour even when the feature is disabled. I recommend an additional guard on aggregating those two new fields
dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/HttpEndpointTagging.java
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/TagsPostProcessorFactory.java
Outdated
Show resolved
Hide resolved
73f00c5 to
07f3244
Compare
e570ee4 to
a13e396
Compare
| final CharSequence httpMethod = resourceRenamingEnabled ? span.getTag(HTTP_METHOD, "") : ""; | ||
| final CharSequence httpEndpoint = resourceRenamingEnabled ? span.getTag(HTTP_ENDPOINT, "") : ""; |
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.
Question: are they mandatory in the payload? if not they should be nullified here and not sent in the msgpack payload later on
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.
@jandro996 can i let you guys take over please?
mtoffl01
left a comment
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.
Reviewing from the Config side. I'm not following the distinction between "resource renaming" versus "resource renaming always simplified endpoint." Can you either link an RFC so I can understand it better, or provide documentation in the code?
Thanks!
What Does This Do
Adds the new http.endpoint tag and modifies the cases where http.route is utilized accordingly
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: APPSEC-58611