instrument inner requests in apache http client#10603
instrument inner requests in apache http client#10603
Conversation
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 61 metrics, 10 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.60.0-SNAPSHOT~8d08af8980, baseline=1.60.0-SNAPSHOT~61376da955
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.073 s) : 0, 1073471
Total [baseline] (10.887 s) : 0, 10887410
Agent [candidate] (1.069 s) : 0, 1069066
Total [candidate] (10.836 s) : 0, 10835675
section appsec
Agent [baseline] (1.24 s) : 0, 1239667
Total [baseline] (11.075 s) : 0, 11074824
Agent [candidate] (1.241 s) : 0, 1240537
Total [candidate] (11.025 s) : 0, 11024653
section iast
Agent [baseline] (1.235 s) : 0, 1235220
Total [baseline] (11.225 s) : 0, 11225129
Agent [candidate] (1.233 s) : 0, 1232739
Total [candidate] (11.25 s) : 0, 11250373
section profiling
Agent [baseline] (1.204 s) : 0, 1203851
Total [baseline] (11.015 s) : 0, 11014951
Agent [candidate] (1.192 s) : 0, 1192494
Total [candidate] (11.063 s) : 0, 11062561
gantt
title petclinic - break down per module: candidate=1.60.0-SNAPSHOT~8d08af8980, baseline=1.60.0-SNAPSHOT~61376da955
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.201 ms) : 0, 1201
crashtracking [candidate] (1.197 ms) : 0, 1197
BytebuddyAgent [baseline] (634.82 ms) : 0, 634820
BytebuddyAgent [candidate] (632.393 ms) : 0, 632393
AgentMeter [baseline] (29.111 ms) : 0, 29111
AgentMeter [candidate] (28.951 ms) : 0, 28951
GlobalTracer [baseline] (258.668 ms) : 0, 258668
GlobalTracer [candidate] (257.699 ms) : 0, 257699
AppSec [baseline] (32.922 ms) : 0, 32922
AppSec [candidate] (32.824 ms) : 0, 32824
Debugger [baseline] (62.959 ms) : 0, 62959
Debugger [candidate] (60.577 ms) : 0, 60577
Remote Config [baseline] (622.029 µs) : 0, 622
Remote Config [candidate] (625.953 µs) : 0, 626
Telemetry [baseline] (12.251 ms) : 0, 12251
Telemetry [candidate] (12.291 ms) : 0, 12291
Flare Poller [baseline] (5.403 ms) : 0, 5403
Flare Poller [candidate] (6.992 ms) : 0, 6992
section appsec
crashtracking [baseline] (1.194 ms) : 0, 1194
crashtracking [candidate] (1.171 ms) : 0, 1171
BytebuddyAgent [baseline] (658.918 ms) : 0, 658918
BytebuddyAgent [candidate] (658.406 ms) : 0, 658406
AgentMeter [baseline] (11.985 ms) : 0, 11985
AgentMeter [candidate] (11.991 ms) : 0, 11991
GlobalTracer [baseline] (258.234 ms) : 0, 258234
GlobalTracer [candidate] (258.216 ms) : 0, 258216
IAST [baseline] (25.025 ms) : 0, 25025
IAST [candidate] (25.348 ms) : 0, 25348
AppSec [baseline] (168.327 ms) : 0, 168327
AppSec [candidate] (168.628 ms) : 0, 168628
Debugger [baseline] (66.452 ms) : 0, 66452
Debugger [candidate] (67.277 ms) : 0, 67277
Remote Config [baseline] (654.223 µs) : 0, 654
Remote Config [candidate] (654.368 µs) : 0, 654
Telemetry [baseline] (9.883 ms) : 0, 9883
Telemetry [candidate] (9.852 ms) : 0, 9852
Flare Poller [baseline] (3.72 ms) : 0, 3720
Flare Poller [candidate] (3.723 ms) : 0, 3723
section iast
crashtracking [baseline] (1.182 ms) : 0, 1182
crashtracking [candidate] (1.175 ms) : 0, 1175
BytebuddyAgent [baseline] (797.944 ms) : 0, 797944
BytebuddyAgent [candidate] (795.76 ms) : 0, 795760
AgentMeter [baseline] (11.272 ms) : 0, 11272
AgentMeter [candidate] (11.248 ms) : 0, 11248
GlobalTracer [baseline] (249.143 ms) : 0, 249143
GlobalTracer [candidate] (248.488 ms) : 0, 248488
IAST [baseline] (27.019 ms) : 0, 27019
IAST [candidate] (26.962 ms) : 0, 26962
AppSec [baseline] (33.156 ms) : 0, 33156
AppSec [candidate] (33.136 ms) : 0, 33136
Debugger [baseline] (67.554 ms) : 0, 67554
Debugger [candidate] (67.994 ms) : 0, 67994
Remote Config [baseline] (533.192 µs) : 0, 533
Remote Config [candidate] (545.797 µs) : 0, 546
Telemetry [baseline] (8.615 ms) : 0, 8615
Telemetry [candidate] (8.72 ms) : 0, 8720
Flare Poller [baseline] (3.493 ms) : 0, 3493
Flare Poller [candidate] (3.445 ms) : 0, 3445
section profiling
crashtracking [baseline] (1.229 ms) : 0, 1229
crashtracking [candidate] (1.225 ms) : 0, 1225
BytebuddyAgent [baseline] (690.681 ms) : 0, 690681
BytebuddyAgent [candidate] (683.749 ms) : 0, 683749
AgentMeter [baseline] (8.777 ms) : 0, 8777
AgentMeter [candidate] (8.62 ms) : 0, 8620
GlobalTracer [baseline] (218.649 ms) : 0, 218649
GlobalTracer [candidate] (215.932 ms) : 0, 215932
AppSec [baseline] (33.262 ms) : 0, 33262
AppSec [candidate] (32.537 ms) : 0, 32537
Debugger [baseline] (67.787 ms) : 0, 67787
Debugger [candidate] (67.247 ms) : 0, 67247
Remote Config [baseline] (621.371 µs) : 0, 621
Remote Config [candidate] (622.295 µs) : 0, 622
Telemetry [baseline] (8.862 ms) : 0, 8862
Telemetry [candidate] (8.963 ms) : 0, 8963
Flare Poller [baseline] (3.718 ms) : 0, 3718
Flare Poller [candidate] (3.776 ms) : 0, 3776
ProfilingAgent [baseline] (99.594 ms) : 0, 99594
ProfilingAgent [candidate] (99.594 ms) : 0, 99594
Profiling [baseline] (100.177 ms) : 0, 100177
Profiling [candidate] (100.172 ms) : 0, 100172
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.60.0-SNAPSHOT~8d08af8980, baseline=1.60.0-SNAPSHOT~61376da955
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.072 s) : 0, 1071920
Total [baseline] (8.773 s) : 0, 8772799
Agent [candidate] (1.079 s) : 0, 1078835
Total [candidate] (8.779 s) : 0, 8778879
section iast
Agent [baseline] (1.234 s) : 0, 1233927
Total [baseline] (9.399 s) : 0, 9399317
Agent [candidate] (1.245 s) : 0, 1244806
Total [candidate] (9.403 s) : 0, 9402628
gantt
title insecure-bank - break down per module: candidate=1.60.0-SNAPSHOT~8d08af8980, baseline=1.60.0-SNAPSHOT~61376da955
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.192 ms) : 0, 1192
crashtracking [candidate] (1.199 ms) : 0, 1199
BytebuddyAgent [baseline] (633.79 ms) : 0, 633790
BytebuddyAgent [candidate] (638.594 ms) : 0, 638594
AgentMeter [baseline] (29.205 ms) : 0, 29205
AgentMeter [candidate] (29.498 ms) : 0, 29498
GlobalTracer [baseline] (259.201 ms) : 0, 259201
GlobalTracer [candidate] (260.924 ms) : 0, 260924
AppSec [baseline] (33.126 ms) : 0, 33126
AppSec [candidate] (33.219 ms) : 0, 33219
Debugger [baseline] (61.759 ms) : 0, 61759
Debugger [candidate] (61.215 ms) : 0, 61215
Remote Config [baseline] (639.11 µs) : 0, 639
Remote Config [candidate] (656.682 µs) : 0, 657
Telemetry [baseline] (11.431 ms) : 0, 11431
Telemetry [candidate] (10.8 ms) : 0, 10800
Flare Poller [baseline] (6.138 ms) : 0, 6138
Flare Poller [candidate] (6.983 ms) : 0, 6983
section iast
crashtracking [baseline] (1.191 ms) : 0, 1191
crashtracking [candidate] (1.199 ms) : 0, 1199
BytebuddyAgent [baseline] (796.901 ms) : 0, 796901
BytebuddyAgent [candidate] (805.599 ms) : 0, 805599
AgentMeter [baseline] (11.353 ms) : 0, 11353
AgentMeter [candidate] (11.514 ms) : 0, 11514
GlobalTracer [baseline] (249.302 ms) : 0, 249302
GlobalTracer [candidate] (249.819 ms) : 0, 249819
IAST [baseline] (27.173 ms) : 0, 27173
IAST [candidate] (27.298 ms) : 0, 27298
AppSec [baseline] (33.953 ms) : 0, 33953
AppSec [candidate] (34.091 ms) : 0, 34091
Debugger [baseline] (66.075 ms) : 0, 66075
Debugger [candidate] (66.86 ms) : 0, 66860
Remote Config [baseline] (547.807 µs) : 0, 548
Remote Config [candidate] (552.47 µs) : 0, 552
Telemetry [baseline] (8.653 ms) : 0, 8653
Telemetry [candidate] (8.729 ms) : 0, 8729
Flare Poller [baseline] (3.483 ms) : 0, 3483
Flare Poller [candidate] (3.543 ms) : 0, 3543
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 0 performance regressions! Performance is the same for 19 metrics, 15 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.60.0-SNAPSHOT~8d08af8980, baseline=1.60.0-SNAPSHOT~61376da955
dateFormat X
axisFormat %s
section baseline
no_agent (18.999 ms) : 18809, 19189
. : milestone, 18999,
appsec (18.434 ms) : 18243, 18624
. : milestone, 18434,
code_origins (17.354 ms) : 17184, 17524
. : milestone, 17354,
iast (17.659 ms) : 17480, 17838
. : milestone, 17659,
profiling (18.53 ms) : 18343, 18717
. : milestone, 18530,
tracing (18.075 ms) : 17897, 18252
. : milestone, 18075,
section candidate
no_agent (17.003 ms) : 16836, 17169
. : milestone, 17003,
appsec (18.313 ms) : 18127, 18500
. : milestone, 18313,
code_origins (17.652 ms) : 17475, 17829
. : milestone, 17652,
iast (17.969 ms) : 17792, 18146
. : milestone, 17969,
profiling (18.384 ms) : 18203, 18566
. : milestone, 18384,
tracing (17.91 ms) : 17731, 18090
. : milestone, 17910,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.60.0-SNAPSHOT~8d08af8980, baseline=1.60.0-SNAPSHOT~61376da955
dateFormat X
axisFormat %s
section baseline
no_agent (1.208 ms) : 1196, 1220
. : milestone, 1208,
iast (3.203 ms) : 3160, 3246
. : milestone, 3203,
iast_FULL (5.692 ms) : 5636, 5748
. : milestone, 5692,
iast_GLOBAL (3.466 ms) : 3415, 3516
. : milestone, 3466,
profiling (1.99 ms) : 1971, 2009
. : milestone, 1990,
tracing (1.775 ms) : 1760, 1790
. : milestone, 1775,
section candidate
no_agent (1.167 ms) : 1156, 1178
. : milestone, 1167,
iast (3.186 ms) : 3143, 3230
. : milestone, 3186,
iast_FULL (5.782 ms) : 5724, 5840
. : milestone, 5782,
iast_GLOBAL (3.377 ms) : 3327, 3427
. : milestone, 3377,
profiling (1.972 ms) : 1953, 1991
. : milestone, 1972,
tracing (1.78 ms) : 1765, 1794
. : milestone, 1780,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.60.0-SNAPSHOT~8d08af8980, baseline=1.60.0-SNAPSHOT~61376da955
dateFormat X
axisFormat %s
section baseline
no_agent (1.481 ms) : 1469, 1492
. : milestone, 1481,
appsec (2.525 ms) : 2470, 2580
. : milestone, 2525,
iast (2.27 ms) : 2201, 2340
. : milestone, 2270,
iast_GLOBAL (2.303 ms) : 2233, 2373
. : milestone, 2303,
profiling (2.105 ms) : 2050, 2160
. : milestone, 2105,
tracing (2.074 ms) : 2020, 2128
. : milestone, 2074,
section candidate
no_agent (1.478 ms) : 1467, 1490
. : milestone, 1478,
appsec (3.798 ms) : 3578, 4018
. : milestone, 3798,
iast (2.261 ms) : 2191, 2330
. : milestone, 2261,
iast_GLOBAL (2.311 ms) : 2241, 2381
. : milestone, 2311,
profiling (2.095 ms) : 2040, 2150
. : milestone, 2095,
tracing (2.085 ms) : 2031, 2139
. : milestone, 2085,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.60.0-SNAPSHOT~8d08af8980, baseline=1.60.0-SNAPSHOT~61376da955
dateFormat X
axisFormat %s
section baseline
no_agent (14.961 s) : 14961000, 14961000
. : milestone, 14961000,
appsec (15.149 s) : 15149000, 15149000
. : milestone, 15149000,
iast (18.067 s) : 18067000, 18067000
. : milestone, 18067000,
iast_GLOBAL (18.019 s) : 18019000, 18019000
. : milestone, 18019000,
profiling (15.359 s) : 15359000, 15359000
. : milestone, 15359000,
tracing (14.873 s) : 14873000, 14873000
. : milestone, 14873000,
section candidate
no_agent (15.27 s) : 15270000, 15270000
. : milestone, 15270000,
appsec (15.023 s) : 15023000, 15023000
. : milestone, 15023000,
iast (18.454 s) : 18454000, 18454000
. : milestone, 18454000,
iast_GLOBAL (17.904 s) : 17904000, 17904000
. : milestone, 17904000,
profiling (15.022 s) : 15022000, 15022000
. : milestone, 15022000,
tracing (14.905 s) : 14905000, 14905000
. : milestone, 14905000,
|
|
I think there is an issue with tests, I'll check tomorrow |
|
there is still an issue, I'm not sure what it is, but I think it's in how the test is setup, because I can never see the ExecInterceptor request being fired when I set breakpoints. I cannot spent more time on this at the moment, going to stash it aside for later |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca75cf3088
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (instrumented == Boolean.TRUE) { | ||
| return true; | ||
| } | ||
| instrumentationMarker.put(request, Boolean.TRUE); |
There was a problem hiding this comment.
Reset request instrumentation marker after execute
HelperMethods.testAndSet stores Boolean.TRUE for each ClassicHttpRequest, but no exit path clears that marker, so any later execute(...) call that reuses the same request instance will always be treated as already instrumented and skip span creation. This drops client spans for common retry/reuse flows (for example, user code re-executing a prepared request after an exception) and is a regression from the previous call-depth behavior that allowed subsequent top-level executes.
Useful? React with 👍 / 👎.
|
|
||
| abstract CloseableHttpResponse executeRequest(T request, URI uri) | ||
|
|
||
| def "same request instance reused across execute calls is instrumented each time"() { |
There was a problem hiding this comment.
putting this here rather than in HttpClientTest directly because it only exposes a doRequest, not something to create and then send like this class does.
| def uri = server.address.resolve("/success") | ||
| def request = createRequest("GET", uri) | ||
| // prevent server-side spans from being created | ||
| request.addHeader(new BasicHeader("is-dd-server", "false")) |
There was a problem hiding this comment.
why do you need that? If it's created it can be anyway asserted
There was a problem hiding this comment.
well, it's not the point of the test, and it was causing me trouble because of the order the spans came in didn't appear to be consistent. The AI suggested this and I thought it was a good thing, for both reasons.
What Does This Do
replace global depth count with one that counts depth per request, so that we can instrument new requests and still avoid double instrumenting.
Motivation
#10383
Additional Notes
Used a ConstextStore to store the depth on the request. I'm not very happy with how verbose it is in the advice, but I don't really see a way around it.
note: test is AI generated
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.