Enabling baggage cache to support limits and non-ascii characters #8713
+141
−39
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 66 metrics, 5 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.49.0-SNAPSHOT~a1e5ffbd50, baseline=1.49.0-SNAPSHOT~5a04e8c591
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.006 s) : 0, 1006084
Total [baseline] (8.631 s) : 0, 8630771
Agent [candidate] (1.014 s) : 0, 1014375
Total [candidate] (8.647 s) : 0, 8646802
section iast
Agent [baseline] (1.135 s) : 0, 1134687
Total [baseline] (9.192 s) : 0, 9191899
Agent [candidate] (1.134 s) : 0, 1134352
Total [candidate] (9.203 s) : 0, 9202617
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.138 s) : 0, 1137599
Total [baseline] (9.248 s) : 0, 9247956
Agent [candidate] (1.142 s) : 0, 1141623
Total [candidate] (9.198 s) : 0, 9198465
section iast_TELEMETRY_OFF
Agent [baseline] (1.136 s) : 0, 1135650
Total [baseline] (9.212 s) : 0, 9211549
Agent [candidate] (1.14 s) : 0, 1140403
Total [candidate] (9.216 s) : 0, 9216496
gantt
title insecure-bank - break down per module: candidate=1.49.0-SNAPSHOT~a1e5ffbd50, baseline=1.49.0-SNAPSHOT~5a04e8c591
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (671.593 ms) : 0, 671593
BytebuddyAgent [candidate] (674.529 ms) : 0, 674529
GlobalTracer [baseline] (240.297 ms) : 0, 240297
GlobalTracer [candidate] (241.56 ms) : 0, 241560
AppSec [baseline] (54.66 ms) : 0, 54660
AppSec [candidate] (54.887 ms) : 0, 54887
Debugger [baseline] (6.868 ms) : 0, 6868
Debugger [candidate] (7.765 ms) : 0, 7765
Remote Config [baseline] (696.039 µs) : 0, 696
Remote Config [candidate] (707.368 µs) : 0, 707
Telemetry [baseline] (8.438 ms) : 0, 8438
Telemetry [candidate] (11.381 ms) : 0, 11381
section iast
BytebuddyAgent [baseline] (788.209 ms) : 0, 788209
BytebuddyAgent [candidate] (788.721 ms) : 0, 788721
GlobalTracer [baseline] (229.435 ms) : 0, 229435
GlobalTracer [candidate] (229.175 ms) : 0, 229175
IAST [baseline] (22.718 ms) : 0, 22718
IAST [candidate] (22.645 ms) : 0, 22645
AppSec [baseline] (56.553 ms) : 0, 56553
AppSec [candidate] (56.209 ms) : 0, 56209
Debugger [baseline] (5.903 ms) : 0, 5903
Debugger [candidate] (5.858 ms) : 0, 5858
Remote Config [baseline] (608.108 µs) : 0, 608
Remote Config [candidate] (597.761 µs) : 0, 598
Telemetry [baseline] (7.881 ms) : 0, 7881
Telemetry [candidate] (7.819 ms) : 0, 7819
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (790.455 ms) : 0, 790455
BytebuddyAgent [candidate] (792.16 ms) : 0, 792160
GlobalTracer [baseline] (229.508 ms) : 0, 229508
GlobalTracer [candidate] (231.676 ms) : 0, 231676
IAST [baseline] (22.765 ms) : 0, 22765
IAST [candidate] (23.015 ms) : 0, 23015
AppSec [baseline] (56.997 ms) : 0, 56997
AppSec [candidate] (56.806 ms) : 0, 56806
Debugger [baseline] (5.938 ms) : 0, 5938
Debugger [candidate] (5.936 ms) : 0, 5936
Remote Config [baseline] (604.01 µs) : 0, 604
Remote Config [candidate] (595.298 µs) : 0, 595
Telemetry [baseline] (7.996 ms) : 0, 7996
Telemetry [candidate] (7.951 ms) : 0, 7951
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (787.744 ms) : 0, 787744
BytebuddyAgent [candidate] (792.483 ms) : 0, 792483
GlobalTracer [baseline] (231.011 ms) : 0, 231011
GlobalTracer [candidate] (230.922 ms) : 0, 230922
IAST [baseline] (22.557 ms) : 0, 22557
IAST [candidate] (22.344 ms) : 0, 22344
AppSec [baseline] (56.587 ms) : 0, 56587
AppSec [candidate] (56.814 ms) : 0, 56814
Debugger [baseline] (5.986 ms) : 0, 5986
Debugger [candidate] (5.98 ms) : 0, 5980
Remote Config [baseline] (603.155 µs) : 0, 603
Remote Config [candidate] (602.813 µs) : 0, 603
Telemetry [baseline] (7.766 ms) : 0, 7766
Telemetry [candidate] (7.781 ms) : 0, 7781
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.49.0-SNAPSHOT~a1e5ffbd50, baseline=1.49.0-SNAPSHOT~5a04e8c591
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.013 s) : 0, 1013049
Total [baseline] (10.485 s) : 0, 10485415
Agent [candidate] (1.015 s) : 0, 1015301
Total [candidate] (10.515 s) : 0, 10515289
section appsec
Agent [baseline] (1.148 s) : 0, 1148188
Total [baseline] (10.626 s) : 0, 10626058
Agent [candidate] (1.149 s) : 0, 1148664
Total [candidate] (10.675 s) : 0, 10674596
section iast
Agent [baseline] (1.144 s) : 0, 1144274
Total [baseline] (10.887 s) : 0, 10886942
Agent [candidate] (1.14 s) : 0, 1139640
Total [candidate] (10.876 s) : 0, 10875500
section profiling
Agent [baseline] (1.255 s) : 0, 1255404
Total [baseline] (10.837 s) : 0, 10837245
Agent [candidate] (1.264 s) : 0, 1263579
Total [candidate] (10.799 s) : 0, 10798875
gantt
title petclinic - break down per module: candidate=1.49.0-SNAPSHOT~a1e5ffbd50, baseline=1.49.0-SNAPSHOT~5a04e8c591
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (675.19 ms) : 0, 675190
BytebuddyAgent [candidate] (677.193 ms) : 0, 677193
GlobalTracer [baseline] (240.858 ms) : 0, 240858
GlobalTracer [candidate] (241.807 ms) : 0, 241807
AppSec [baseline] (55.057 ms) : 0, 55057
AppSec [candidate] (55.085 ms) : 0, 55085
Debugger [baseline] (6.208 ms) : 0, 6208
Debugger [candidate] (6.212 ms) : 0, 6212
Remote Config [baseline] (704.373 µs) : 0, 704
Remote Config [candidate] (706.689 µs) : 0, 707
Telemetry [baseline] (11.489 ms) : 0, 11489
Telemetry [candidate] (10.743 ms) : 0, 10743
section appsec
BytebuddyAgent [baseline] (688.142 ms) : 0, 688142
BytebuddyAgent [candidate] (688.098 ms) : 0, 688098
GlobalTracer [baseline] (236.141 ms) : 0, 236141
GlobalTracer [candidate] (236.199 ms) : 0, 236199
AppSec [baseline] (175.075 ms) : 0, 175075
AppSec [candidate] (175.78 ms) : 0, 175780
Debugger [baseline] (5.836 ms) : 0, 5836
Debugger [candidate] (5.886 ms) : 0, 5886
Remote Config [baseline] (630.453 µs) : 0, 630
Remote Config [candidate] (635.499 µs) : 0, 635
Telemetry [baseline] (8.109 ms) : 0, 8109
Telemetry [candidate] (7.792 ms) : 0, 7792
IAST [baseline] (21.749 ms) : 0, 21749
IAST [candidate] (21.837 ms) : 0, 21837
section iast
BytebuddyAgent [baseline] (795.513 ms) : 0, 795513
BytebuddyAgent [candidate] (791.918 ms) : 0, 791918
GlobalTracer [baseline] (231.175 ms) : 0, 231175
GlobalTracer [candidate] (230.431 ms) : 0, 230431
AppSec [baseline] (56.591 ms) : 0, 56591
AppSec [candidate] (56.558 ms) : 0, 56558
Debugger [baseline] (5.916 ms) : 0, 5916
Debugger [candidate] (5.874 ms) : 0, 5874
Remote Config [baseline] (598.338 µs) : 0, 598
Remote Config [candidate] (594.521 µs) : 0, 595
Telemetry [baseline] (7.975 ms) : 0, 7975
Telemetry [candidate] (8.044 ms) : 0, 8044
IAST [baseline] (22.928 ms) : 0, 22928
IAST [candidate] (22.805 ms) : 0, 22805
section profiling
BytebuddyAgent [baseline] (661.353 ms) : 0, 661353
BytebuddyAgent [candidate] (666.872 ms) : 0, 666872
GlobalTracer [baseline] (377.95 ms) : 0, 377950
GlobalTracer [candidate] (379.562 ms) : 0, 379562
AppSec [baseline] (54.637 ms) : 0, 54637
AppSec [candidate] (54.056 ms) : 0, 54056
Debugger [baseline] (6.141 ms) : 0, 6141
Debugger [candidate] (6.183 ms) : 0, 6183
Remote Config [baseline] (670.977 µs) : 0, 671
Remote Config [candidate] (649.368 µs) : 0, 649
Telemetry [baseline] (8.145 ms) : 0, 8145
Telemetry [candidate] (8.226 ms) : 0, 8226
ProfilingAgent [baseline] (96.408 ms) : 0, 96408
ProfilingAgent [candidate] (96.982 ms) : 0, 96982
Profiling [baseline] (96.432 ms) : 0, 96432
Profiling [candidate] (97.007 ms) : 0, 97007
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 petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.49.0-SNAPSHOT~a1e5ffbd50, baseline=1.49.0-SNAPSHOT~5a04e8c591
dateFormat X
axisFormat %s
section baseline
no_agent (1.351 ms) : 1330, 1371
. : milestone, 1351,
appsec (1.707 ms) : 1683, 1730
. : milestone, 1707,
appsec_no_iast (1.703 ms) : 1679, 1727
. : milestone, 1703,
code_origins (1.661 ms) : 1635, 1688
. : milestone, 1661,
iast (1.508 ms) : 1485, 1532
. : milestone, 1508,
profiling (1.505 ms) : 1482, 1529
. : milestone, 1505,
tracing (1.477 ms) : 1452, 1501
. : milestone, 1477,
section candidate
no_agent (1.346 ms) : 1326, 1366
. : milestone, 1346,
appsec (1.742 ms) : 1719, 1765
. : milestone, 1742,
appsec_no_iast (1.749 ms) : 1725, 1772
. : milestone, 1749,
code_origins (1.676 ms) : 1650, 1703
. : milestone, 1676,
iast (1.511 ms) : 1487, 1535
. : milestone, 1511,
profiling (1.504 ms) : 1480, 1527
. : milestone, 1504,
tracing (1.483 ms) : 1458, 1508
. : milestone, 1483,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.49.0-SNAPSHOT~a1e5ffbd50, baseline=1.49.0-SNAPSHOT~5a04e8c591
dateFormat X
axisFormat %s
section baseline
no_agent (377.892 µs) : 358, 398
. : milestone, 378,
iast (523.743 µs) : 501, 547
. : milestone, 524,
iast_FULL (739.047 µs) : 716, 762
. : milestone, 739,
iast_GLOBAL (570.042 µs) : 547, 593
. : milestone, 570,
iast_HARDCODED_SECRET_DISABLED (521.825 µs) : 499, 544
. : milestone, 522,
iast_INACTIVE (470.399 µs) : 447, 494
. : milestone, 470,
iast_TELEMETRY_OFF (510.111 µs) : 486, 534
. : milestone, 510,
tracing (457.658 µs) : 436, 480
. : milestone, 458,
section candidate
no_agent (381.601 µs) : 361, 402
. : milestone, 382,
iast (516.144 µs) : 493, 539
. : milestone, 516,
iast_FULL (731.67 µs) : 708, 755
. : milestone, 732,
iast_GLOBAL (566.406 µs) : 543, 590
. : milestone, 566,
iast_HARDCODED_SECRET_DISABLED (515.993 µs) : 493, 539
. : milestone, 516,
iast_INACTIVE (464.876 µs) : 442, 487
. : milestone, 465,
iast_TELEMETRY_OFF (512.491 µs) : 489, 536
. : milestone, 512,
tracing (459.372 µs) : 437, 482
. : milestone, 459,
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 tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.49.0-SNAPSHOT~a1e5ffbd50, baseline=1.49.0-SNAPSHOT~5a04e8c591
dateFormat X
axisFormat %s
section baseline
no_agent (1.476 ms) : 1464, 1487
. : milestone, 1476,
appsec (2.382 ms) : 2335, 2429
. : milestone, 2382,
iast (2.154 ms) : 2095, 2213
. : milestone, 2154,
iast_GLOBAL (2.208 ms) : 2148, 2268
. : milestone, 2208,
profiling (2.037 ms) : 1987, 2086
. : milestone, 2037,
tracing (1.978 ms) : 1933, 2024
. : milestone, 1978,
section candidate
no_agent (1.473 ms) : 1461, 1484
. : milestone, 1473,
appsec (2.367 ms) : 2320, 2414
. : milestone, 2367,
iast (2.166 ms) : 2107, 2226
. : milestone, 2166,
iast_GLOBAL (2.198 ms) : 2138, 2258
. : milestone, 2198,
profiling (2.014 ms) : 1966, 2061
. : milestone, 2014,
tracing (1.987 ms) : 1941, 2033
. : milestone, 1987,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.49.0-SNAPSHOT~a1e5ffbd50, baseline=1.49.0-SNAPSHOT~5a04e8c591
dateFormat X
axisFormat %s
section baseline
no_agent (14.951 s) : 14951000, 14951000
. : milestone, 14951000,
appsec (14.912 s) : 14912000, 14912000
. : milestone, 14912000,
iast (19.251 s) : 19251000, 19251000
. : milestone, 19251000,
iast_GLOBAL (18.242 s) : 18242000, 18242000
. : milestone, 18242000,
profiling (15.058 s) : 15058000, 15058000
. : milestone, 15058000,
tracing (14.908 s) : 14908000, 14908000
. : milestone, 14908000,
section candidate
no_agent (15.641 s) : 15641000, 15641000
. : milestone, 15641000,
appsec (15.027 s) : 15027000, 15027000
. : milestone, 15027000,
iast (18.514 s) : 18514000, 18514000
. : milestone, 18514000,
iast_GLOBAL (18.129 s) : 18129000, 18129000
. : milestone, 18129000,
profiling (14.944 s) : 14944000, 14944000
. : milestone, 14944000,
tracing (14.907 s) : 14907000, 14907000
. : milestone, 14907000,
|
mhlidd
changed the title
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
mhlidd
commented
Apr 23, 2025
Comment on lines
125
to
136
| public boolean keyNeedsEncoding(String key) { | ||
| int slen = key.length(); | ||
| for (int index = 0; index < slen; index++) { | ||
| char c = key.charAt(index); | ||
| if (needsEncoding(c, unsafeKeyOctets)) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| public boolean valNeedsEncoding(String val) { |
There was a problem hiding this comment.
I included both of these to avoid checking which unsafe characters to use for each string we decode. Let me know if that is preferred for code-conciseness
There was a problem hiding this comment.
No that’s good 👍
But you can make it more concise if you like by still having the public boolean keyNeedsEncoding(String key) and public boolean valNeedsEncoding(String val) that calls an private method public boolean needsEncoding(String s, boolean[] unsafeOctets) and do all the logic / iteration on this single method (the length, for, charAt, etc...).
|
I had a quick look Today but will complete the review Tomorrow. |
No I have not, I will look into that today! |
Temporarily, the baggage propagator benchmarks are included here! |
| } else{ | ||
| this.w3cHeader = input.substring(0, start - 1); // -1 to ignore the k/v separator | ||
| } | ||
| truncatedCache = true; |
There was a problem hiding this comment.
You should break here if you reach max size or max bytes.
It will simply the if clause too.
There was a problem hiding this comment.
Even if we don't cache here, we still need to maintain all of the raw baggage data so we can't break.
| @@ -152,11 +161,29 @@ private Map<String, String> parseBaggageHeaders(String input) { | |||
| } | |||
| baggage.put(key, value); | |||
|
|
|||
| // need to percent-encode non-ascii headers we pass down | |||
| if (UTF_ESCAPER.keyNeedsEncoding(key) || UTF_ESCAPER.valNeedsEncoding(value)) { | |||
| truncatedCache = true; | |||
There was a problem hiding this comment.
It’s not strictly truncated, it’s invalid in this case as non ascii characters are not allowed in HTTP headers. I though the RFC asked to drop the header and not parse it if it was invalid.
From RFC encoding:
In other words, the HTTP baggage header is encoded using only ASCII characters.
From RFC error handling:
When this occurs, the APM SDK should not try to extract any values from the entire header. In other words, APM SDKs should ignore the header and not try to extract "good" values while ignoring "bad" ones.
There was a problem hiding this comment.
I would argue that this isn't an invalid case and should be something that we can support since HTTP clients technically can send non-ascii characters as headers and we do support non-ascii characters as baggage. I would say that the invalid case is more for parsing issues with not being able to figure out what baggage should be used.
Comment on lines
125
to
136
| public boolean keyNeedsEncoding(String key) { | ||
| int slen = key.length(); | ||
| for (int index = 0; index < slen; index++) { | ||
| char c = key.charAt(index); | ||
| if (needsEncoding(c, unsafeKeyOctets)) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| public boolean valNeedsEncoding(String val) { |
There was a problem hiding this comment.
No that’s good 👍
But you can make it more concise if you like by still having the public boolean keyNeedsEncoding(String key) and public boolean valNeedsEncoding(String val) that calls an private method public boolean needsEncoding(String s, boolean[] unsafeOctets) and do all the logic / iteration on this single method (the length, for, charAt, etc...).
PerfectSlayer
approved these changes
May 2, 2025
Thanks! This can be merged whenever we verify through our benchmarking that this will not introduce major performance regressions. 😅 |
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
Previously, the Baggage Cache implemented injected incoming headers if no modification to baggage was made while ignoring limits. In order to be in compliance with limits, this PR adds support to truncate the baggage cache if limits are violated. Furthermore, Java does not support sending non-ascii characters in http headers. This PR also forcefully builds a outgoing baggage header if the incoming header includes non-encoded non-ascii characters by disabling the cache when extracting such headers.
Motivation
Additional Notes
Verified through unit tests and system-tests in
test_baggage.py.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]