-
Notifications
You must be signed in to change notification settings - Fork 318
Support propagating OTel API created baggage via outgoing W3C headers #9987
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
This comment has been minimized.
This comment has been minimized.
| .put("FOO","OTEL_UNTOUCHED") | ||
| .put("remove_me_key", "otel_remove_me_value") | ||
| .build() | ||
| .makeCurrent() |
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.
❓ (For my own understanding). I assume that this makeCurrent() invokes our OTelContext instead of the OpenTelemetry Context implementation. How does the makeCurrent() know to use our implementation? 🤔
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.
Note this is Baggage.makeCurrent() :)
Baggage inherits this default method from ImplicitContextKeyed: https://github.com/open-telemetry/opentelemetry-java/blob/main/context/src/main/java/io/opentelemetry/context/ImplicitContextKeyed.java#L33
default Scope makeCurrent() {
return Context.current().with(this).makeCurrent();
}
Notice how it's implemented by getting the current Context (which will be our OtelContext wrapper) to which it adds the new baggage via with (which we'll intercept via our OtelContext wrapper) before activating the resulting context.
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.
Most of this makes sense. The part that I'm confused about is how our OtelContext wrapper gets magically used here instead of the OTel implementation of Context, since I don't see anywhere where we explicitly "set" OtelContext as the global implementation for io.opentelemetry.context.Context.
Hope the question makes sense!
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.
sure, that's done by instrumenting the underlying storage: https://github.com/DataDog/dd-trace-java/blob/master/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/context/OpenTelemetryContextStorageInstrumentation.java#L97
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.
Ahh that makes sense. We instrument the calls made to OTel Context and specify to use our OtelContext instead. Thanks for the explanation! :)
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 58 metrics, 7 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.56.0-SNAPSHOT~9fa02e6d8a, baseline=1.56.0-SNAPSHOT~6721645b9b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.098 s) : 0, 1097560
Total [baseline] (8.864 s) : 0, 8864124
Agent [candidate] (1.1 s) : 0, 1100003
Total [candidate] (8.839 s) : 0, 8838687
section iast
Agent [baseline] (1.239 s) : 0, 1239459
Total [baseline] (9.62 s) : 0, 9619958
Agent [candidate] (1.238 s) : 0, 1238411
Total [candidate] (9.538 s) : 0, 9537972
gantt
title insecure-bank - break down per module: candidate=1.56.0-SNAPSHOT~9fa02e6d8a, baseline=1.56.0-SNAPSHOT~6721645b9b
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.468 ms) : 0, 1468
crashtracking [candidate] (1.474 ms) : 0, 1474
BytebuddyAgent [baseline] (704.877 ms) : 0, 704877
BytebuddyAgent [candidate] (706.192 ms) : 0, 706192
GlobalTracer [baseline] (248.449 ms) : 0, 248449
GlobalTracer [candidate] (249.111 ms) : 0, 249111
AppSec [baseline] (32.227 ms) : 0, 32227
AppSec [candidate] (32.31 ms) : 0, 32310
Debugger [baseline] (63.301 ms) : 0, 63301
Debugger [candidate] (63.566 ms) : 0, 63566
Remote Config [baseline] (628.548 µs) : 0, 629
Remote Config [candidate] (628.011 µs) : 0, 628
Telemetry [baseline] (8.053 ms) : 0, 8053
Telemetry [candidate] (8.073 ms) : 0, 8073
Flare Poller [baseline] (3.636 ms) : 0, 3636
Flare Poller [candidate] (3.686 ms) : 0, 3686
section iast
crashtracking [baseline] (1.468 ms) : 0, 1468
crashtracking [candidate] (1.471 ms) : 0, 1471
BytebuddyAgent [baseline] (829.7 ms) : 0, 829700
BytebuddyAgent [candidate] (831.145 ms) : 0, 831145
GlobalTracer [baseline] (238.468 ms) : 0, 238468
GlobalTracer [candidate] (237.724 ms) : 0, 237724
AppSec [baseline] (33.404 ms) : 0, 33404
AppSec [candidate] (33.373 ms) : 0, 33373
Debugger [baseline] (61.057 ms) : 0, 61057
Debugger [candidate] (59.8 ms) : 0, 59800
Remote Config [baseline] (562.361 µs) : 0, 562
Remote Config [candidate] (540.33 µs) : 0, 540
Telemetry [baseline] (7.734 ms) : 0, 7734
Telemetry [candidate] (7.561 ms) : 0, 7561
Flare Poller [baseline] (3.509 ms) : 0, 3509
Flare Poller [candidate] (3.465 ms) : 0, 3465
IAST [baseline] (28.521 ms) : 0, 28521
IAST [candidate] (28.26 ms) : 0, 28260
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.56.0-SNAPSHOT~9fa02e6d8a, baseline=1.56.0-SNAPSHOT~6721645b9b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.106 s) : 0, 1105849
Total [baseline] (10.863 s) : 0, 10863123
Agent [candidate] (1.101 s) : 0, 1101360
Total [candidate] (10.711 s) : 0, 10710849
section appsec
Agent [baseline] (1.281 s) : 0, 1280563
Total [baseline] (11.007 s) : 0, 11006529
Agent [candidate] (1.287 s) : 0, 1286779
Total [candidate] (11.071 s) : 0, 11070598
section iast
Agent [baseline] (1.235 s) : 0, 1235499
Total [baseline] (11.217 s) : 0, 11217252
Agent [candidate] (1.244 s) : 0, 1243901
Total [candidate] (11.253 s) : 0, 11253003
section profiling
Agent [baseline] (1.233 s) : 0, 1233145
Total [baseline] (11.066 s) : 0, 11065844
Agent [candidate] (1.243 s) : 0, 1243255
Total [candidate] (11.135 s) : 0, 11134592
gantt
title petclinic - break down per module: candidate=1.56.0-SNAPSHOT~9fa02e6d8a, baseline=1.56.0-SNAPSHOT~6721645b9b
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.475 ms) : 0, 1475
crashtracking [candidate] (1.463 ms) : 0, 1463
BytebuddyAgent [baseline] (709.604 ms) : 0, 709604
BytebuddyAgent [candidate] (706.191 ms) : 0, 706191
GlobalTracer [baseline] (250.002 ms) : 0, 250002
GlobalTracer [candidate] (249.336 ms) : 0, 249336
AppSec [baseline] (32.576 ms) : 0, 32576
AppSec [candidate] (32.468 ms) : 0, 32468
Debugger [baseline] (64.625 ms) : 0, 64625
Debugger [candidate] (64.366 ms) : 0, 64366
Remote Config [baseline] (637.91 µs) : 0, 638
Remote Config [candidate] (640.23 µs) : 0, 640
Telemetry [baseline] (8.141 ms) : 0, 8141
Telemetry [candidate] (8.209 ms) : 0, 8209
Flare Poller [baseline] (3.685 ms) : 0, 3685
Flare Poller [candidate] (3.705 ms) : 0, 3705
section appsec
crashtracking [baseline] (1.461 ms) : 0, 1461
crashtracking [candidate] (1.474 ms) : 0, 1474
BytebuddyAgent [baseline] (729.399 ms) : 0, 729399
BytebuddyAgent [candidate] (734.803 ms) : 0, 734803
GlobalTracer [baseline] (240.792 ms) : 0, 240792
GlobalTracer [candidate] (241.66 ms) : 0, 241660
AppSec [baseline] (174.672 ms) : 0, 174672
AppSec [candidate] (174.971 ms) : 0, 174971
Debugger [baseline] (61.424 ms) : 0, 61424
Debugger [candidate] (60.933 ms) : 0, 60933
Remote Config [baseline] (690.163 µs) : 0, 690
Remote Config [candidate] (691.97 µs) : 0, 692
Telemetry [baseline] (8.314 ms) : 0, 8314
Telemetry [candidate] (8.338 ms) : 0, 8338
Flare Poller [baseline] (3.922 ms) : 0, 3922
Flare Poller [candidate] (3.945 ms) : 0, 3945
IAST [baseline] (24.728 ms) : 0, 24728
IAST [candidate] (24.837 ms) : 0, 24837
section iast
crashtracking [baseline] (1.462 ms) : 0, 1462
crashtracking [candidate] (1.473 ms) : 0, 1473
BytebuddyAgent [baseline] (827.279 ms) : 0, 827279
BytebuddyAgent [candidate] (833.798 ms) : 0, 833798
GlobalTracer [baseline] (238.19 ms) : 0, 238190
GlobalTracer [candidate] (239.604 ms) : 0, 239604
AppSec [baseline] (32.334 ms) : 0, 32334
AppSec [candidate] (28.925 ms) : 0, 28925
Debugger [baseline] (60.648 ms) : 0, 60648
Debugger [candidate] (60.713 ms) : 0, 60713
Remote Config [baseline] (563.755 µs) : 0, 564
Remote Config [candidate] (554.85 µs) : 0, 555
Telemetry [baseline] (7.69 ms) : 0, 7690
Telemetry [candidate] (7.716 ms) : 0, 7716
Flare Poller [baseline] (3.486 ms) : 0, 3486
Flare Poller [candidate] (3.507 ms) : 0, 3507
IAST [baseline] (28.876 ms) : 0, 28876
IAST [candidate] (32.621 ms) : 0, 32621
section profiling
crashtracking [baseline] (1.458 ms) : 0, 1458
crashtracking [candidate] (1.473 ms) : 0, 1473
BytebuddyAgent [baseline] (733.351 ms) : 0, 733351
BytebuddyAgent [candidate] (739.366 ms) : 0, 739366
GlobalTracer [baseline] (222.773 ms) : 0, 222773
GlobalTracer [candidate] (224.826 ms) : 0, 224826
AppSec [baseline] (32.468 ms) : 0, 32468
AppSec [candidate] (33.554 ms) : 0, 33554
Debugger [baseline] (63.029 ms) : 0, 63029
Debugger [candidate] (63.117 ms) : 0, 63117
Remote Config [baseline] (717.841 µs) : 0, 718
Remote Config [candidate] (666.95 µs) : 0, 667
Telemetry [baseline] (7.99 ms) : 0, 7990
Telemetry [candidate] (8.164 ms) : 0, 8164
Flare Poller [baseline] (3.84 ms) : 0, 3840
Flare Poller [candidate] (3.826 ms) : 0, 3826
ProfilingAgent [baseline] (97.262 ms) : 0, 97262
ProfilingAgent [candidate] (97.773 ms) : 0, 97773
Profiling [baseline] (97.862 ms) : 0, 97862
Profiling [candidate] (98.358 ms) : 0, 98358
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 18 metrics, 18 unstable metrics. Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~9fa02e6d8a, baseline=1.56.0-SNAPSHOT~6721645b9b
dateFormat X
axisFormat %s
section baseline
no_agent (18.921 ms) : 18731, 19112
. : milestone, 18921,
appsec (18.583 ms) : 18392, 18773
. : milestone, 18583,
code_origins (17.674 ms) : 17503, 17846
. : milestone, 17674,
iast (17.633 ms) : 17455, 17810
. : milestone, 17633,
profiling (20.111 ms) : 19903, 20320
. : milestone, 20111,
tracing (17.426 ms) : 17256, 17596
. : milestone, 17426,
section candidate
no_agent (18.928 ms) : 18733, 19122
. : milestone, 18928,
appsec (18.851 ms) : 18662, 19040
. : milestone, 18851,
code_origins (17.69 ms) : 17513, 17868
. : milestone, 17690,
iast (17.467 ms) : 17295, 17638
. : milestone, 17467,
profiling (19.699 ms) : 19499, 19898
. : milestone, 19699,
tracing (17.59 ms) : 17415, 17765
. : milestone, 17590,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~9fa02e6d8a, baseline=1.56.0-SNAPSHOT~6721645b9b
dateFormat X
axisFormat %s
section baseline
no_agent (1.2 ms) : 1187, 1212
. : milestone, 1200,
iast (3.199 ms) : 3159, 3238
. : milestone, 3199,
iast_FULL (5.769 ms) : 5677, 5861
. : milestone, 5769,
iast_GLOBAL (3.463 ms) : 3411, 3516
. : milestone, 3463,
profiling (2.073 ms) : 2054, 2092
. : milestone, 2073,
tracing (1.786 ms) : 1771, 1801
. : milestone, 1786,
section candidate
no_agent (1.187 ms) : 1176, 1198
. : milestone, 1187,
iast (3.174 ms) : 3131, 3217
. : milestone, 3174,
iast_FULL (5.76 ms) : 5676, 5843
. : milestone, 5760,
iast_GLOBAL (3.542 ms) : 3491, 3592
. : milestone, 3542,
profiling (2.158 ms) : 2139, 2177
. : milestone, 2158,
tracing (1.829 ms) : 1813, 1844
. : milestone, 1829,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 2 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.56.0-SNAPSHOT~9fa02e6d8a, baseline=1.56.0-SNAPSHOT~6721645b9b
dateFormat X
axisFormat %s
section baseline
no_agent (1.472 ms) : 1460, 1483
. : milestone, 1472,
appsec (2.479 ms) : 2426, 2532
. : milestone, 2479,
iast (2.209 ms) : 2145, 2272
. : milestone, 2209,
iast_GLOBAL (2.248 ms) : 2184, 2313
. : milestone, 2248,
profiling (2.475 ms) : 2254, 2697
. : milestone, 2475,
tracing (2.032 ms) : 1982, 2082
. : milestone, 2032,
section candidate
no_agent (1.473 ms) : 1462, 1485
. : milestone, 1473,
appsec (3.703 ms) : 3485, 3922
. : milestone, 3703,
iast (2.199 ms) : 2136, 2262
. : milestone, 2199,
iast_GLOBAL (2.24 ms) : 2176, 2303
. : milestone, 2240,
profiling (2.041 ms) : 1990, 2092
. : milestone, 2041,
tracing (2.016 ms) : 1966, 2065
. : milestone, 2016,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.56.0-SNAPSHOT~9fa02e6d8a, baseline=1.56.0-SNAPSHOT~6721645b9b
dateFormat X
axisFormat %s
section baseline
no_agent (15.454 s) : 15454000, 15454000
. : milestone, 15454000,
appsec (14.915 s) : 14915000, 14915000
. : milestone, 14915000,
iast (18.627 s) : 18627000, 18627000
. : milestone, 18627000,
iast_GLOBAL (17.799 s) : 17799000, 17799000
. : milestone, 17799000,
profiling (14.721 s) : 14721000, 14721000
. : milestone, 14721000,
tracing (14.928 s) : 14928000, 14928000
. : milestone, 14928000,
section candidate
no_agent (14.977 s) : 14977000, 14977000
. : milestone, 14977000,
appsec (14.98 s) : 14980000, 14980000
. : milestone, 14980000,
iast (18.606 s) : 18606000, 18606000
. : milestone, 18606000,
iast_GLOBAL (17.896 s) : 17896000, 17896000
. : milestone, 17896000,
profiling (14.703 s) : 14703000, 14703000
. : milestone, 14703000,
tracing (14.744 s) : 14744000, 14744000
. : milestone, 14744000,
|
mhlidd
left a comment
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.
Thanks for the explanations!
| .put("FOO","OTEL_UNTOUCHED") | ||
| .put("remove_me_key", "otel_remove_me_value") | ||
| .build() | ||
| .makeCurrent() |
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.
Ahh that makes sense. We instrument the calls made to OTel Context and specify to use our OtelContext instead. Thanks for the explanation! :)
PerfectSlayer
left a comment
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. Proposed a possible performance improvement.
| Baggage baggage = Baggage.empty(); | ||
| // transfer baggage to our internal container for propagation purposes | ||
| ((io.opentelemetry.api.baggage.Baggage) value) | ||
| .forEach((k, b) -> baggage.addItem(k, b.getValue())); |
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.
🎯 suggestion: That might be an over optimization but (follow me here), what about implementing a custom Map implementation that projects the Map<String, BaggageEntry> returned by (OTel) Baggage.asMap() into a Map<String, String>?
We can then use (DD) Baggage.create(Map) from it, meaning having zero collection allocation as OTel uses the ReadOnlyArrayMap projection and we can have ours to translate value to their string counter parts.
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.
One hiccup is that projection would be read-only - and our internal Baggage container has mutable operations (addItem / removeItem) that assume a mutable map.
So I'd prefer to do that as part of a refactoring to optimize the internal Baggage API, ideally making it inherently immutable once created/built.
What Does This Do
Counterpart to #9982 - while #9982 supports manipulation of existing W3C baggage using the OpenTelemetry API, this PR supports propagation of purely OpenTelemetry created baggage via outgoing W3C headers.
The approach taken here was to automatically transfer OpenTelemetry baggage on activation into our internal baggage container, because that's what ends up getting checked by the W3C propagator. Our baggage container remains visible via the OpenTelemetry API thanks to #9982
The alternative approach would have been to instrument all origins of OpenTelemetry baggage to instead return our wrappers. This would have involved instrumenting both
Baggage.empty()andBaggageBuilder.build().Motivation
Some users want to create W3C baggage using custom instrumentation.
Additional Notes
Custom BaggageEntryMetadata is not yet used in OpenTelemetry (src) so it is not supported as part of this PR.
Contributor Checklist
type:and (comp:orinst:) labels in addition to any useful labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: APMAPI-1754