-
Couldn't load subscription status.
- Fork 315
Implement Rum injection for servlet 3 #9102
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
| * | ||
| * @return The HTML snippet to inject, {@code null} if RUM injection is disabled to inject. | ||
| */ | ||
| public static byte[] getSnippet(String encoding) { |
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.
@PerfectSlayer I changed the signature of those since the injector works with raw bytes. I put in place a reasonable cache per encoding
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.
Sounds good to me! 👍
.../src/main/java/datadog/trace/bootstrap/instrumentation/buffer/InjectingPipeOutputStream.java
Show resolved
Hide resolved
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 44 metrics, 9 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.51.0-SNAPSHOT~486ee57a40, baseline=1.51.0-SNAPSHOT~9ad6afd361
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (996.075 ms) : 0, 996075
Total [baseline] (10.631 s) : 0, 10631105
Agent [candidate] (993.568 ms) : 0, 993568
Total [candidate] (10.624 s) : 0, 10623947
section appsec
Agent [baseline] (1.174 s) : 0, 1174269
Total [baseline] (10.831 s) : 0, 10830735
Agent [candidate] (1.176 s) : 0, 1175620
Total [candidate] (10.716 s) : 0, 10715628
section iast
Agent [baseline] (1.141 s) : 0, 1141334
Total [baseline] (10.994 s) : 0, 10994386
Agent [candidate] (1.137 s) : 0, 1137330
Total [candidate] (10.839 s) : 0, 10838638
section profiling
Agent [baseline] (1.247 s) : 0, 1247230
Total [baseline] (10.917 s) : 0, 10916573
Agent [candidate] (1.254 s) : 0, 1253795
Total [candidate] (11.004 s) : 0, 11003935
gantt
title petclinic - break down per module: candidate=1.51.0-SNAPSHOT~486ee57a40, baseline=1.51.0-SNAPSHOT~9ad6afd361
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (687.12 ms) : 0, 687120
BytebuddyAgent [candidate] (686.254 ms) : 0, 686254
GlobalTracer [baseline] (242.452 ms) : 0, 242452
GlobalTracer [candidate] (241.577 ms) : 0, 241577
AppSec [baseline] (30.262 ms) : 0, 30262
AppSec [candidate] (30.105 ms) : 0, 30105
Debugger [baseline] (6.03 ms) : 0, 6030
Debugger [candidate] (6.034 ms) : 0, 6034
Remote Config [baseline] (684.935 µs) : 0, 685
Remote Config [candidate] (680.909 µs) : 0, 681
Telemetry [baseline] (8.214 ms) : 0, 8214
Telemetry [candidate] (8.19 ms) : 0, 8190
section appsec
BytebuddyAgent [baseline] (710.017 ms) : 0, 710017
BytebuddyAgent [candidate] (709.944 ms) : 0, 709944
GlobalTracer [baseline] (234.895 ms) : 0, 234895
GlobalTracer [candidate] (235.666 ms) : 0, 235666
AppSec [baseline] (170.348 ms) : 0, 170348
AppSec [candidate] (171.505 ms) : 0, 171505
Debugger [baseline] (5.797 ms) : 0, 5797
Debugger [candidate] (5.751 ms) : 0, 5751
Remote Config [baseline] (603.285 µs) : 0, 603
Remote Config [candidate] (593.677 µs) : 0, 594
Telemetry [baseline] (8.134 ms) : 0, 8134
Telemetry [candidate] (8.076 ms) : 0, 8076
IAST [baseline] (23.059 ms) : 0, 23059
IAST [candidate] (23.31 ms) : 0, 23310
section iast
BytebuddyAgent [baseline] (813.175 ms) : 0, 813175
BytebuddyAgent [candidate] (809.637 ms) : 0, 809637
GlobalTracer [baseline] (233.912 ms) : 0, 233912
GlobalTracer [candidate] (234.35 ms) : 0, 234350
AppSec [baseline] (30.164 ms) : 0, 30164
AppSec [candidate] (28.838 ms) : 0, 28838
Debugger [baseline] (5.891 ms) : 0, 5891
Debugger [candidate] (5.9 ms) : 0, 5900
Remote Config [baseline] (604.628 µs) : 0, 605
Remote Config [candidate] (596.392 µs) : 0, 596
Telemetry [baseline] (8.072 ms) : 0, 8072
Telemetry [candidate] (8.191 ms) : 0, 8191
IAST [baseline] (27.943 ms) : 0, 27943
IAST [candidate] (28.187 ms) : 0, 28187
section profiling
ProfilingAgent [baseline] (102.954 ms) : 0, 102954
ProfilingAgent [candidate] (105.35 ms) : 0, 105350
BytebuddyAgent [baseline] (681.49 ms) : 0, 681490
BytebuddyAgent [candidate] (682.255 ms) : 0, 682255
GlobalTracer [baseline] (360.633 ms) : 0, 360633
GlobalTracer [candidate] (363.278 ms) : 0, 363278
AppSec [baseline] (31.417 ms) : 0, 31417
AppSec [candidate] (32.634 ms) : 0, 32634
Debugger [baseline] (12.845 ms) : 0, 12845
Debugger [candidate] (11.057 ms) : 0, 11057
Remote Config [baseline] (664.525 µs) : 0, 665
Remote Config [candidate] (669.383 µs) : 0, 669
Telemetry [baseline] (7.985 ms) : 0, 7985
Telemetry [candidate] (8.858 ms) : 0, 8858
Profiling [baseline] (102.978 ms) : 0, 102978
Profiling [candidate] (105.375 ms) : 0, 105375
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.51.0-SNAPSHOT~486ee57a40, baseline=1.51.0-SNAPSHOT~9ad6afd361
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.001 s) : 0, 1001322
Total [baseline] (8.564 s) : 0, 8564242
Agent [candidate] (995.317 ms) : 0, 995317
Total [candidate] (8.565 s) : 0, 8565436
section iast
Agent [baseline] (1.132 s) : 0, 1132255
Total [baseline] (9.261 s) : 0, 9260604
Agent [candidate] (1.132 s) : 0, 1132320
Total [candidate] (9.283 s) : 0, 9282676
gantt
title insecure-bank - break down per module: candidate=1.51.0-SNAPSHOT~486ee57a40, baseline=1.51.0-SNAPSHOT~9ad6afd361
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (692.648 ms) : 0, 692648
BytebuddyAgent [candidate] (687.155 ms) : 0, 687155
GlobalTracer [baseline] (241.438 ms) : 0, 241438
GlobalTracer [candidate] (242.045 ms) : 0, 242045
AppSec [baseline] (30.126 ms) : 0, 30126
AppSec [candidate] (30.351 ms) : 0, 30351
Debugger [baseline] (6.061 ms) : 0, 6061
Debugger [candidate] (6.038 ms) : 0, 6038
Remote Config [baseline] (679.915 µs) : 0, 680
Remote Config [candidate] (676.326 µs) : 0, 676
Telemetry [baseline] (8.934 ms) : 0, 8934
Telemetry [candidate] (8.236 ms) : 0, 8236
section iast
BytebuddyAgent [baseline] (806.419 ms) : 0, 806419
BytebuddyAgent [candidate] (807.44 ms) : 0, 807440
GlobalTracer [baseline] (232.375 ms) : 0, 232375
GlobalTracer [candidate] (232.557 ms) : 0, 232557
AppSec [baseline] (29.857 ms) : 0, 29857
AppSec [candidate] (29.325 ms) : 0, 29325
Debugger [baseline] (6.59 ms) : 0, 6590
Debugger [candidate] (5.84 ms) : 0, 5840
Remote Config [baseline] (579.464 µs) : 0, 579
Remote Config [candidate] (587.116 µs) : 0, 587
Telemetry [baseline] (8.02 ms) : 0, 8020
Telemetry [candidate] (7.994 ms) : 0, 7994
IAST [baseline] (26.974 ms) : 0, 26974
IAST [candidate] (27.815 ms) : 0, 27815
LoadParameters
See matching parameters
SummaryFound 3 performance improvements and 2 performance regressions! Performance is the same for 7 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.51.0-SNAPSHOT~486ee57a40, baseline=1.51.0-SNAPSHOT~9ad6afd361
dateFormat X
axisFormat %s
section baseline
no_agent (4.425 ms) : 4375, 4474
. : milestone, 4425,
iast (10.001 ms) : 9834, 10167
. : milestone, 10001,
iast_FULL (14.171 ms) : 13888, 14455
. : milestone, 14171,
iast_GLOBAL (10.678 ms) : 10480, 10875
. : milestone, 10678,
profiling (8.894 ms) : 8751, 9037
. : milestone, 8894,
tracing (7.545 ms) : 7436, 7653
. : milestone, 7545,
section candidate
no_agent (4.315 ms) : 4258, 4373
. : milestone, 4315,
iast (9.332 ms) : 9180, 9483
. : milestone, 9332,
iast_FULL (14.39 ms) : 14105, 14675
. : milestone, 14390,
iast_GLOBAL (10.516 ms) : 10332, 10700
. : milestone, 10516,
profiling (8.747 ms) : 8615, 8879
. : milestone, 8747,
tracing (8.04 ms) : 7922, 8157
. : milestone, 8040,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.51.0-SNAPSHOT~486ee57a40, baseline=1.51.0-SNAPSHOT~9ad6afd361
dateFormat X
axisFormat %s
section baseline
no_agent (38.402 ms) : 38090, 38714
. : milestone, 38402,
appsec (49.163 ms) : 48727, 49599
. : milestone, 49163,
code_origins (47.372 ms) : 46967, 47778
. : milestone, 47372,
iast (45.562 ms) : 45160, 45964
. : milestone, 45562,
profiling (47.112 ms) : 46653, 47572
. : milestone, 47112,
tracing (44.357 ms) : 44000, 44714
. : milestone, 44357,
section candidate
no_agent (38.063 ms) : 37745, 38382
. : milestone, 38063,
appsec (46.194 ms) : 45804, 46585
. : milestone, 46194,
code_origins (45.759 ms) : 45341, 46176
. : milestone, 45759,
iast (45.436 ms) : 45033, 45839
. : milestone, 45436,
profiling (49.325 ms) : 48870, 49779
. : milestone, 49325,
tracing (44.214 ms) : 43841, 44588
. : milestone, 44214,
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.51.0-SNAPSHOT~486ee57a40, baseline=1.51.0-SNAPSHOT~9ad6afd361
dateFormat X
axisFormat %s
section baseline
no_agent (1.468 ms) : 1457, 1480
. : milestone, 1468,
appsec (2.397 ms) : 2348, 2446
. : milestone, 2397,
iast (2.193 ms) : 2130, 2255
. : milestone, 2193,
iast_GLOBAL (2.233 ms) : 2170, 2295
. : milestone, 2233,
profiling (2.029 ms) : 1979, 2079
. : milestone, 2029,
tracing (2.013 ms) : 1964, 2061
. : milestone, 2013,
section candidate
no_agent (1.471 ms) : 1459, 1482
. : milestone, 1471,
appsec (2.404 ms) : 2354, 2453
. : milestone, 2404,
iast (2.197 ms) : 2135, 2260
. : milestone, 2197,
iast_GLOBAL (2.237 ms) : 2175, 2300
. : milestone, 2237,
profiling (2.04 ms) : 1990, 2090
. : milestone, 2040,
tracing (2.003 ms) : 1955, 2051
. : milestone, 2003,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.51.0-SNAPSHOT~486ee57a40, baseline=1.51.0-SNAPSHOT~9ad6afd361
dateFormat X
axisFormat %s
section baseline
no_agent (15.494 s) : 15494000, 15494000
. : milestone, 15494000,
appsec (14.608 s) : 14608000, 14608000
. : milestone, 14608000,
iast (18.811 s) : 18811000, 18811000
. : milestone, 18811000,
iast_GLOBAL (18.106 s) : 18106000, 18106000
. : milestone, 18106000,
profiling (15.162 s) : 15162000, 15162000
. : milestone, 15162000,
tracing (14.833 s) : 14833000, 14833000
. : milestone, 14833000,
section candidate
no_agent (14.864 s) : 14864000, 14864000
. : milestone, 14864000,
appsec (14.88 s) : 14880000, 14880000
. : milestone, 14880000,
iast (18.586 s) : 18586000, 18586000
. : milestone, 18586000,
iast_GLOBAL (18.012 s) : 18012000, 18012000
. : milestone, 18012000,
profiling (15.204 s) : 15204000, 15204000
. : milestone, 15204000,
tracing (14.933 s) : 14933000, 14933000
. : milestone, 14933000,
|
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.
Looking good! 👍
Let's continue in project branch from now on 🤝
| private final byte[] contentToInject; | ||
| private boolean found = false; | ||
| private int matchingPos = 0; | ||
| private final Consumer<Void> onContentInjected; |
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.
| private final Consumer<Void> onContentInjected; | |
| private final Runnable onContentInjected; |
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.
Good catch! I will change it (need to change the test as well)
| if (outputStream == null) { | ||
| String encoding = getCharacterEncoding(); | ||
| if (encoding == null) { | ||
| encoding = "UTF-8"; |
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.
Is there a reason to use UTF-8 as default?
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.
right. perhaps using the platform default is better
| if (type != null && type.contains("html")) { | ||
| shouldInject = true; | ||
| } |
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.
Would you like to re-introduce my ContentTypeCache from: https://github.com/DataDog/inject-browser-sdk-backup/blob/main/injector_java_servlet/datadog-servlet-config/src/main/java/com/datadog/rum/browser/ContentTypesCache.java
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.
I'm wondering if that simple logic is just enough? Otherwise we can reintroduce it. I've no strong opinions
| * | ||
| * @return The HTML snippet to inject, {@code null} if RUM injection is disabled to inject. | ||
| */ | ||
| public static byte[] getSnippet(String encoding) { |
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.
Sounds good to me! 👍
What Does This Do
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]