-
Notifications
You must be signed in to change notification settings - Fork 324
Introduce @AppliesOn to override advices InstrumenterModule target system #10404
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
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 6 performance improvements and 2 performance regressions! Performance is the same for 49 metrics, 8 unstable metrics.
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.59.0-SNAPSHOT~38964dbcc8, baseline=1.59.0-SNAPSHOT~ea999452fb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.09 s) : 0, 1090449
Total [baseline] (8.815 s) : 0, 8814864
Agent [candidate] (1.061 s) : 0, 1061274
Total [candidate] (8.766 s) : 0, 8765680
section iast
Agent [baseline] (1.225 s) : 0, 1225289
Total [baseline] (9.33 s) : 0, 9329511
Agent [candidate] (1.218 s) : 0, 1217874
Total [candidate] (9.343 s) : 0, 9343147
gantt
title insecure-bank - break down per module: candidate=1.59.0-SNAPSHOT~38964dbcc8, baseline=1.59.0-SNAPSHOT~ea999452fb
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.187 ms) : 0, 1187
crashtracking [candidate] (1.215 ms) : 0, 1215
BytebuddyAgent [baseline] (655.912 ms) : 0, 655912
BytebuddyAgent [candidate] (625.028 ms) : 0, 625028
GlobalTracer [baseline] (283.671 ms) : 0, 283671
GlobalTracer [candidate] (285.149 ms) : 0, 285149
AppSec [baseline] (32.768 ms) : 0, 32768
AppSec [candidate] (33.214 ms) : 0, 33214
Debugger [baseline] (67.594 ms) : 0, 67594
Debugger [candidate] (64.244 ms) : 0, 64244
Remote Config [baseline] (633.992 µs) : 0, 634
Remote Config [candidate] (612.848 µs) : 0, 613
Telemetry [baseline] (9.016 ms) : 0, 9016
Telemetry [candidate] (9.089 ms) : 0, 9089
Flare Poller [baseline] (3.869 ms) : 0, 3869
Flare Poller [candidate] (6.964 ms) : 0, 6964
section iast
crashtracking [baseline] (1.179 ms) : 0, 1179
crashtracking [candidate] (1.18 ms) : 0, 1180
BytebuddyAgent [baseline] (793.106 ms) : 0, 793106
BytebuddyAgent [candidate] (786.216 ms) : 0, 786216
GlobalTracer [baseline] (256.717 ms) : 0, 256717
GlobalTracer [candidate] (255.397 ms) : 0, 255397
IAST [baseline] (26.934 ms) : 0, 26934
IAST [candidate] (26.929 ms) : 0, 26929
AppSec [baseline] (35.38 ms) : 0, 35380
AppSec [candidate] (34.583 ms) : 0, 34583
Debugger [baseline] (63.954 ms) : 0, 63954
Debugger [candidate] (65.536 ms) : 0, 65536
Remote Config [baseline] (565.811 µs) : 0, 566
Remote Config [candidate] (595.812 µs) : 0, 596
Telemetry [baseline] (8.359 ms) : 0, 8359
Telemetry [candidate] (8.56 ms) : 0, 8560
Flare Poller [baseline] (3.57 ms) : 0, 3570
Flare Poller [candidate] (3.522 ms) : 0, 3522
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.59.0-SNAPSHOT~38964dbcc8, baseline=1.59.0-SNAPSHOT~ea999452fb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.086 s) : 0, 1085912
Total [baseline] (10.84 s) : 0, 10840422
Agent [candidate] (1.054 s) : 0, 1053861
Total [candidate] (11.066 s) : 0, 11065967
section appsec
Agent [baseline] (1.265 s) : 0, 1264659
Total [baseline] (11.014 s) : 0, 11014173
Agent [candidate] (1.227 s) : 0, 1227002
Total [candidate] (11.138 s) : 0, 11138404
section iast
Agent [baseline] (1.224 s) : 0, 1224274
Total [baseline] (11.191 s) : 0, 11191077
Agent [candidate] (1.219 s) : 0, 1218959
Total [candidate] (11.103 s) : 0, 11102630
section profiling
Agent [baseline] (1.205 s) : 0, 1205273
Total [baseline] (10.897 s) : 0, 10896821
Agent [candidate] (1.185 s) : 0, 1185438
Total [candidate] (10.92 s) : 0, 10919863
gantt
title petclinic - break down per module: candidate=1.59.0-SNAPSHOT~38964dbcc8, baseline=1.59.0-SNAPSHOT~ea999452fb
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.182 ms) : 0, 1182
crashtracking [candidate] (1.179 ms) : 0, 1179
BytebuddyAgent [baseline] (651.949 ms) : 0, 651949
BytebuddyAgent [candidate] (619.21 ms) : 0, 619210
GlobalTracer [baseline] (283.239 ms) : 0, 283239
GlobalTracer [candidate] (283.336 ms) : 0, 283336
AppSec [baseline] (32.495 ms) : 0, 32495
AppSec [candidate] (33.01 ms) : 0, 33010
Debugger [baseline] (67.973 ms) : 0, 67973
Debugger [candidate] (66.449 ms) : 0, 66449
Remote Config [baseline] (705.806 µs) : 0, 706
Remote Config [candidate] (618.741 µs) : 0, 619
Telemetry [baseline] (8.97 ms) : 0, 8970
Telemetry [candidate] (9.092 ms) : 0, 9092
Flare Poller [baseline] (3.784 ms) : 0, 3784
Flare Poller [candidate] (5.368 ms) : 0, 5368
section appsec
crashtracking [baseline] (1.177 ms) : 0, 1177
crashtracking [candidate] (1.183 ms) : 0, 1183
BytebuddyAgent [baseline] (691.108 ms) : 0, 691108
BytebuddyAgent [candidate] (649.089 ms) : 0, 649089
GlobalTracer [baseline] (258.615 ms) : 0, 258615
GlobalTracer [candidate] (266.205 ms) : 0, 266205
AppSec [baseline] (173.273 ms) : 0, 173273
AppSec [candidate] (168.383 ms) : 0, 168383
Debugger [baseline] (66.667 ms) : 0, 66667
Debugger [candidate] (67.592 ms) : 0, 67592
Remote Config [baseline] (853.587 µs) : 0, 854
Remote Config [candidate] (734.021 µs) : 0, 734
Telemetry [baseline] (9.357 ms) : 0, 9357
Telemetry [candidate] (9.288 ms) : 0, 9288
Flare Poller [baseline] (3.761 ms) : 0, 3761
Flare Poller [candidate] (3.759 ms) : 0, 3759
IAST [baseline] (24.459 ms) : 0, 24459
IAST [candidate] (25.305 ms) : 0, 25305
section iast
crashtracking [baseline] (1.177 ms) : 0, 1177
crashtracking [candidate] (1.183 ms) : 0, 1183
BytebuddyAgent [baseline] (792.353 ms) : 0, 792353
BytebuddyAgent [candidate] (785.924 ms) : 0, 785924
GlobalTracer [baseline] (255.059 ms) : 0, 255059
GlobalTracer [candidate] (255.516 ms) : 0, 255516
AppSec [baseline] (34.45 ms) : 0, 34450
AppSec [candidate] (32.743 ms) : 0, 32743
Debugger [baseline] (66.195 ms) : 0, 66195
Debugger [candidate] (68.327 ms) : 0, 68327
Remote Config [baseline] (593.808 µs) : 0, 594
Remote Config [candidate] (605.842 µs) : 0, 606
Telemetry [baseline] (8.59 ms) : 0, 8590
Telemetry [candidate] (8.564 ms) : 0, 8564
Flare Poller [baseline] (3.608 ms) : 0, 3608
Flare Poller [candidate] (3.532 ms) : 0, 3532
IAST [baseline] (26.829 ms) : 0, 26829
IAST [candidate] (27.026 ms) : 0, 27026
section profiling
ProfilingAgent [baseline] (97.159 ms) : 0, 97159
ProfilingAgent [candidate] (98.955 ms) : 0, 98955
crashtracking [baseline] (1.211 ms) : 0, 1211
crashtracking [candidate] (1.227 ms) : 0, 1227
BytebuddyAgent [baseline] (702.902 ms) : 0, 702902
BytebuddyAgent [candidate] (677.997 ms) : 0, 677997
GlobalTracer [baseline] (220.974 ms) : 0, 220974
GlobalTracer [candidate] (222.606 ms) : 0, 222606
AppSec [baseline] (32.047 ms) : 0, 32047
AppSec [candidate] (32.484 ms) : 0, 32484
Debugger [baseline] (67.786 ms) : 0, 67786
Debugger [candidate] (68.785 ms) : 0, 68785
Remote Config [baseline] (644.257 µs) : 0, 644
Remote Config [candidate] (624.48 µs) : 0, 624
Telemetry [baseline] (8.902 ms) : 0, 8902
Telemetry [candidate] (8.721 ms) : 0, 8721
Flare Poller [baseline] (3.779 ms) : 0, 3779
Flare Poller [candidate] (3.786 ms) : 0, 3786
Profiling [baseline] (97.733 ms) : 0, 97733
Profiling [candidate] (99.534 ms) : 0, 99534
LoadParameters
See matching parameters
SummaryFound 3 performance improvements and 1 performance regressions! Performance is the same for 16 metrics, 16 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.59.0-SNAPSHOT~38964dbcc8, baseline=1.59.0-SNAPSHOT~ea999452fb
dateFormat X
axisFormat %s
section baseline
no_agent (18.32 ms) : 18129, 18511
. : milestone, 18320,
appsec (18.629 ms) : 18441, 18818
. : milestone, 18629,
code_origins (17.647 ms) : 17475, 17819
. : milestone, 17647,
iast (17.744 ms) : 17567, 17921
. : milestone, 17744,
profiling (18.572 ms) : 18386, 18757
. : milestone, 18572,
tracing (18.622 ms) : 18433, 18812
. : milestone, 18622,
section candidate
no_agent (17.453 ms) : 17273, 17634
. : milestone, 17453,
appsec (19.414 ms) : 19216, 19611
. : milestone, 19414,
code_origins (17.653 ms) : 17477, 17829
. : milestone, 17653,
iast (17.73 ms) : 17557, 17903
. : milestone, 17730,
profiling (18.499 ms) : 18315, 18682
. : milestone, 18499,
tracing (17.511 ms) : 17340, 17683
. : milestone, 17511,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.59.0-SNAPSHOT~38964dbcc8, baseline=1.59.0-SNAPSHOT~ea999452fb
dateFormat X
axisFormat %s
section baseline
no_agent (1.218 ms) : 1204, 1232
. : milestone, 1218,
iast (3.101 ms) : 3060, 3141
. : milestone, 3101,
iast_FULL (5.688 ms) : 5631, 5745
. : milestone, 5688,
iast_GLOBAL (3.581 ms) : 3522, 3639
. : milestone, 3581,
profiling (2.034 ms) : 2016, 2052
. : milestone, 2034,
tracing (1.762 ms) : 1748, 1776
. : milestone, 1762,
section candidate
no_agent (1.176 ms) : 1164, 1187
. : milestone, 1176,
iast (3.173 ms) : 3136, 3209
. : milestone, 3173,
iast_FULL (5.645 ms) : 5589, 5701
. : milestone, 5645,
iast_GLOBAL (3.508 ms) : 3458, 3559
. : milestone, 3508,
profiling (1.954 ms) : 1938, 1970
. : milestone, 1954,
tracing (1.816 ms) : 1801, 1831
. : milestone, 1816,
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.59.0-SNAPSHOT~38964dbcc8, baseline=1.59.0-SNAPSHOT~ea999452fb
dateFormat X
axisFormat %s
section baseline
no_agent (15.084 s) : 15084000, 15084000
. : milestone, 15084000,
appsec (14.583 s) : 14583000, 14583000
. : milestone, 14583000,
iast (17.984 s) : 17984000, 17984000
. : milestone, 17984000,
iast_GLOBAL (17.919 s) : 17919000, 17919000
. : milestone, 17919000,
profiling (15.078 s) : 15078000, 15078000
. : milestone, 15078000,
tracing (14.79 s) : 14790000, 14790000
. : milestone, 14790000,
section candidate
no_agent (14.765 s) : 14765000, 14765000
. : milestone, 14765000,
appsec (14.868 s) : 14868000, 14868000
. : milestone, 14868000,
iast (18.533 s) : 18533000, 18533000
. : milestone, 18533000,
iast_GLOBAL (17.679 s) : 17679000, 17679000
. : milestone, 17679000,
profiling (15.449 s) : 15449000, 15449000
. : milestone, 15449000,
tracing (15.182 s) : 15182000, 15182000
. : milestone, 15182000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.59.0-SNAPSHOT~38964dbcc8, baseline=1.59.0-SNAPSHOT~ea999452fb
dateFormat X
axisFormat %s
section baseline
no_agent (1.469 ms) : 1457, 1480
. : milestone, 1469,
appsec (2.457 ms) : 2405, 2509
. : milestone, 2457,
iast (2.207 ms) : 2142, 2273
. : milestone, 2207,
iast_GLOBAL (2.247 ms) : 2182, 2312
. : milestone, 2247,
profiling (2.064 ms) : 2011, 2116
. : milestone, 2064,
tracing (2.041 ms) : 1989, 2092
. : milestone, 2041,
section candidate
no_agent (1.468 ms) : 1456, 1479
. : milestone, 1468,
appsec (3.713 ms) : 3494, 3932
. : milestone, 3713,
iast (2.204 ms) : 2139, 2269
. : milestone, 2204,
iast_GLOBAL (2.252 ms) : 2186, 2317
. : milestone, 2252,
profiling (2.468 ms) : 2305, 2632
. : milestone, 2468,
tracing (2.031 ms) : 1981, 2082
. : milestone, 2031,
|
33b8ea3 to
c1ed2d0
Compare
PerfectSlayer
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.
Hey there 👋 Quick and early review!
❔ question: Can instrumentation changes related to isApplicable vs isEnabled can be split in another PR?
Similarly, the migration to the new context tracking system can move a to follow up PR.
...-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterModuleFilter.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/annotation/AppliesOn.java
Outdated
Show resolved
Hide resolved
...mcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/TomcatServerInstrumentation.java
Outdated
Show resolved
Hide resolved
c1ed2d0 to
3cd1ce7
Compare
9989dc0 to
aa9d043
Compare
PerfectSlayer
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.
First pass review on API and doc. I will have a look at implementation as soon as I get a stable dev env.
...bility-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilitySmokeTest.groovy
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/annotation/AppliesOn.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/annotation/AppliesOn.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/annotation/AppliesOn.java
Outdated
Show resolved
Hide resolved
...mcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/TomcatServerInstrumentation.java
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Stuart McCulloch <stuart.mcculloch@datadoghq.com> Co-authored-by: Bruce Bujon <PerfectSlayer@users.noreply.github.com>
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterIndex.java
Outdated
Show resolved
Hide resolved
…tooling/InstrumenterIndex.java Co-authored-by: Stuart McCulloch <stuart.mcculloch@datadoghq.com>
…tooling/InstrumenterIndex.java Co-authored-by: Stuart McCulloch <stuart.mcculloch@datadoghq.com>
mcculls
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.
This is a really nice addition, great work!
What Does This Do
This PR introduces a new
@AppliesOnannotation that allows method advices to explicitly override the target system(s) defined at theInstrumenterModulelevel.In addition to the annotation itself, this PR refactors how applicability is computed and recorded so that only advices that are known a priori to be applicable are ever loaded or considered, significantly improving startup performance and reducing unnecessary classloading.
This PR also refactor the tomcat server instrumentation in order to separate tracing and pure context extraction concerns. This showcases how to use
@AppliesOnPerformance impact
A major benefit of this change is startup-time improvement.
By limiting classloading strictly to modules and advices that are known in advance to be applicable to the enabled target systems,
we:
This is especially impactful in environments with:
Please note that the startup gain is independent from the application classloading / size / nature and, for a given number of enabled target systems, will give the same startup gain. So that, being the startup time relatively small, the gain will be more appreciable in applications having a lightweight quick startup.
Motivation
Historically, target-system applicability (
TargetSystem) has been defined at theInstrumenterModulelevel and applied uniformly to all advices contributed by the module. This approach has several limitations:Key changes
1.
@AppliesOnannotation for advices@AppliesOn(TargetSystem...)annotation can be applied to method advice classes.InstrumenterModule.2. New target systems:
CONTEXT_TRACKINGandRASPThis PR introduces two new
TargetSystems:CONTEXT_TRACKING: always enabled, it's meant to be used by advices contributing to pure context manipulation to separate them from the pure tracing ones.RASP: a dedicated one to separate from APPSEC/IASTAdditional Notes
The
@AppliesOnannotation is today used to override the applicability of an advice. In a larger refactoring effort, we could think annotating all the advices in order to get rid to the more strict hierarchy ofInstrumenterModule.(Tracing|Iast|,..). Only using then interfaces will bring design advantages.Contributor Checklist
type:and (comp:orinst:) labels in addition to any useful labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]