-
Notifications
You must be signed in to change notification settings - Fork 303
Supporting Baggage for Instrumentations used in Weblog Tests #8773
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
...rap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java
Show resolved
Hide resolved
/* Verify whether we have only span contexts or more contexts */ | ||
public ContextScope activateScope(Context context, AgentSpan span) { | ||
Baggage baggage = Baggage.fromContext(context); | ||
if (baggage == null) { |
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.
@mcculls This is what I understood when you mentioned that we can only use span context if it is the only context that was extracted. Let me know if I interpreted that idea correctly here. 😃
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.
This part feels weird to me. Would need to discuss with @mcculls
- It will drop any other context data than baggage (like DSM or other product)
- If there is no data in context, context will be root, and
root.with(span)
is equals tospan
(no allocation, no perf cost) so this will always be save to usecontext.with(span).attach()
nitpick: And on the semantic side, you are not activating a scope, you are activating a context, which activation results in a scope
BenchmarksStartupParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 59 metrics, 11 unstable metrics.
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.50.0-SNAPSHOT~c2124fabec, baseline=1.50.0-SNAPSHOT~0ebd52ba72
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.024 s) : 0, 1023684
Total [baseline] (10.496 s) : 0, 10496471
Agent [candidate] (1.025 s) : 0, 1024518
Total [candidate] (10.483 s) : 0, 10483183
section appsec
Agent [baseline] (1.168 s) : 0, 1168155
Total [baseline] (10.708 s) : 0, 10707905
Agent [candidate] (1.17 s) : 0, 1170243
Total [candidate] (10.667 s) : 0, 10666691
section iast
Agent [baseline] (1.157 s) : 0, 1156750
Total [baseline] (11.016 s) : 0, 11015666
Agent [candidate] (1.15 s) : 0, 1149556
Total [candidate] (10.822 s) : 0, 10822134
section profiling
Agent [baseline] (1.278 s) : 0, 1278255
Total [baseline] (10.914 s) : 0, 10914163
Agent [candidate] (1.276 s) : 0, 1276261
Total [candidate] (10.831 s) : 0, 10831023
gantt
title petclinic - break down per module: candidate=1.50.0-SNAPSHOT~c2124fabec, baseline=1.50.0-SNAPSHOT~0ebd52ba72
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (684.546 ms) : 0, 684546
BytebuddyAgent [candidate] (685.591 ms) : 0, 685591
GlobalTracer [baseline] (241.152 ms) : 0, 241152
GlobalTracer [candidate] (241.008 ms) : 0, 241008
AppSec [baseline] (54.144 ms) : 0, 54144
AppSec [candidate] (54.609 ms) : 0, 54609
Debugger [baseline] (8.932 ms) : 0, 8932
Debugger [candidate] (8.363 ms) : 0, 8363
Remote Config [baseline] (689.665 µs) : 0, 690
Remote Config [candidate] (692.773 µs) : 0, 693
Telemetry [baseline] (10.666 ms) : 0, 10666
Telemetry [candidate] (10.58 ms) : 0, 10580
section appsec
BytebuddyAgent [baseline] (704.959 ms) : 0, 704959
BytebuddyAgent [candidate] (707.144 ms) : 0, 707144
GlobalTracer [baseline] (238.171 ms) : 0, 238171
GlobalTracer [candidate] (238.062 ms) : 0, 238062
AppSec [baseline] (176.364 ms) : 0, 176364
AppSec [candidate] (176.32 ms) : 0, 176320
Debugger [baseline] (5.936 ms) : 0, 5936
Debugger [candidate] (5.948 ms) : 0, 5948
Remote Config [baseline] (620.512 µs) : 0, 621
Remote Config [candidate] (625.286 µs) : 0, 625
Telemetry [baseline] (7.413 ms) : 0, 7413
Telemetry [candidate] (7.432 ms) : 0, 7432
IAST [baseline] (21.882 ms) : 0, 21882
IAST [candidate] (21.844 ms) : 0, 21844
section iast
BytebuddyAgent [baseline] (807.248 ms) : 0, 807248
BytebuddyAgent [candidate] (801.58 ms) : 0, 801580
GlobalTracer [baseline] (232.101 ms) : 0, 232101
GlobalTracer [candidate] (230.48 ms) : 0, 230480
AppSec [baseline] (50.597 ms) : 0, 50597
AppSec [candidate] (53.412 ms) : 0, 53412
Debugger [baseline] (5.98 ms) : 0, 5980
Debugger [candidate] (5.914 ms) : 0, 5914
Remote Config [baseline] (600.409 µs) : 0, 600
Remote Config [candidate] (596.375 µs) : 0, 596
Telemetry [baseline] (7.994 ms) : 0, 7994
Telemetry [candidate] (7.901 ms) : 0, 7901
IAST [baseline] (28.499 ms) : 0, 28499
IAST [candidate] (25.359 ms) : 0, 25359
section profiling
BytebuddyAgent [baseline] (671.585 ms) : 0, 671585
BytebuddyAgent [candidate] (672.733 ms) : 0, 672733
GlobalTracer [baseline] (374.784 ms) : 0, 374784
GlobalTracer [candidate] (373.727 ms) : 0, 373727
AppSec [baseline] (61.948 ms) : 0, 61948
AppSec [candidate] (61.776 ms) : 0, 61776
Debugger [baseline] (6.302 ms) : 0, 6302
Debugger [candidate] (6.203 ms) : 0, 6203
Remote Config [baseline] (673.429 µs) : 0, 673
Remote Config [candidate] (634.815 µs) : 0, 635
Telemetry [baseline] (8.282 ms) : 0, 8282
Telemetry [candidate] (8.151 ms) : 0, 8151
ProfilingAgent [baseline] (103.968 ms) : 0, 103968
ProfilingAgent [candidate] (102.461 ms) : 0, 102461
Profiling [baseline] (103.993 ms) : 0, 103993
Profiling [candidate] (102.487 ms) : 0, 102487
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.50.0-SNAPSHOT~c2124fabec, baseline=1.50.0-SNAPSHOT~0ebd52ba72
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.027 s) : 0, 1027131
Total [baseline] (8.704 s) : 0, 8704265
Agent [candidate] (1.022 s) : 0, 1021941
Total [candidate] (8.636 s) : 0, 8635764
section iast
Agent [baseline] (1.147 s) : 0, 1146743
Total [baseline] (9.209 s) : 0, 9208941
Agent [candidate] (1.151 s) : 0, 1150690
Total [candidate] (9.227 s) : 0, 9226689
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.172 s) : 0, 1172302
Total [baseline] (9.251 s) : 0, 9251012
Agent [candidate] (1.156 s) : 0, 1155921
Total [candidate] (9.215 s) : 0, 9214700
section iast_TELEMETRY_OFF
Agent [baseline] (1.152 s) : 0, 1152378
Total [baseline] (9.192 s) : 0, 9192348
Agent [candidate] (1.148 s) : 0, 1147740
Total [candidate] (9.212 s) : 0, 9212380
gantt
title insecure-bank - break down per module: candidate=1.50.0-SNAPSHOT~c2124fabec, baseline=1.50.0-SNAPSHOT~0ebd52ba72
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (687.117 ms) : 0, 687117
BytebuddyAgent [candidate] (684.204 ms) : 0, 684204
GlobalTracer [baseline] (242.195 ms) : 0, 242195
GlobalTracer [candidate] (240.135 ms) : 0, 240135
AppSec [baseline] (55.696 ms) : 0, 55696
AppSec [candidate] (56.019 ms) : 0, 56019
Debugger [baseline] (9.23 ms) : 0, 9230
Debugger [candidate] (9.05 ms) : 0, 9050
Remote Config [baseline] (706.962 µs) : 0, 707
Remote Config [candidate] (691.116 µs) : 0, 691
Telemetry [baseline] (8.502 ms) : 0, 8502
Telemetry [candidate] (8.245 ms) : 0, 8245
section iast
BytebuddyAgent [baseline] (800.614 ms) : 0, 800614
BytebuddyAgent [candidate] (803.0 ms) : 0, 803000
GlobalTracer [baseline] (230.452 ms) : 0, 230452
GlobalTracer [candidate] (230.806 ms) : 0, 230806
IAST [baseline] (28.009 ms) : 0, 28009
IAST [candidate] (27.597 ms) : 0, 27597
AppSec [baseline] (50.023 ms) : 0, 50023
AppSec [candidate] (51.382 ms) : 0, 51382
Debugger [baseline] (5.861 ms) : 0, 5861
Debugger [candidate] (5.91 ms) : 0, 5910
Remote Config [baseline] (587.582 µs) : 0, 588
Remote Config [candidate] (593.027 µs) : 0, 593
Telemetry [baseline] (7.801 ms) : 0, 7801
Telemetry [candidate] (7.9 ms) : 0, 7900
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (818.899 ms) : 0, 818899
BytebuddyAgent [candidate] (807.025 ms) : 0, 807025
GlobalTracer [baseline] (234.824 ms) : 0, 234824
GlobalTracer [candidate] (231.787 ms) : 0, 231787
IAST [baseline] (29.862 ms) : 0, 29862
IAST [candidate] (28.486 ms) : 0, 28486
AppSec [baseline] (50.08 ms) : 0, 50080
AppSec [candidate] (50.365 ms) : 0, 50365
Debugger [baseline] (6.025 ms) : 0, 6025
Debugger [candidate] (5.974 ms) : 0, 5974
Remote Config [baseline] (610.461 µs) : 0, 610
Remote Config [candidate] (599.906 µs) : 0, 600
Telemetry [baseline] (8.087 ms) : 0, 8087
Telemetry [candidate] (7.997 ms) : 0, 7997
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (804.164 ms) : 0, 804164
BytebuddyAgent [candidate] (801.801 ms) : 0, 801801
GlobalTracer [baseline] (231.389 ms) : 0, 231389
GlobalTracer [candidate] (230.441 ms) : 0, 230441
IAST [baseline] (23.38 ms) : 0, 23380
IAST [candidate] (22.95 ms) : 0, 22950
AppSec [baseline] (55.514 ms) : 0, 55514
AppSec [candidate] (54.77 ms) : 0, 54770
Debugger [baseline] (5.977 ms) : 0, 5977
Debugger [candidate] (5.883 ms) : 0, 5883
Remote Config [baseline] (588.391 µs) : 0, 588
Remote Config [candidate] (601.376 µs) : 0, 601
Telemetry [baseline] (7.777 ms) : 0, 7777
Telemetry [candidate] (7.664 ms) : 0, 7664
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 insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.50.0-SNAPSHOT~c2124fabec, baseline=1.50.0-SNAPSHOT~0ebd52ba72
dateFormat X
axisFormat %s
section baseline
no_agent (373.448 µs) : 354, 393
. : milestone, 373,
iast (520.125 µs) : 497, 543
. : milestone, 520,
iast_FULL (730.906 µs) : 709, 753
. : milestone, 731,
iast_GLOBAL (554.96 µs) : 533, 577
. : milestone, 555,
iast_HARDCODED_SECRET_DISABLED (524.532 µs) : 501, 548
. : milestone, 525,
iast_INACTIVE (469.949 µs) : 447, 493
. : milestone, 470,
iast_TELEMETRY_OFF (503.323 µs) : 480, 527
. : milestone, 503,
tracing (459.178 µs) : 436, 483
. : milestone, 459,
section candidate
no_agent (377.696 µs) : 358, 397
. : milestone, 378,
iast (522.659 µs) : 499, 546
. : milestone, 523,
iast_FULL (730.52 µs) : 707, 754
. : milestone, 731,
iast_GLOBAL (567.607 µs) : 546, 590
. : milestone, 568,
iast_HARDCODED_SECRET_DISABLED (524.768 µs) : 502, 548
. : milestone, 525,
iast_INACTIVE (464.272 µs) : 442, 487
. : milestone, 464,
iast_TELEMETRY_OFF (510.759 µs) : 487, 534
. : milestone, 511,
tracing (457.281 µs) : 435, 479
. : milestone, 457,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.50.0-SNAPSHOT~c2124fabec, baseline=1.50.0-SNAPSHOT~0ebd52ba72
dateFormat X
axisFormat %s
section baseline
no_agent (1.358 ms) : 1339, 1377
. : milestone, 1358,
appsec (1.719 ms) : 1696, 1743
. : milestone, 1719,
appsec_no_iast (1.714 ms) : 1691, 1738
. : milestone, 1714,
code_origins (1.663 ms) : 1636, 1690
. : milestone, 1663,
iast (1.519 ms) : 1494, 1543
. : milestone, 1519,
profiling (1.552 ms) : 1527, 1577
. : milestone, 1552,
tracing (1.502 ms) : 1479, 1525
. : milestone, 1502,
section candidate
no_agent (1.357 ms) : 1337, 1377
. : milestone, 1357,
appsec (1.742 ms) : 1718, 1765
. : milestone, 1742,
appsec_no_iast (1.74 ms) : 1717, 1764
. : milestone, 1740,
code_origins (1.68 ms) : 1652, 1707
. : milestone, 1680,
iast (1.529 ms) : 1505, 1553
. : milestone, 1529,
profiling (1.512 ms) : 1488, 1536
. : milestone, 1512,
tracing (1.492 ms) : 1467, 1517
. : milestone, 1492,
Dacapo |
...rap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java
Show resolved
Hide resolved
/* Verify whether we have only span contexts or more contexts */ | ||
public ContextScope activateScope(Context context, AgentSpan span) { | ||
Baggage baggage = Baggage.fromContext(context); | ||
if (baggage == null) { |
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.
This part feels weird to me. Would need to discuss with @mcculls
- It will drop any other context data than baggage (like DSM or other product)
- If there is no data in context, context will be root, and
root.with(span)
is equals tospan
(no allocation, no perf cost) so this will always be save to usecontext.with(span).attach()
nitpick: And on the semantic side, you are not activating a scope, you are activating a context, which activation results in a scope
...c/main/java/datadog/trace/instrumentation/azure/functions/AzureFunctionsInstrumentation.java
Outdated
Show resolved
Hide resolved
...mcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/TomcatServerInstrumentation.java
Outdated
Show resolved
Hide resolved
e2d8ef7
to
81a9768
Compare
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
50f5b7a
to
4cd49f4
Compare
resolve merge conflicts
5ce53bc
to
e2fb44c
Compare
Datadog Summary✅ Code Quality ✅ Code Security ✅ Dependencies Was this helpful? Give us feedback! |
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.
That's looking good! I only added few nitpicks.
And feel free to add comments about renaming extractContext
to extract
and startSpanFromContext
to startSpan
when refactoring is complete -- this can be done if the following PRs.
...rap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java
Outdated
Show resolved
Hide resolved
...rap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java
Outdated
Show resolved
Hide resolved
...rap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java
Outdated
Show resolved
Hide resolved
@@ -31,7 +32,8 @@ public Response intercept(final Chain chain) throws IOException { | |||
|
|||
final Request.Builder requestBuilder = chain.request().newBuilder(); | |||
DataStreamsContext dsmContext = DataStreamsContext.fromTags(CLIENT_PATHWAY_EDGE_TAGS); | |||
defaultPropagator().inject(span.with(dsmContext), requestBuilder, SETTER); | |||
defaultPropagator() |
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.
FYI: I might refactor this part by introducing an inject
method in helpers.
It will be the mirror operation of extract
, and will allow to deal with the DSM context in a single location rather than all instrumentations.
What Does This Do
This PR migrates context propagation from Span contexts to generic Contexts for the instrumentations used in weblog tests (Tomcat on the server-side and okhttp3 on the client-side). This allows us to verify that we can properly propagate generic contexts end-to-end through System-tests verification.
Additionally, this PR introduces more helper methods in the
HttpServerDecorator
in order to reduce repetitive code in the instrumentation advice layer and ensure consistency among how we handle instrumentations.Motivation
Additional Notes
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any usefull labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]