Wrap servlet original PrintWriter on rum injector #9146
Merged
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
amarziali
added
type: enhancement
amarziali
commented
Jul 10, 2025
| } | ||
| } | ||
| } else { | ||
| // use slow path because the length to write is small and within the lookbehind buffer size | ||
| super.write(b, off, len); | ||
| for (int i = off; i < off + len; i++) { |
There was a problem hiding this comment.
@mcculls added the loop not to delegate to super
amarziali
commented
Jul 10, 2025
| } | ||
| } | ||
|
|
||
| private int arrayContains(byte[] array, byte[] search) { | ||
| for (int i = 0; i < array.length - search.length; i++) { | ||
| private int arrayContains(byte[] array, int off, int len, byte[] search) { |
There was a problem hiding this comment.
changed the signature to take into account potential offset and len
amarziali
commented
Jul 10, 2025
| @@ -3,24 +3,6 @@ package datadog.trace.bootstrap.instrumentation.buffer | |||
| import datadog.trace.test.util.DDSpecification | |||
|
|
|||
| class InjectingPipeOutputStreamTest extends DDSpecification { | |||
|
|
|||
| static class ExceptionControlledOutputStream extends FilterOutputStream { | |||
There was a problem hiding this comment.
was a leftover. I need to implement that when I will test the reliability in case of exception (next PR)
amarziali
force-pushed
the
andrea.marziali/printwriter
branch
from
a4d11be to
39fb03d
Compare
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 8 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.52.0-SNAPSHOT~23a97d92c1, baseline=1.52.0-SNAPSHOT~a90ccbcd72
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (994.243 ms) : 0, 994243
Total [baseline] (8.585 s) : 0, 8584746
Agent [candidate] (993.308 ms) : 0, 993308
Total [candidate] (8.535 s) : 0, 8535444
section iast
Agent [baseline] (1.149 s) : 0, 1148701
Total [baseline] (9.298 s) : 0, 9298226
Agent [candidate] (1.133 s) : 0, 1133358
Total [candidate] (9.303 s) : 0, 9303320
gantt
title insecure-bank - break down per module: candidate=1.52.0-SNAPSHOT~23a97d92c1, baseline=1.52.0-SNAPSHOT~a90ccbcd72
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (686.589 ms) : 0, 686589
BytebuddyAgent [candidate] (685.619 ms) : 0, 685619
GlobalTracer [baseline] (241.91 ms) : 0, 241910
GlobalTracer [candidate] (241.784 ms) : 0, 241784
AppSec [baseline] (30.157 ms) : 0, 30157
AppSec [candidate] (30.337 ms) : 0, 30337
Debugger [baseline] (5.944 ms) : 0, 5944
Debugger [candidate] (5.959 ms) : 0, 5959
Remote Config [baseline] (683.968 µs) : 0, 684
Remote Config [candidate] (683.521 µs) : 0, 684
Telemetry [baseline] (8.24 ms) : 0, 8240
Telemetry [candidate] (8.249 ms) : 0, 8249
section iast
BytebuddyAgent [baseline] (820.18 ms) : 0, 820180
BytebuddyAgent [candidate] (808.701 ms) : 0, 808701
GlobalTracer [baseline] (234.361 ms) : 0, 234361
GlobalTracer [candidate] (232.187 ms) : 0, 232187
AppSec [baseline] (31.866 ms) : 0, 31866
AppSec [candidate] (30.361 ms) : 0, 30361
Debugger [baseline] (5.784 ms) : 0, 5784
Debugger [candidate] (5.724 ms) : 0, 5724
Remote Config [baseline] (588.084 µs) : 0, 588
Remote Config [candidate] (568.773 µs) : 0, 569
Telemetry [baseline] (8.021 ms) : 0, 8021
Telemetry [candidate] (7.917 ms) : 0, 7917
IAST [baseline] (26.951 ms) : 0, 26951
IAST [candidate] (27.189 ms) : 0, 27189
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.52.0-SNAPSHOT~23a97d92c1, baseline=1.52.0-SNAPSHOT~a90ccbcd72
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (995.856 ms) : 0, 995856
Total [baseline] (10.579 s) : 0, 10579480
Agent [candidate] (997.956 ms) : 0, 997956
Total [candidate] (10.6 s) : 0, 10600117
section appsec
Agent [baseline] (1.176 s) : 0, 1176420
Total [baseline] (10.742 s) : 0, 10742228
Agent [candidate] (1.175 s) : 0, 1174729
Total [candidate] (10.785 s) : 0, 10785320
section iast
Agent [baseline] (1.133 s) : 0, 1132806
Total [baseline] (10.831 s) : 0, 10831232
Agent [candidate] (1.133 s) : 0, 1132655
Total [candidate] (10.829 s) : 0, 10829056
section profiling
Agent [baseline] (1.246 s) : 0, 1246350
Total [baseline] (11.022 s) : 0, 11022321
Agent [candidate] (1.245 s) : 0, 1245212
Total [candidate] (10.989 s) : 0, 10988652
gantt
title petclinic - break down per module: candidate=1.52.0-SNAPSHOT~23a97d92c1, baseline=1.52.0-SNAPSHOT~a90ccbcd72
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (687.438 ms) : 0, 687438
BytebuddyAgent [candidate] (689.246 ms) : 0, 689246
GlobalTracer [baseline] (242.484 ms) : 0, 242484
GlobalTracer [candidate] (242.589 ms) : 0, 242589
AppSec [baseline] (30.349 ms) : 0, 30349
AppSec [candidate] (30.321 ms) : 0, 30321
Debugger [baseline] (5.937 ms) : 0, 5937
Debugger [candidate] (5.995 ms) : 0, 5995
Remote Config [baseline] (671.72 µs) : 0, 672
Remote Config [candidate] (674.924 µs) : 0, 675
Telemetry [baseline] (8.182 ms) : 0, 8182
Telemetry [candidate] (8.203 ms) : 0, 8203
section appsec
BytebuddyAgent [baseline] (711.296 ms) : 0, 711296
BytebuddyAgent [candidate] (709.448 ms) : 0, 709448
GlobalTracer [baseline] (235.441 ms) : 0, 235441
GlobalTracer [candidate] (235.275 ms) : 0, 235275
IAST [baseline] (23.614 ms) : 0, 23614
IAST [candidate] (23.437 ms) : 0, 23437
AppSec [baseline] (170.882 ms) : 0, 170882
AppSec [candidate] (171.326 ms) : 0, 171326
Debugger [baseline] (5.705 ms) : 0, 5705
Debugger [candidate] (5.729 ms) : 0, 5729
Remote Config [baseline] (599.577 µs) : 0, 600
Remote Config [candidate] (595.835 µs) : 0, 596
Telemetry [baseline] (8.042 ms) : 0, 8042
Telemetry [candidate] (8.081 ms) : 0, 8081
section iast
BytebuddyAgent [baseline] (807.475 ms) : 0, 807475
BytebuddyAgent [candidate] (807.837 ms) : 0, 807837
GlobalTracer [baseline] (232.278 ms) : 0, 232278
GlobalTracer [candidate] (232.188 ms) : 0, 232188
IAST [baseline] (26.508 ms) : 0, 26508
IAST [candidate] (28.791 ms) : 0, 28791
AppSec [baseline] (31.526 ms) : 0, 31526
AppSec [candidate] (28.857 ms) : 0, 28857
Debugger [baseline] (5.76 ms) : 0, 5760
Debugger [candidate] (5.738 ms) : 0, 5738
Remote Config [baseline] (576.757 µs) : 0, 577
Remote Config [candidate] (572.677 µs) : 0, 573
Telemetry [baseline] (7.91 ms) : 0, 7910
Telemetry [candidate] (7.899 ms) : 0, 7899
section profiling
BytebuddyAgent [baseline] (677.125 ms) : 0, 677125
BytebuddyAgent [candidate] (677.965 ms) : 0, 677965
GlobalTracer [baseline] (362.713 ms) : 0, 362713
GlobalTracer [candidate] (361.438 ms) : 0, 361438
AppSec [baseline] (34.026 ms) : 0, 34026
AppSec [candidate] (33.134 ms) : 0, 33134
Debugger [baseline] (9.901 ms) : 0, 9901
Debugger [candidate] (9.132 ms) : 0, 9132
Remote Config [baseline] (674.893 µs) : 0, 675
Remote Config [candidate] (676.494 µs) : 0, 676
Telemetry [baseline] (8.123 ms) : 0, 8123
Telemetry [candidate] (9.507 ms) : 0, 9507
ProfilingAgent [baseline] (105.154 ms) : 0, 105154
ProfilingAgent [candidate] (104.826 ms) : 0, 104826
Profiling [baseline] (105.178 ms) : 0, 105178
Profiling [candidate] (104.849 ms) : 0, 104849
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 1 performance regressions! Performance is the same for 9 metrics, 12 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.52.0-SNAPSHOT~23a97d92c1, baseline=1.52.0-SNAPSHOT~a90ccbcd72
dateFormat X
axisFormat %s
section baseline
no_agent (36.542 ms) : 36252, 36831
. : milestone, 36542,
appsec (47.592 ms) : 47172, 48013
. : milestone, 47592,
code_origins (45.057 ms) : 44683, 45432
. : milestone, 45057,
iast (43.504 ms) : 43120, 43888
. : milestone, 43504,
profiling (47.556 ms) : 47101, 48011
. : milestone, 47556,
tracing (44.408 ms) : 44041, 44774
. : milestone, 44408,
section candidate
no_agent (37.466 ms) : 37165, 37766
. : milestone, 37466,
appsec (46.102 ms) : 45690, 46515
. : milestone, 46102,
code_origins (43.831 ms) : 43457, 44204
. : milestone, 43831,
iast (42.781 ms) : 42412, 43150
. : milestone, 42781,
profiling (50.206 ms) : 49754, 50658
. : milestone, 50206,
tracing (44.616 ms) : 44251, 44981
. : milestone, 44616,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.52.0-SNAPSHOT~23a97d92c1, baseline=1.52.0-SNAPSHOT~a90ccbcd72
dateFormat X
axisFormat %s
section baseline
no_agent (4.363 ms) : 4312, 4413
. : milestone, 4363,
iast (9.608 ms) : 9440, 9777
. : milestone, 9608,
iast_FULL (14.174 ms) : 13897, 14451
. : milestone, 14174,
iast_GLOBAL (10.746 ms) : 10555, 10936
. : milestone, 10746,
profiling (8.466 ms) : 8341, 8591
. : milestone, 8466,
tracing (7.696 ms) : 7579, 7812
. : milestone, 7696,
section candidate
no_agent (4.374 ms) : 4325, 4422
. : milestone, 4374,
iast (9.503 ms) : 9346, 9661
. : milestone, 9503,
iast_FULL (14.439 ms) : 14154, 14724
. : milestone, 14439,
iast_GLOBAL (10.134 ms) : 9962, 10307
. : milestone, 10134,
profiling (8.74 ms) : 8605, 8876
. : milestone, 8740,
tracing (7.781 ms) : 7671, 7891
. : milestone, 7781,
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 biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.52.0-SNAPSHOT~23a97d92c1, baseline=1.52.0-SNAPSHOT~a90ccbcd72
dateFormat X
axisFormat %s
section baseline
no_agent (14.927 s) : 14927000, 14927000
. : milestone, 14927000,
appsec (14.705 s) : 14705000, 14705000
. : milestone, 14705000,
iast (18.276 s) : 18276000, 18276000
. : milestone, 18276000,
iast_GLOBAL (18.081 s) : 18081000, 18081000
. : milestone, 18081000,
profiling (15.361 s) : 15361000, 15361000
. : milestone, 15361000,
tracing (14.885 s) : 14885000, 14885000
. : milestone, 14885000,
section candidate
no_agent (15.054 s) : 15054000, 15054000
. : milestone, 15054000,
appsec (15.098 s) : 15098000, 15098000
. : milestone, 15098000,
iast (18.098 s) : 18098000, 18098000
. : milestone, 18098000,
iast_GLOBAL (18.233 s) : 18233000, 18233000
. : milestone, 18233000,
profiling (15.432 s) : 15432000, 15432000
. : milestone, 15432000,
tracing (15.004 s) : 15004000, 15004000
. : milestone, 15004000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.52.0-SNAPSHOT~23a97d92c1, baseline=1.52.0-SNAPSHOT~a90ccbcd72
dateFormat X
axisFormat %s
section baseline
no_agent (1.478 ms) : 1466, 1490
. : milestone, 1478,
appsec (2.421 ms) : 2371, 2471
. : milestone, 2421,
iast (2.207 ms) : 2144, 2270
. : milestone, 2207,
iast_GLOBAL (2.242 ms) : 2180, 2305
. : milestone, 2242,
profiling (2.047 ms) : 1997, 2098
. : milestone, 2047,
tracing (2.017 ms) : 1969, 2065
. : milestone, 2017,
section candidate
no_agent (1.475 ms) : 1464, 1487
. : milestone, 1475,
appsec (2.418 ms) : 2368, 2468
. : milestone, 2418,
iast (2.203 ms) : 2140, 2265
. : milestone, 2203,
iast_GLOBAL (2.237 ms) : 2175, 2300
. : milestone, 2237,
profiling (2.504 ms) : 2328, 2679
. : milestone, 2504,
tracing (2.015 ms) : 1966, 2064
. : milestone, 2015,
|
mcculls
reviewed
Jul 15, 2025
| * @return The marker chars, {@code null} if RUM injection is disabled. | ||
| */ | ||
| @Nullable | ||
| public char[] getMarker() { |
There was a problem hiding this comment.
Suggested change
| public char[] getMarker() { | |
| public char[] getMarkerChars() { |
just to make clear this returns chars whereas getMarker(String encoding) returns bytes
(you could also change the old method to getMarkerBytes to make it even clearer)
mcculls
reviewed
Jul 15, 2025
| * @return The HTML snippet chars to inject, {@code null} if RUM injection is disabled. | ||
| */ | ||
| @Nullable | ||
| public char[] getSnippet() { |
There was a problem hiding this comment.
Suggested change
| public char[] getSnippet() { | |
| public char[] getSnippetChars() { |
Same reason as below - maybe also consider renaming getSnippet(String encoding) to getSnippetBytes(String encoding)
mcculls
reviewed
Jul 15, 2025
| import java.io.Writer; | ||
|
|
||
| /** | ||
| * An Writer containing a circular buffer with a lookbehind buffer of n bytes. The first time that |
There was a problem hiding this comment.
Suggested change
| * An Writer containing a circular buffer with a lookbehind buffer of n bytes. The first time that | |
| * A Writer containing a circular buffer with a lookbehind buffer of n bytes. The first time that |
mcculls
reviewed
Jul 15, 2025
| } | ||
|
|
||
| @Override | ||
| public void write(int b) throws IOException { |
There was a problem hiding this comment.
Suggested change
| public void write(int b) throws IOException { | |
| public void write(int c) throws IOException { |
matches the Writer javadoc
mcculls
reviewed
Jul 15, 2025
|
|
||
| @Override | ||
| public void flush() throws IOException { | ||
| downstream.flush(); |
There was a problem hiding this comment.
I think we also need this in the updated InjectingPipeOutputStream
There was a problem hiding this comment.
Yes in the outputstream I didn't implement the first time since extending FilterOutputStream but now is not the case anymore thanks
mcculls
approved these changes
Jul 15, 2025
mhlidd
pushed a commit
that referenced
this pull request
Jul 17, 2025
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
In case we inject content for RUM, it wraps the servlet printwriter instead of creating one with the underlying outputstream in order not to skip potential mechanisms beneath HttpServletResponse
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: [PROJ-IDENT]