Simplify context propagation #8719
Open
+95
−75
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
PerfectSlayer
added
type: enhancement
tag: do not merge
BenchmarksStartupParameters
See matching parameters
SummaryFound 8 performance improvements and 22 performance regressions! Performance is the same for 33 metrics, 8 unstable metrics.
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.49.0-SNAPSHOT~a84184b02cd, baseline=1.49.0-SNAPSHOT~4d01d583bb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.005 s) : 0, 1004665
Total [baseline] (8.651 s) : 0, 8651453
Agent [candidate] (1.064 s) : 0, 1064138
Total [candidate] (8.652 s) : 0, 8652218
section iast
Agent [baseline] (1.144 s) : 0, 1144310
Total [baseline] (9.2 s) : 0, 9200394
Agent [candidate] (1.183 s) : 0, 1182774
Total [candidate] (9.205 s) : 0, 9205259
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.136 s) : 0, 1135670
Total [baseline] (9.151 s) : 0, 9151465
Agent [candidate] (1.185 s) : 0, 1185144
Total [candidate] (9.203 s) : 0, 9202712
section iast_TELEMETRY_OFF
Agent [baseline] (1.135 s) : 0, 1134860
Total [baseline] (9.169 s) : 0, 9168549
Agent [candidate] (1.177 s) : 0, 1177160
Total [candidate] (9.256 s) : 0, 9255726
gantt
title insecure-bank - break down per module: candidate=1.49.0-SNAPSHOT~a84184b02cd, baseline=1.49.0-SNAPSHOT~4d01d583bb
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (669.293 ms) : 0, 669293
BytebuddyAgent [candidate] (724.554 ms) : 0, 724554
GlobalTracer [baseline] (239.914 ms) : 0, 239914
GlobalTracer [candidate] (240.302 ms) : 0, 240302
AppSec [baseline] (54.525 ms) : 0, 54525
AppSec [candidate] (56.321 ms) : 0, 56321
Debugger [baseline] (6.103 ms) : 0, 6103
Debugger [candidate] (4.401 ms) : 0, 4401
Remote Config [baseline] (689.715 µs) : 0, 690
Remote Config [candidate] (2.24 ms) : 0, 2240
Telemetry [baseline] (10.605 ms) : 0, 10605
Telemetry [candidate] (12.874 ms) : 0, 12874
section iast
BytebuddyAgent [baseline] (797.464 ms) : 0, 797464
BytebuddyAgent [candidate] (837.47 ms) : 0, 837470
GlobalTracer [baseline] (230.294 ms) : 0, 230294
GlobalTracer [candidate] (229.406 ms) : 0, 229406
IAST [baseline] (22.895 ms) : 0, 22895
IAST [candidate] (22.885 ms) : 0, 22885
AppSec [baseline] (55.874 ms) : 0, 55874
AppSec [candidate] (56.046 ms) : 0, 56046
Debugger [baseline] (5.869 ms) : 0, 5869
Debugger [candidate] (4.272 ms) : 0, 4272
Remote Config [baseline] (596.312 µs) : 0, 596
Remote Config [candidate] (606.428 µs) : 0, 606
Telemetry [baseline] (7.776 ms) : 0, 7776
Telemetry [candidate] (8.712 ms) : 0, 8712
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (788.89 ms) : 0, 788890
BytebuddyAgent [candidate] (839.867 ms) : 0, 839867
GlobalTracer [baseline] (229.731 ms) : 0, 229731
GlobalTracer [candidate] (229.814 ms) : 0, 229814
IAST [baseline] (22.995 ms) : 0, 22995
IAST [candidate] (24.285 ms) : 0, 24285
AppSec [baseline] (56.339 ms) : 0, 56339
AppSec [candidate] (54.253 ms) : 0, 54253
Debugger [baseline] (5.896 ms) : 0, 5896
Debugger [candidate] (4.189 ms) : 0, 4189
Remote Config [baseline] (611.299 µs) : 0, 611
Remote Config [candidate] (610.336 µs) : 0, 610
Telemetry [baseline] (7.863 ms) : 0, 7863
Telemetry [candidate] (8.68 ms) : 0, 8680
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (789.311 ms) : 0, 789311
BytebuddyAgent [candidate] (834.655 ms) : 0, 834655
GlobalTracer [baseline] (229.695 ms) : 0, 229695
GlobalTracer [candidate] (228.576 ms) : 0, 228576
IAST [baseline] (22.262 ms) : 0, 22262
IAST [candidate] (22.156 ms) : 0, 22156
AppSec [baseline] (56.006 ms) : 0, 56006
AppSec [candidate] (55.347 ms) : 0, 55347
Debugger [baseline] (5.824 ms) : 0, 5824
Debugger [candidate] (4.066 ms) : 0, 4066
Remote Config [baseline] (589.517 µs) : 0, 590
Remote Config [candidate] (599.655 µs) : 0, 600
Telemetry [baseline] (7.699 ms) : 0, 7699
Telemetry [candidate] (8.498 ms) : 0, 8498
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.49.0-SNAPSHOT~a84184b02cd, baseline=1.49.0-SNAPSHOT~4d01d583bb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.004 s) : 0, 1003829
Total [baseline] (10.428 s) : 0, 10427955
Agent [candidate] (1.055 s) : 0, 1054591
Total [candidate] (10.439 s) : 0, 10439493
section appsec
Agent [baseline] (1.149 s) : 0, 1149015
Total [baseline] (10.631 s) : 0, 10630741
Agent [candidate] (1.201 s) : 0, 1201224
Total [candidate] (10.779 s) : 0, 10778960
section iast
Agent [baseline] (1.135 s) : 0, 1135028
Total [baseline] (10.794 s) : 0, 10793761
Agent [candidate] (1.196 s) : 0, 1195520
Total [candidate] (11.096 s) : 0, 11095633
section profiling
Agent [baseline] (1.259 s) : 0, 1258912
Total [baseline] (10.754 s) : 0, 10754266
Agent [candidate] (1.31 s) : 0, 1309665
Total [candidate] (10.898 s) : 0, 10897772
gantt
title petclinic - break down per module: candidate=1.49.0-SNAPSHOT~a84184b02cd, baseline=1.49.0-SNAPSHOT~4d01d583bb
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (669.806 ms) : 0, 669806
BytebuddyAgent [candidate] (719.798 ms) : 0, 719798
GlobalTracer [baseline] (240.042 ms) : 0, 240042
GlobalTracer [candidate] (239.218 ms) : 0, 239218
AppSec [baseline] (54.508 ms) : 0, 54508
AppSec [candidate] (55.818 ms) : 0, 55818
Debugger [baseline] (6.111 ms) : 0, 6111
Debugger [candidate] (4.34 ms) : 0, 4340
Remote Config [baseline] (706.0 µs) : 0, 706
Remote Config [candidate] (685.944 µs) : 0, 686
Telemetry [baseline] (9.195 ms) : 0, 9195
Telemetry [candidate] (11.412 ms) : 0, 11412
section appsec
BytebuddyAgent [baseline] (688.441 ms) : 0, 688441
BytebuddyAgent [candidate] (741.795 ms) : 0, 741795
GlobalTracer [baseline] (236.106 ms) : 0, 236106
GlobalTracer [candidate] (236.466 ms) : 0, 236466
AppSec [baseline] (174.385 ms) : 0, 174385
AppSec [candidate] (175.112 ms) : 0, 175112
Debugger [baseline] (5.883 ms) : 0, 5883
Debugger [candidate] (4.208 ms) : 0, 4208
Remote Config [baseline] (633.4 µs) : 0, 633
Remote Config [candidate] (631.724 µs) : 0, 632
Telemetry [baseline] (9.316 ms) : 0, 9316
Telemetry [candidate] (8.469 ms) : 0, 8469
IAST [baseline] (21.635 ms) : 0, 21635
IAST [candidate] (22.067 ms) : 0, 22067
section iast
BytebuddyAgent [baseline] (788.503 ms) : 0, 788503
BytebuddyAgent [candidate] (847.382 ms) : 0, 847382
GlobalTracer [baseline] (229.81 ms) : 0, 229810
GlobalTracer [candidate] (231.184 ms) : 0, 231184
AppSec [baseline] (56.433 ms) : 0, 56433
AppSec [candidate] (54.852 ms) : 0, 54852
Debugger [baseline] (5.847 ms) : 0, 5847
Debugger [candidate] (4.281 ms) : 0, 4281
Remote Config [baseline] (580.419 µs) : 0, 580
Remote Config [candidate] (625.125 µs) : 0, 625
Telemetry [baseline] (7.814 ms) : 0, 7814
Telemetry [candidate] (8.864 ms) : 0, 8864
IAST [baseline] (22.702 ms) : 0, 22702
IAST [candidate] (24.782 ms) : 0, 24782
section profiling
BytebuddyAgent [baseline] (665.669 ms) : 0, 665669
BytebuddyAgent [candidate] (715.0 ms) : 0, 715000
GlobalTracer [baseline] (375.71 ms) : 0, 375710
GlobalTracer [candidate] (375.55 ms) : 0, 375550
AppSec [baseline] (54.698 ms) : 0, 54698
AppSec [candidate] (53.262 ms) : 0, 53262
Debugger [baseline] (6.196 ms) : 0, 6196
Debugger [candidate] (4.393 ms) : 0, 4393
Remote Config [baseline] (664.526 µs) : 0, 665
Remote Config [candidate] (685.306 µs) : 0, 685
Telemetry [baseline] (8.266 ms) : 0, 8266
Telemetry [candidate] (8.936 ms) : 0, 8936
ProfilingAgent [baseline] (97.322 ms) : 0, 97322
ProfilingAgent [candidate] (101.253 ms) : 0, 101253
Profiling [baseline] (97.346 ms) : 0, 97346
Profiling [candidate] (101.28 ms) : 0, 101280
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 18 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.49.0-SNAPSHOT~a84184b02cd, baseline=1.49.0-SNAPSHOT~4d01d583bb
dateFormat X
axisFormat %s
section baseline
no_agent (389.823 µs) : 370, 410
. : milestone, 390,
iast (526.246 µs) : 503, 550
. : milestone, 526,
iast_FULL (741.573 µs) : 718, 765
. : milestone, 742,
iast_GLOBAL (576.151 µs) : 553, 599
. : milestone, 576,
iast_HARDCODED_SECRET_DISABLED (532.006 µs) : 509, 555
. : milestone, 532,
iast_INACTIVE (465.015 µs) : 443, 487
. : milestone, 465,
iast_TELEMETRY_OFF (515.449 µs) : 492, 539
. : milestone, 515,
tracing (462.326 µs) : 441, 484
. : milestone, 462,
section candidate
no_agent (380.532 µs) : 361, 400
. : milestone, 381,
iast (517.835 µs) : 496, 540
. : milestone, 518,
iast_FULL (738.229 µs) : 716, 761
. : milestone, 738,
iast_GLOBAL (570.95 µs) : 548, 594
. : milestone, 571,
iast_HARDCODED_SECRET_DISABLED (515.023 µs) : 493, 537
. : milestone, 515,
iast_INACTIVE (472.543 µs) : 451, 494
. : milestone, 473,
iast_TELEMETRY_OFF (502.946 µs) : 481, 524
. : milestone, 503,
tracing (464.957 µs) : 443, 487
. : milestone, 465,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.49.0-SNAPSHOT~a84184b02cd, baseline=1.49.0-SNAPSHOT~4d01d583bb
dateFormat X
axisFormat %s
section baseline
no_agent (1.379 ms) : 1359, 1399
. : milestone, 1379,
appsec (1.738 ms) : 1713, 1763
. : milestone, 1738,
appsec_no_iast (1.744 ms) : 1721, 1768
. : milestone, 1744,
code_origins (1.681 ms) : 1654, 1708
. : milestone, 1681,
iast (1.519 ms) : 1495, 1544
. : milestone, 1519,
profiling (1.525 ms) : 1501, 1548
. : milestone, 1525,
tracing (1.488 ms) : 1464, 1512
. : milestone, 1488,
section candidate
no_agent (1.347 ms) : 1328, 1367
. : milestone, 1347,
appsec (1.744 ms) : 1720, 1767
. : milestone, 1744,
appsec_no_iast (1.754 ms) : 1731, 1777
. : milestone, 1754,
code_origins (1.679 ms) : 1652, 1706
. : milestone, 1679,
iast (1.52 ms) : 1495, 1545
. : milestone, 1520,
profiling (1.523 ms) : 1500, 1546
. : milestone, 1523,
tracing (1.504 ms) : 1479, 1529
. : milestone, 1504,
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.49.0-SNAPSHOT~a84184b02cd, baseline=1.49.0-SNAPSHOT~4d01d583bb
dateFormat X
axisFormat %s
section baseline
no_agent (14.826 s) : 14826000, 14826000
. : milestone, 14826000,
appsec (15.126 s) : 15126000, 15126000
. : milestone, 15126000,
iast (18.958 s) : 18958000, 18958000
. : milestone, 18958000,
iast_GLOBAL (18.059 s) : 18059000, 18059000
. : milestone, 18059000,
profiling (14.786 s) : 14786000, 14786000
. : milestone, 14786000,
tracing (14.848 s) : 14848000, 14848000
. : milestone, 14848000,
section candidate
no_agent (15.428 s) : 15428000, 15428000
. : milestone, 15428000,
appsec (15.076 s) : 15076000, 15076000
. : milestone, 15076000,
iast (18.767 s) : 18767000, 18767000
. : milestone, 18767000,
iast_GLOBAL (18.287 s) : 18287000, 18287000
. : milestone, 18287000,
profiling (15.011 s) : 15011000, 15011000
. : milestone, 15011000,
tracing (14.948 s) : 14948000, 14948000
. : milestone, 14948000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.49.0-SNAPSHOT~a84184b02cd, baseline=1.49.0-SNAPSHOT~4d01d583bb
dateFormat X
axisFormat %s
section baseline
no_agent (1.473 ms) : 1461, 1484
. : milestone, 1473,
appsec (2.369 ms) : 2323, 2416
. : milestone, 2369,
iast (2.155 ms) : 2096, 2214
. : milestone, 2155,
iast_GLOBAL (2.192 ms) : 2133, 2251
. : milestone, 2192,
profiling (2.025 ms) : 1977, 2073
. : milestone, 2025,
tracing (1.978 ms) : 1932, 2023
. : milestone, 1978,
section candidate
no_agent (1.474 ms) : 1462, 1485
. : milestone, 1474,
appsec (2.372 ms) : 2325, 2419
. : milestone, 2372,
iast (2.152 ms) : 2093, 2211
. : milestone, 2152,
iast_GLOBAL (2.2 ms) : 2141, 2260
. : milestone, 2200,
profiling (2.013 ms) : 1964, 2061
. : milestone, 2013,
tracing (1.987 ms) : 1941, 2032
. : milestone, 1987,
|
mhlidd
requested changes
Apr 22, 2025
Comment on lines
-1531
to
-1534
| links = new ArrayList<>(); | ||
| parentContext = null; | ||
| } else if (behaviorExtract == TracePropagationBehaviorExtract.RESTART) { | ||
| links = new ArrayList<>(); |
There was a problem hiding this comment.
One feature in the RFC is that we don't want to include any span links that are created from terminated_context during the extraction phase. The resetting of the links ArrayList was to remove those, since there is a call to addTerminatedContextAsLinks() in buildSpan, prior to the call to buildSpanContext(). Maybe that call should be removed?
There was a problem hiding this comment.
That was only an early try to check how the CI would behave.
Yes, the call from buildSpan went away, the main goal is to handle the terminated context / span link in a single location (buildSpanContext) to avoid multiple allocation.
I keep trying thing in this PR, because I feel the isRemote() behavior on DDSpanContext is wrong compared to its interface behavior. It's only usage what for last parent id tag, which also need improvements / refactoring.
| .build()) | ||
| : SpanLink.from(parentContext); | ||
| links.add(link); | ||
| if (Config.get().getTracePropagationBehaviorExtract() == RESTART) { |
There was a problem hiding this comment.
Suggested change
| if (Config.get().getTracePropagationBehaviorExtract() == RESTART) { | |
| if (Config.get().getTracePropagationBehaviorExtract() == RESTART || Config.get().getTracePropagationBehaviorExtract() == IGNORE) { |
PerfectSlayer
force-pushed
the
bbujon/trace-propagation
branch
from
02351e0 to
5b031c9
Compare
Avoid adding terminated contexts as span links and clearing them if not needed.
PerfectSlayer
force-pushed
the
bbujon/trace-propagation
branch
from
8aeb60d to
cf070a5
Compare
mhlidd
approved these changes
Apr 24, 2025
| if (propagationTags.getLastParentId() == null) { | ||
| if (context.isRemote()) { | ||
| CharSequence lastParentId = context.getLastParentId(); | ||
| if (lastParentId != null) { | ||
| propagationTags.updateLastParentId(lastParentId); | ||
| } | ||
| } else { | ||
| propagationTags.updateLastParentId(DDSpanId.toHexStringPadded(context.getSpanId())); | ||
| } | ||
| } | ||
|
|
||
| propagationTags.updateLastParentId(DDSpanId.toHexStringPadded(context.getSpanId())); |
There was a problem hiding this comment.
What is the reason for removing this here? It feels like to me that the end effect would be different since we are no longer checking for null before doing an update of the parentId?
There was a problem hiding this comment.
Yeah, and the previous behavior feels bogus to me.
We want the last parent id to be the current span id when injected, right?
The last parent id is stored in the root span (of the local pending trace).
So if at some point the last parent id is set, and later on another span of the same local trace we do injection, the last parent id will already be set (to some other span, not the current one) and it won’t be updated.
So with this change, I’m enforcing to always use the current span id as last parent id.
Should I write some test to check for the expected behavior? -- and test it again master and this PR
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
Avoid adding terminated contexts as span links and clearing them if not needed.
Motivation
The recent trace propagation behavior complicated the context propagation and ended up creating span links that can be clear if context is restart.
In addition, it can unexpectedly void customer manually added span links too.
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: LANGPLAT-452