Clean up DSM context injection #8776
Open
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 3 performance improvements and 2 performance regressions! Performance is the same for 53 metrics, 13 unstable metrics.
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.50.0-SNAPSHOT~9889997c55, baseline=1.50.0-SNAPSHOT~ad6d5fef42
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.019 s) : 0, 1019052
Total [baseline] (10.454 s) : 0, 10453937
Agent [candidate] (1.03 s) : 0, 1030440
Total [candidate] (10.505 s) : 0, 10504825
section appsec
Agent [baseline] (1.172 s) : 0, 1171887
Total [baseline] (10.705 s) : 0, 10704768
Agent [candidate] (1.169 s) : 0, 1169429
Total [candidate] (10.711 s) : 0, 10711069
section iast
Agent [baseline] (1.163 s) : 0, 1163003
Total [baseline] (10.919 s) : 0, 10918904
Agent [candidate] (1.163 s) : 0, 1162833
Total [candidate] (10.872 s) : 0, 10872175
section profiling
Agent [baseline] (1.296 s) : 0, 1296421
Total [baseline] (10.972 s) : 0, 10971661
Agent [candidate] (1.265 s) : 0, 1264590
Total [candidate] (10.822 s) : 0, 10822498
gantt
title petclinic - break down per module: candidate=1.50.0-SNAPSHOT~9889997c55, baseline=1.50.0-SNAPSHOT~ad6d5fef42
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (682.02 ms) : 0, 682020
BytebuddyAgent [candidate] (689.372 ms) : 0, 689372
GlobalTracer [baseline] (240.594 ms) : 0, 240594
GlobalTracer [candidate] (242.934 ms) : 0, 242934
AppSec [baseline] (55.194 ms) : 0, 55194
AppSec [candidate] (55.568 ms) : 0, 55568
Debugger [baseline] (6.908 ms) : 0, 6908
Debugger [candidate] (9.783 ms) : 0, 9783
Remote Config [baseline] (706.45 µs) : 0, 706
Remote Config [candidate] (700.226 µs) : 0, 700
Telemetry [baseline] (10.126 ms) : 0, 10126
Telemetry [candidate] (8.278 ms) : 0, 8278
section appsec
BytebuddyAgent [baseline] (707.632 ms) : 0, 707632
BytebuddyAgent [candidate] (706.369 ms) : 0, 706369
GlobalTracer [baseline] (238.507 ms) : 0, 238507
GlobalTracer [candidate] (238.296 ms) : 0, 238296
AppSec [baseline] (176.499 ms) : 0, 176499
AppSec [candidate] (176.155 ms) : 0, 176155
Debugger [baseline] (5.972 ms) : 0, 5972
Debugger [candidate] (5.976 ms) : 0, 5976
Remote Config [baseline] (622.575 µs) : 0, 623
Remote Config [candidate] (640.838 µs) : 0, 641
Telemetry [baseline] (7.761 ms) : 0, 7761
Telemetry [candidate] (7.434 ms) : 0, 7434
IAST [baseline] (22.271 ms) : 0, 22271
IAST [candidate] (21.714 ms) : 0, 21714
section iast
BytebuddyAgent [baseline] (812.51 ms) : 0, 812510
BytebuddyAgent [candidate] (812.65 ms) : 0, 812650
GlobalTracer [baseline] (232.623 ms) : 0, 232623
GlobalTracer [candidate] (233.123 ms) : 0, 233123
AppSec [baseline] (51.654 ms) : 0, 51654
AppSec [candidate] (48.625 ms) : 0, 48625
Debugger [baseline] (6.013 ms) : 0, 6013
Debugger [candidate] (5.957 ms) : 0, 5957
Remote Config [baseline] (606.892 µs) : 0, 607
Remote Config [candidate] (591.507 µs) : 0, 592
Telemetry [baseline] (8.001 ms) : 0, 8001
Telemetry [candidate] (7.88 ms) : 0, 7880
IAST [baseline] (27.044 ms) : 0, 27044
IAST [candidate] (30.194 ms) : 0, 30194
section profiling
BytebuddyAgent [baseline] (682.99 ms) : 0, 682990
BytebuddyAgent [candidate] (673.457 ms) : 0, 673457
GlobalTracer [baseline] (381.482 ms) : 0, 381482
GlobalTracer [candidate] (359.955 ms) : 0, 359955
AppSec [baseline] (54.864 ms) : 0, 54864
AppSec [candidate] (61.849 ms) : 0, 61849
Debugger [baseline] (6.21 ms) : 0, 6210
Debugger [candidate] (6.255 ms) : 0, 6255
Remote Config [baseline] (653.466 µs) : 0, 653
Remote Config [candidate] (647.55 µs) : 0, 648
Telemetry [baseline] (8.246 ms) : 0, 8246
Telemetry [candidate] (8.192 ms) : 0, 8192
ProfilingAgent [baseline] (110.745 ms) : 0, 110745
ProfilingAgent [candidate] (103.37 ms) : 0, 103370
Profiling [baseline] (110.771 ms) : 0, 110771
Profiling [candidate] (103.394 ms) : 0, 103394
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.50.0-SNAPSHOT~9889997c55, baseline=1.50.0-SNAPSHOT~ad6d5fef42
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.019 s) : 0, 1019004
Total [baseline] (8.646 s) : 0, 8646354
Agent [candidate] (1.025 s) : 0, 1024733
Total [candidate] (8.644 s) : 0, 8643786
section iast
Agent [baseline] (1.148 s) : 0, 1147832
Total [baseline] (9.21 s) : 0, 9209738
Agent [candidate] (1.157 s) : 0, 1156633
Total [candidate] (9.237 s) : 0, 9237157
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.147 s) : 0, 1147020
Total [baseline] (9.207 s) : 0, 9206817
Agent [candidate] (1.166 s) : 0, 1165904
Total [candidate] (9.193 s) : 0, 9193447
section iast_TELEMETRY_OFF
Agent [baseline] (1.152 s) : 0, 1151671
Total [baseline] (9.238 s) : 0, 9238374
Agent [candidate] (1.147 s) : 0, 1146704
Total [candidate] (9.254 s) : 0, 9254048
gantt
title insecure-bank - break down per module: candidate=1.50.0-SNAPSHOT~9889997c55, baseline=1.50.0-SNAPSHOT~ad6d5fef42
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (680.013 ms) : 0, 680013
BytebuddyAgent [candidate] (687.094 ms) : 0, 687094
GlobalTracer [baseline] (238.941 ms) : 0, 238941
GlobalTracer [candidate] (241.191 ms) : 0, 241191
AppSec [baseline] (54.999 ms) : 0, 54999
AppSec [candidate] (54.598 ms) : 0, 54598
Debugger [baseline] (10.43 ms) : 0, 10430
Debugger [candidate] (7.675 ms) : 0, 7675
Remote Config [baseline] (682.979 µs) : 0, 683
Remote Config [candidate] (716.598 µs) : 0, 717
Telemetry [baseline] (10.427 ms) : 0, 10427
Telemetry [candidate] (9.138 ms) : 0, 9138
section iast
BytebuddyAgent [baseline] (801.874 ms) : 0, 801874
BytebuddyAgent [candidate] (806.47 ms) : 0, 806470
GlobalTracer [baseline] (229.959 ms) : 0, 229959
GlobalTracer [candidate] (233.142 ms) : 0, 233142
AppSec [baseline] (48.36 ms) : 0, 48360
AppSec [candidate] (50.552 ms) : 0, 50552
Debugger [baseline] (5.889 ms) : 0, 5889
Debugger [candidate] (5.945 ms) : 0, 5945
Remote Config [baseline] (593.01 µs) : 0, 593
Remote Config [candidate] (591.776 µs) : 0, 592
Telemetry [baseline] (7.866 ms) : 0, 7866
Telemetry [candidate] (7.917 ms) : 0, 7917
IAST [baseline] (29.831 ms) : 0, 29831
IAST [candidate] (28.539 ms) : 0, 28539
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (800.513 ms) : 0, 800513
BytebuddyAgent [candidate] (814.272 ms) : 0, 814272
GlobalTracer [baseline] (230.45 ms) : 0, 230450
GlobalTracer [candidate] (233.605 ms) : 0, 233605
AppSec [baseline] (49.993 ms) : 0, 49993
AppSec [candidate] (50.862 ms) : 0, 50862
Debugger [baseline] (5.856 ms) : 0, 5856
Debugger [candidate] (5.998 ms) : 0, 5998
Remote Config [baseline] (580.239 µs) : 0, 580
Remote Config [candidate] (610.041 µs) : 0, 610
Telemetry [baseline] (7.86 ms) : 0, 7860
Telemetry [candidate] (8.001 ms) : 0, 8001
IAST [baseline] (28.27 ms) : 0, 28270
IAST [candidate] (28.755 ms) : 0, 28755
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (803.674 ms) : 0, 803674
BytebuddyAgent [candidate] (799.079 ms) : 0, 799079
GlobalTracer [baseline] (231.018 ms) : 0, 231018
GlobalTracer [candidate] (231.233 ms) : 0, 231233
AppSec [baseline] (55.578 ms) : 0, 55578
AppSec [candidate] (56.073 ms) : 0, 56073
Debugger [baseline] (5.997 ms) : 0, 5997
Debugger [candidate] (5.971 ms) : 0, 5971
Remote Config [baseline] (611.293 µs) : 0, 611
Remote Config [candidate] (610.606 µs) : 0, 611
Telemetry [baseline] (7.902 ms) : 0, 7902
Telemetry [candidate] (7.87 ms) : 0, 7870
IAST [baseline] (23.201 ms) : 0, 23201
IAST [candidate] (22.294 ms) : 0, 22294
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.50.0-SNAPSHOT~9889997c55, baseline=1.50.0-SNAPSHOT~ad6d5fef42
dateFormat X
axisFormat %s
section baseline
no_agent (1.345 ms) : 1325, 1364
. : milestone, 1345,
appsec (1.733 ms) : 1710, 1756
. : milestone, 1733,
appsec_no_iast (1.728 ms) : 1704, 1751
. : milestone, 1728,
code_origins (1.657 ms) : 1630, 1685
. : milestone, 1657,
iast (1.524 ms) : 1500, 1549
. : milestone, 1524,
profiling (1.512 ms) : 1488, 1536
. : milestone, 1512,
tracing (1.486 ms) : 1462, 1511
. : milestone, 1486,
section candidate
no_agent (1.356 ms) : 1336, 1377
. : milestone, 1356,
appsec (1.722 ms) : 1699, 1746
. : milestone, 1722,
appsec_no_iast (1.736 ms) : 1713, 1760
. : milestone, 1736,
code_origins (1.693 ms) : 1666, 1721
. : milestone, 1693,
iast (1.521 ms) : 1497, 1546
. : milestone, 1521,
profiling (1.508 ms) : 1485, 1531
. : milestone, 1508,
tracing (1.508 ms) : 1483, 1533
. : milestone, 1508,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.50.0-SNAPSHOT~9889997c55, baseline=1.50.0-SNAPSHOT~ad6d5fef42
dateFormat X
axisFormat %s
section baseline
no_agent (377.983 µs) : 358, 398
. : milestone, 378,
iast (513.925 µs) : 492, 536
. : milestone, 514,
iast_FULL (730.111 µs) : 708, 752
. : milestone, 730,
iast_GLOBAL (557.922 µs) : 535, 581
. : milestone, 558,
iast_HARDCODED_SECRET_DISABLED (517.273 µs) : 495, 540
. : milestone, 517,
iast_INACTIVE (467.105 µs) : 444, 490
. : milestone, 467,
iast_TELEMETRY_OFF (517.4 µs) : 494, 541
. : milestone, 517,
tracing (459.916 µs) : 438, 482
. : milestone, 460,
section candidate
no_agent (385.249 µs) : 366, 405
. : milestone, 385,
iast (519.792 µs) : 497, 543
. : milestone, 520,
iast_FULL (730.957 µs) : 709, 753
. : milestone, 731,
iast_GLOBAL (560.426 µs) : 539, 582
. : milestone, 560,
iast_HARDCODED_SECRET_DISABLED (529.605 µs) : 507, 553
. : milestone, 530,
iast_INACTIVE (471.505 µs) : 449, 494
. : milestone, 472,
iast_TELEMETRY_OFF (504.381 µs) : 480, 528
. : milestone, 504,
tracing (454.337 µs) : 432, 477
. : milestone, 454,
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.50.0-SNAPSHOT~9889997c55, baseline=1.50.0-SNAPSHOT~ad6d5fef42
dateFormat X
axisFormat %s
section baseline
no_agent (1.472 ms) : 1460, 1483
. : milestone, 1472,
appsec (2.403 ms) : 2354, 2452
. : milestone, 2403,
iast (2.175 ms) : 2114, 2236
. : milestone, 2175,
iast_GLOBAL (2.221 ms) : 2159, 2283
. : milestone, 2221,
profiling (2.032 ms) : 1982, 2082
. : milestone, 2032,
tracing (2.011 ms) : 1964, 2059
. : milestone, 2011,
section candidate
no_agent (1.477 ms) : 1465, 1488
. : milestone, 1477,
appsec (2.397 ms) : 2348, 2445
. : milestone, 2397,
iast (2.175 ms) : 2114, 2236
. : milestone, 2175,
iast_GLOBAL (2.215 ms) : 2154, 2277
. : milestone, 2215,
profiling (2.029 ms) : 1980, 2079
. : milestone, 2029,
tracing (1.991 ms) : 1944, 2038
. : milestone, 1991,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.50.0-SNAPSHOT~9889997c55, baseline=1.50.0-SNAPSHOT~ad6d5fef42
dateFormat X
axisFormat %s
section baseline
no_agent (15.65 s) : 15650000, 15650000
. : milestone, 15650000,
appsec (14.868 s) : 14868000, 14868000
. : milestone, 14868000,
iast (18.231 s) : 18231000, 18231000
. : milestone, 18231000,
iast_GLOBAL (17.972 s) : 17972000, 17972000
. : milestone, 17972000,
profiling (15.748 s) : 15748000, 15748000
. : milestone, 15748000,
tracing (15.047 s) : 15047000, 15047000
. : milestone, 15047000,
section candidate
no_agent (15.178 s) : 15178000, 15178000
. : milestone, 15178000,
appsec (14.961 s) : 14961000, 14961000
. : milestone, 14961000,
iast (18.755 s) : 18755000, 18755000
. : milestone, 18755000,
iast_GLOBAL (18.12 s) : 18120000, 18120000
. : milestone, 18120000,
profiling (15.03 s) : 15030000, 15030000
. : milestone, 15030000,
tracing (14.916 s) : 14916000, 14916000
. : milestone, 14916000,
|
PerfectSlayer
force-pushed
the
bbujon/context-propagation-dsm
branch
from
5f1247d to
3b7384c
Compare
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| @@ -8,16 +8,47 @@ | |||
| public class DataStreamsContext implements ImplicitContextKeyed { | |||
| private static final ContextKey<DataStreamsContext> CONTEXT_KEY = | |||
| ContextKey.named("dsm-context-key"); | |||
| private static final LinkedHashMap<String, String> CLIENT_PATHWAY_EDGE_TAGS; | |||
| private static final LinkedHashMap<String, String> SERVER_PATHWAY_EDGE_TAGS; | |||
There was a problem hiding this comment.
⚪ Code Quality Violation
Suggested change
| private static final LinkedHashMap<String, String> SERVER_PATHWAY_EDGE_TAGS; | |
| private static final Map<String, String> SERVER_PATHWAY_EDGE_TAGS; |
Avoid using a specific implementation type; use the more general Map instead (...read more)
Relying on particular implementation types, such as, HashSet or LinkedList can limit your adaptability to embrace alternative implementations in the future, particularly as your requirements change and your code needs to undergo changes.
It is recommended to opt for general types such as Set or List when declaring variables and parameters.
There was a problem hiding this comment.
Yeah... We asked the DSM team to create their own data structure but not much update about it 🤷
| @@ -8,16 +8,47 @@ | |||
| public class DataStreamsContext implements ImplicitContextKeyed { | |||
| private static final ContextKey<DataStreamsContext> CONTEXT_KEY = | |||
| ContextKey.named("dsm-context-key"); | |||
| private static final LinkedHashMap<String, String> CLIENT_PATHWAY_EDGE_TAGS; | |||
There was a problem hiding this comment.
⚪ Code Quality Violation
Suggested change
| private static final LinkedHashMap<String, String> CLIENT_PATHWAY_EDGE_TAGS; | |
| private static final Map<String, String> CLIENT_PATHWAY_EDGE_TAGS; |
Avoid using a specific implementation type; use the more general Map instead (...read more)
Relying on particular implementation types, such as, HashSet or LinkedList can limit your adaptability to embrace alternative implementations in the future, particularly as your requirements change and your code needs to undergo changes.
It is recommended to opt for general types such as Set or List when declaring variables and parameters.
There was a problem hiding this comment.
Yeah... We asked the DSM team to create their own data structure but not much update about it 🤷
PerfectSlayer
force-pushed
the
bbujon/context-propagation-dsm
branch
2 times, most recently
from
08c2357 to
08df84e
Compare
PerfectSlayer
force-pushed
the
bbujon/context-propagation-dsm
branch
from
08df84e to
9889997
Compare
mhlidd
reviewed
Jun 11, 2025
| final TraceConfig tracerConfig; | ||
| return (agentSpan = AgentSpan.fromContext(context)) != null | ||
| && (tracerConfig = agentSpan.traceConfig()) != null | ||
| && tracerConfig.isDataStreamsEnabled(); |
There was a problem hiding this comment.
Why do we need to fetch the isDataStreamsEnabled() through tracerConfig? How is this different from getting it directly from Config.get()?
Also this seems to be also added in DataStreamsMonitoring.java. Do we need it in both places or can we remove it here?
There was a problem hiding this comment.
Why do we need to fetch the isDataStreamsEnabled() through tracerConfig? How is this different from getting it directly from Config.get()?
Not sure but I think it might be related to remove config. I would need more details from the DSM team itself. I will ping it from its channel.
Do we need it in both places or can we remove it here?
There is one in decorator to avoid DSM context allocation and new context creation (the context.with() call).
While the one in the propagator is here to make sure we don't propagate if the feature is not enabled.
While the later is product related, the former is for performance concern.
If we can simplify the check, I would not mind having him twice. Let's see what the DSM team think about it first.
| public static final GrpcClientDecorator DECORATE = new GrpcClientDecorator(); | ||
|
|
||
| private static final Set<String> IGNORED_METHODS = Config.get().getGrpcIgnoredOutboundMethods(); | ||
| private static final BitSet CLIENT_ERROR_STATUSES = Config.get().getGrpcClientErrorStatuses(); | ||
| private static final boolean DATA_STREAMS_ENABLED = Config.get().isDataStreamsEnabled(); |
There was a problem hiding this comment.
Same as above, what's the difference between getting the config value from Config.get() vs AgentSpan.tracerConfig()?
There was a problem hiding this comment.
I introduced this guard as performance improvement. Let have the DSM team answer about the config / activation thing.
| @@ -11,6 +17,7 @@ public class RequestDispatcherDecorator extends BaseDecorator { | |||
| UTF8BytesString.create("java-web-servlet-dispatcher"); | |||
| public static final String DD_CONTEXT_PATH_ATTRIBUTE = "datadog.context.path"; | |||
| public static final String DD_SERVLET_PATH_ATTRIBUTE = "datadog.servlet.path"; | |||
| private static final boolean DATA_STREAMS_ENABLED = Config.get().isDataStreamsEnabled(); | |||
Comment on lines
+54
to
+56
| || (dsmContext = DataStreamsContext.fromContext(context)) == null | ||
| || (traceConfig = span.traceConfig()) == null | ||
| || !traceConfig.isDataStreamsEnabled()) { |
There was a problem hiding this comment.
I see this check is the same as the one in UriBasedClientDecorator.java. Is there a need to have it in both places? I would understand that having this check in UriBasedClientDecorator and the 2 GrpcClientDecorator could be repetitive, so could we remove all checks to traceConfig except this one?
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
This PR introduce context injection helper methods for HTTP client and server decorators.
Those new methods now handle the Data Streams context injection too, simplifying the instrumentation themselves.
DSM was also refactor to own its data / logic into its own classes rather than relying on the generic tracing ones.
Context API usage was simplified too.
Motivation
This will simplify instrumentation logic by removing most of the DSM code, unless DSM actually has interesting data to send.
This will also reduce context allocation by not updating context if DSM is not enabled.
DSM refactoring should help with code ownership.
Context refactoring should help to set examples about how to use it.
Additional Notes
Injection was moved to the
UriBasedClientDecoratorrather than theHttpClientDecoratoras it is needed for theUrlConnectionDecoratortoo.I though about reusing
HttpClientDecoratorREQUESTtype for theinjectContext()method and add an abstractsetter()method (similar to theHttpServerDecorator.getter()method) but 1. not all setters are stateless (immutable collection make setters need to update their internal references once a field is set) 2.REQUESTand carrier are not always the same type.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-474