Instrument Java Websocket API (JSR356)#8440
Merged
Conversation
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
...et-1.0/src/main/java/datadog/trace/instrumentation/websocket/jsr256/TracingOutputStream.java
Show resolved
Hide resolved
...ebsocket-1.0/src/main/java/datadog/trace/instrumentation/websocket/jsr256/TracingWriter.java
Show resolved
Hide resolved
...trap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/WebsocketDecorator.java
Show resolved
Hide resolved
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 56 metrics, 7 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.48.0-SNAPSHOT~978afb6bc8, baseline=1.48.0-SNAPSHOT~01d3277601
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.049 s) : 0, 1049328
Total [baseline] (10.492 s) : 0, 10492142
Agent [candidate] (1.046 s) : 0, 1046224
Total [candidate] (10.429 s) : 0, 10428702
section appsec
Agent [baseline] (1.188 s) : 0, 1188332
Total [baseline] (10.716 s) : 0, 10715651
Agent [candidate] (1.186 s) : 0, 1185783
Total [candidate] (10.735 s) : 0, 10734597
section iast
Agent [baseline] (1.194 s) : 0, 1193895
Total [baseline] (10.973 s) : 0, 10973498
Agent [candidate] (1.171 s) : 0, 1171062
Total [candidate] (11.078 s) : 0, 11078275
section profiling
Agent [baseline] (1.263 s) : 0, 1262744
Total [baseline] (10.844 s) : 0, 10844474
Agent [candidate] (1.265 s) : 0, 1265263
Total [candidate] (10.81 s) : 0, 10809511
gantt
title petclinic - break down per module: candidate=1.48.0-SNAPSHOT~978afb6bc8, baseline=1.48.0-SNAPSHOT~01d3277601
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (723.37 ms) : 0, 723370
BytebuddyAgent [candidate] (723.217 ms) : 0, 723217
GlobalTracer [baseline] (240.971 ms) : 0, 240971
GlobalTracer [candidate] (239.704 ms) : 0, 239704
AppSec [baseline] (55.333 ms) : 0, 55333
AppSec [candidate] (55.422 ms) : 0, 55422
Remote Config [baseline] (716.565 µs) : 0, 717
Remote Config [candidate] (703.652 µs) : 0, 704
Telemetry [baseline] (13.944 ms) : 0, 13944
Telemetry [candidate] (12.231 ms) : 0, 12231
section appsec
BytebuddyAgent [baseline] (738.995 ms) : 0, 738995
BytebuddyAgent [candidate] (738.375 ms) : 0, 738375
GlobalTracer [baseline] (237.366 ms) : 0, 237366
GlobalTracer [candidate] (236.553 ms) : 0, 236553
AppSec [baseline] (176.939 ms) : 0, 176939
AppSec [candidate] (176.113 ms) : 0, 176113
Remote Config [baseline] (670.97 µs) : 0, 671
Remote Config [candidate] (682.14 µs) : 0, 682
Telemetry [baseline] (8.665 ms) : 0, 8665
Telemetry [candidate] (8.38 ms) : 0, 8380
IAST [baseline] (21.606 ms) : 0, 21606
IAST [candidate] (21.475 ms) : 0, 21475
section iast
BytebuddyAgent [baseline] (855.13 ms) : 0, 855130
BytebuddyAgent [candidate] (837.088 ms) : 0, 837088
GlobalTracer [baseline] (233.431 ms) : 0, 233431
GlobalTracer [candidate] (230.387 ms) : 0, 230387
AppSec [baseline] (57.283 ms) : 0, 57283
AppSec [candidate] (55.888 ms) : 0, 55888
Remote Config [baseline] (629.959 µs) : 0, 630
Remote Config [candidate] (631.118 µs) : 0, 631
Telemetry [baseline] (8.833 ms) : 0, 8833
Telemetry [candidate] (8.768 ms) : 0, 8768
IAST [baseline] (23.365 ms) : 0, 23365
IAST [candidate] (23.383 ms) : 0, 23383
section profiling
ProfilingAgent [baseline] (97.28 ms) : 0, 97280
ProfilingAgent [candidate] (96.65 ms) : 0, 96650
BytebuddyAgent [baseline] (709.767 ms) : 0, 709767
BytebuddyAgent [candidate] (712.628 ms) : 0, 712628
GlobalTracer [baseline] (350.423 ms) : 0, 350423
GlobalTracer [candidate] (351.717 ms) : 0, 351717
AppSec [baseline] (55.1 ms) : 0, 55100
AppSec [candidate] (54.081 ms) : 0, 54081
Remote Config [baseline] (688.02 µs) : 0, 688
Remote Config [candidate] (672.501 µs) : 0, 673
Telemetry [baseline] (9.001 ms) : 0, 9001
Telemetry [candidate] (8.954 ms) : 0, 8954
Profiling [baseline] (97.305 ms) : 0, 97305
Profiling [candidate] (96.677 ms) : 0, 96677
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.48.0-SNAPSHOT~978afb6bc8, baseline=1.48.0-SNAPSHOT~01d3277601
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.04 s) : 0, 1039824
Total [baseline] (8.662 s) : 0, 8661913
Agent [candidate] (1.046 s) : 0, 1046273
Total [candidate] (8.697 s) : 0, 8696938
section iast
Agent [baseline] (1.173 s) : 0, 1172608
Total [baseline] (9.221 s) : 0, 9220718
Agent [candidate] (1.177 s) : 0, 1176620
Total [candidate] (9.26 s) : 0, 9259802
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.175 s) : 0, 1174887
Total [baseline] (9.221 s) : 0, 9221237
Agent [candidate] (1.173 s) : 0, 1173015
Total [candidate] (9.152 s) : 0, 9151745
section iast_TELEMETRY_OFF
Agent [baseline] (1.176 s) : 0, 1176153
Total [baseline] (9.239 s) : 0, 9238611
Agent [candidate] (1.176 s) : 0, 1175518
Total [candidate] (9.257 s) : 0, 9256836
gantt
title insecure-bank - break down per module: candidate=1.48.0-SNAPSHOT~978afb6bc8, baseline=1.48.0-SNAPSHOT~01d3277601
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (717.941 ms) : 0, 717941
BytebuddyAgent [candidate] (722.869 ms) : 0, 722869
GlobalTracer [baseline] (239.692 ms) : 0, 239692
GlobalTracer [candidate] (239.184 ms) : 0, 239184
AppSec [baseline] (55.373 ms) : 0, 55373
AppSec [candidate] (55.074 ms) : 0, 55074
Remote Config [baseline] (700.15 µs) : 0, 700
Remote Config [candidate] (696.743 µs) : 0, 697
Telemetry [baseline] (11.331 ms) : 0, 11331
Telemetry [candidate] (13.523 ms) : 0, 13523
section iast
BytebuddyAgent [baseline] (838.149 ms) : 0, 838149
BytebuddyAgent [candidate] (841.495 ms) : 0, 841495
GlobalTracer [baseline] (230.583 ms) : 0, 230583
GlobalTracer [candidate] (231.352 ms) : 0, 231352
IAST [baseline] (22.82 ms) : 0, 22820
IAST [candidate] (23.568 ms) : 0, 23568
AppSec [baseline] (56.804 ms) : 0, 56804
AppSec [candidate] (56.015 ms) : 0, 56015
Remote Config [baseline] (616.007 µs) : 0, 616
Remote Config [candidate] (625.884 µs) : 0, 626
Telemetry [baseline] (8.726 ms) : 0, 8726
Telemetry [candidate] (8.716 ms) : 0, 8716
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (837.443 ms) : 0, 837443
BytebuddyAgent [candidate] (838.654 ms) : 0, 838654
GlobalTracer [baseline] (232.264 ms) : 0, 232264
GlobalTracer [candidate] (230.689 ms) : 0, 230689
IAST [baseline] (23.353 ms) : 0, 23353
IAST [candidate] (22.966 ms) : 0, 22966
AppSec [baseline] (57.444 ms) : 0, 57444
AppSec [candidate] (56.396 ms) : 0, 56396
Remote Config [baseline] (631.919 µs) : 0, 632
Remote Config [candidate] (605.039 µs) : 0, 605
Telemetry [baseline] (8.88 ms) : 0, 8880
Telemetry [candidate] (8.71 ms) : 0, 8710
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (840.273 ms) : 0, 840273
BytebuddyAgent [candidate] (840.591 ms) : 0, 840591
GlobalTracer [baseline] (231.724 ms) : 0, 231724
GlobalTracer [candidate] (231.041 ms) : 0, 231041
IAST [baseline] (22.767 ms) : 0, 22767
IAST [candidate] (22.552 ms) : 0, 22552
AppSec [baseline] (56.928 ms) : 0, 56928
AppSec [candidate] (56.998 ms) : 0, 56998
Remote Config [baseline] (634.544 µs) : 0, 635
Remote Config [candidate] (632.776 µs) : 0, 633
Telemetry [baseline] (8.722 ms) : 0, 8722
Telemetry [candidate] (8.68 ms) : 0, 8680
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 14 metrics, 16 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.48.0-SNAPSHOT~978afb6bc8, baseline=1.48.0-SNAPSHOT~01d3277601
dateFormat X
axisFormat %s
section baseline
no_agent (382.209 µs) : 362, 402
. : milestone, 382,
iast (512.257 µs) : 490, 534
. : milestone, 512,
iast_FULL (728.815 µs) : 707, 751
. : milestone, 729,
iast_GLOBAL (552.886 µs) : 531, 575
. : milestone, 553,
iast_HARDCODED_SECRET_DISABLED (512.166 µs) : 490, 534
. : milestone, 512,
iast_INACTIVE (458.394 µs) : 438, 479
. : milestone, 458,
iast_TELEMETRY_OFF (494.238 µs) : 472, 517
. : milestone, 494,
tracing (460.098 µs) : 439, 481
. : milestone, 460,
section candidate
no_agent (384.529 µs) : 365, 405
. : milestone, 385,
iast (512.229 µs) : 490, 534
. : milestone, 512,
iast_FULL (729.069 µs) : 707, 751
. : milestone, 729,
iast_GLOBAL (551.716 µs) : 530, 573
. : milestone, 552,
iast_HARDCODED_SECRET_DISABLED (516.784 µs) : 494, 539
. : milestone, 517,
iast_INACTIVE (458.071 µs) : 437, 479
. : milestone, 458,
iast_TELEMETRY_OFF (497.255 µs) : 476, 519
. : milestone, 497,
tracing (461.793 µs) : 441, 483
. : milestone, 462,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.48.0-SNAPSHOT~978afb6bc8, baseline=1.48.0-SNAPSHOT~01d3277601
dateFormat X
axisFormat %s
section baseline
no_agent (1.355 ms) : 1335, 1376
. : milestone, 1355,
appsec (1.733 ms) : 1710, 1757
. : milestone, 1733,
appsec_no_iast (1.746 ms) : 1720, 1771
. : milestone, 1746,
code_origins (1.69 ms) : 1658, 1723
. : milestone, 1690,
iast (1.5 ms) : 1476, 1525
. : milestone, 1500,
profiling (1.496 ms) : 1472, 1520
. : milestone, 1496,
tracing (1.508 ms) : 1483, 1533
. : milestone, 1508,
section candidate
no_agent (1.34 ms) : 1320, 1360
. : milestone, 1340,
appsec (1.731 ms) : 1708, 1755
. : milestone, 1731,
appsec_no_iast (1.742 ms) : 1716, 1767
. : milestone, 1742,
code_origins (1.673 ms) : 1640, 1707
. : milestone, 1673,
iast (1.514 ms) : 1490, 1538
. : milestone, 1514,
profiling (1.505 ms) : 1481, 1529
. : milestone, 1505,
tracing (1.486 ms) : 1461, 1512
. : milestone, 1486,
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.48.0-SNAPSHOT~978afb6bc8, baseline=1.48.0-SNAPSHOT~01d3277601
dateFormat X
axisFormat %s
section baseline
no_agent (1.478 ms) : 1466, 1489
. : milestone, 1478,
appsec (2.355 ms) : 2311, 2399
. : milestone, 2355,
iast (2.126 ms) : 2070, 2182
. : milestone, 2126,
iast_GLOBAL (2.167 ms) : 2111, 2223
. : milestone, 2167,
profiling (1.98 ms) : 1937, 2024
. : milestone, 1980,
tracing (1.953 ms) : 1911, 1996
. : milestone, 1953,
section candidate
no_agent (1.476 ms) : 1465, 1488
. : milestone, 1476,
appsec (2.354 ms) : 2310, 2398
. : milestone, 2354,
iast (2.128 ms) : 2071, 2184
. : milestone, 2128,
iast_GLOBAL (2.169 ms) : 2113, 2225
. : milestone, 2169,
profiling (1.989 ms) : 1944, 2035
. : milestone, 1989,
tracing (1.954 ms) : 1911, 1996
. : milestone, 1954,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.48.0-SNAPSHOT~978afb6bc8, baseline=1.48.0-SNAPSHOT~01d3277601
dateFormat X
axisFormat %s
section baseline
no_agent (14.883 s) : 14883000, 14883000
. : milestone, 14883000,
appsec (14.939 s) : 14939000, 14939000
. : milestone, 14939000,
iast (18.908 s) : 18908000, 18908000
. : milestone, 18908000,
iast_GLOBAL (18.104 s) : 18104000, 18104000
. : milestone, 18104000,
profiling (15.626 s) : 15626000, 15626000
. : milestone, 15626000,
tracing (15.004 s) : 15004000, 15004000
. : milestone, 15004000,
section candidate
no_agent (14.893 s) : 14893000, 14893000
. : milestone, 14893000,
appsec (15.027 s) : 15027000, 15027000
. : milestone, 15027000,
iast (18.564 s) : 18564000, 18564000
. : milestone, 18564000,
iast_GLOBAL (18.04 s) : 18040000, 18040000
. : milestone, 18040000,
profiling (15.011 s) : 15011000, 15011000
. : milestone, 15011000,
tracing (15.115 s) : 15115000, 15115000
. : milestone, 15115000,
|
amarziali
force-pushed
the
andrea.marziali/jsr356
branch
4 times, most recently
from
b017c7b to
52d9691
Compare
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/LinksAssert.groovy
Outdated
Show resolved
Hide resolved
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/LinksAssert.groovy
Show resolved
Hide resolved
Contributor
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
There are a lot of changes in this PR 😅
I will have a closer look the public and internal API later this week.
amarziali
force-pushed
the
andrea.marziali/jsr356
branch
from
542304a to
ab80d28
Compare
Contributor
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
Reviewed the API part and left some comments :)
Let me know if you need help with the OTel part.
| private final Set<AgentSpanLink> assertedLinks = [] | ||
|
|
||
| private LinksAssert(DDSpan span) { | ||
| this.links = span.links // this is class protected but for the moment groovy can access it |
Contributor
There was a problem hiding this comment.
Can it be left private? Can’t groovy access them when private too?
Contributor
Author
There was a problem hiding this comment.
Now I remember why. It does not work: (see that CI run)
It fails for
Caused by: groovy.lang.MissingPropertyException: No such property: links for class: datadog.trace.core.DDSpan$SpockMock$863262640
because that span it's got spied and then cannot be accessed if private. Any advice for it?
Contributor
There was a problem hiding this comment.
Maybe just leave it as a comment with this reason.
| private volatile int longRunningVersion = 0; | ||
|
|
||
| private final List<AgentSpanLink> links; | ||
| protected final List<AgentSpanLink> links; |
Contributor
There was a problem hiding this comment.
Can it stay private? -- see related comment in SpanLinkTest
internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanLink.java
Outdated
Show resolved
Hide resolved
| assertTraces(1) { | ||
| trace(1) { | ||
| span { | ||
| ignoreSpanLinks() // check is done on the content of the tag below |
Contributor
There was a problem hiding this comment.
Do you need some help porting those checks?
Contributor
Author
There was a problem hiding this comment.
Thanks for proposing I don't refuse it :) I did this way mostly because here we wanted to check the serialized form so I just decided to skip them. Would you like to add the links part on this test?
ygree
reviewed
Mar 4, 2025
dd-java-agent/instrumentation/aws-java-s3-2.0/src/test/groovy/S3ClientTest.groovy
Show resolved
Hide resolved
ygree
reviewed
Mar 4, 2025
dd-java-agent/instrumentation/aws-java-s3-2.0/src/test/groovy/S3ClientTest.groovy
Show resolved
Hide resolved
ygree
reviewed
Mar 4, 2025
...n/jetty-9/src/main/java_jetty10/datadog/trace/instrumentation/jetty10/OnCompletedAdvice.java
Outdated
Show resolved
Hide resolved
ygree
reviewed
Mar 4, 2025
| assert span.tags.containsKey(DDTags.SPAN_LINKS) | ||
| assert span.tags[DDTags.SPAN_LINKS] != null | ||
| links({ | ||
| link(DDTraceId.from((long)12052652441736835200), (long)-6394091631972716416) |
Contributor
Author
There was a problem hiding this comment.
I don't know where they come from 🤷 That trace ID is hardcoded somwhere so I had to hardcode it as well
ygree
reviewed
Mar 4, 2025
...6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy
Outdated
Show resolved
Hide resolved
ygree
reviewed
Mar 4, 2025
ygree
reviewed
Mar 4, 2025
ygree
reviewed
Mar 4, 2025
...trap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/WebsocketDecorator.java
Show resolved
Hide resolved
ygree
reviewed
Mar 4, 2025
...trap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/WebsocketDecorator.java
Show resolved
Hide resolved
ygree
reviewed
Mar 4, 2025
...ootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/websocket/HandlerContext.java
Show resolved
Hide resolved
ygree
reviewed
Mar 4, 2025
...ootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/websocket/HandlerContext.java
Show resolved
Hide resolved
ygree
requested changes
Mar 4, 2025
amarziali
force-pushed
the
andrea.marziali/jsr356
branch
2 times, most recently
from
cbd26e7 to
99e400b
Compare
amarziali
force-pushed
the
andrea.marziali/jsr356
branch
2 times, most recently
from
a947aef to
700b1c3
Compare
| return s; | ||
| } | ||
| final CharSequence ret = s.subSequence(idx + 1, s.length()); | ||
| if (ret.length() == 0) { |
Contributor
There was a problem hiding this comment.
Can this case happen if idx == s.length() - 1 is already filtered above?
Contributor
Author
There was a problem hiding this comment.
yep right I can simplify
| import jakarta.websocket.server.ServerEndpointConfig | ||
| import java.nio.ByteBuffer | ||
|
|
||
| class WebsocketTest extends AgentTestRunner { |
Contributor
There was a problem hiding this comment.
Thanks for excluding random data from test case name! 🙇
PerfectSlayer
approved these changes
Mar 7, 2025
Contributor
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
Looking good from the platform / api side! 🙌
amarziali
force-pushed
the
andrea.marziali/jsr356
branch
from
700b1c3 to
978afb6
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
Instrumenter module for
javax.websocketandjakarta.websocketAPIs. Supports both client and server API.Provides spans for:
websocket.send)websocket.receive)websocket.close)Tested with
Limitations
Endpointclass but direct calls annotated methods viaMethodHandler. An additional work is still needed to support themMotivation
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: AIDM-535