-
Notifications
You must be signed in to change notification settings - Fork 314
Do not allocate a tracing context when not fully extracted #9693
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
🎯 Code Coverage 🔗 Commit SHA: 66a1ee4 | Docs | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 2 performance improvements and 0 performance regressions! Performance is the same for 60 metrics, 3 unstable metrics.
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.55.0-SNAPSHOT~66a1ee4867, baseline=1.55.0-SNAPSHOT~425276dceb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.017 s) : 0, 1017401
Total [baseline] (8.738 s) : 0, 8737741
Agent [candidate] (1.015 s) : 0, 1015311
Total [candidate] (8.659 s) : 0, 8658912
section iast
Agent [baseline] (1.154 s) : 0, 1154097
Total [baseline] (9.281 s) : 0, 9280848
Agent [candidate] (1.148 s) : 0, 1147975
Total [candidate] (9.261 s) : 0, 9260826
gantt
title insecure-bank - break down per module: candidate=1.55.0-SNAPSHOT~66a1ee4867, baseline=1.55.0-SNAPSHOT~425276dceb
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.479 ms) : 0, 1479
crashtracking [candidate] (1.458 ms) : 0, 1458
BytebuddyAgent [baseline] (692.282 ms) : 0, 692282
BytebuddyAgent [candidate] (691.646 ms) : 0, 691646
GlobalTracer [baseline] (242.264 ms) : 0, 242264
GlobalTracer [candidate] (241.184 ms) : 0, 241184
AppSec [baseline] (32.685 ms) : 0, 32685
AppSec [candidate] (32.703 ms) : 0, 32703
Debugger [baseline] (6.474 ms) : 0, 6474
Debugger [candidate] (6.399 ms) : 0, 6399
Remote Config [baseline] (711.566 µs) : 0, 712
Remote Config [candidate] (704.348 µs) : 0, 704
Telemetry [baseline] (9.301 ms) : 0, 9301
Telemetry [candidate] (9.137 ms) : 0, 9137
Flare Poller [baseline] (11.062 ms) : 0, 11062
Flare Poller [candidate] (10.981 ms) : 0, 10981
section iast
crashtracking [baseline] (1.505 ms) : 0, 1505
crashtracking [candidate] (1.448 ms) : 0, 1448
BytebuddyAgent [baseline] (818.936 ms) : 0, 818936
BytebuddyAgent [candidate] (813.122 ms) : 0, 813122
GlobalTracer [baseline] (230.998 ms) : 0, 230998
GlobalTracer [candidate] (230.877 ms) : 0, 230877
IAST [baseline] (26.365 ms) : 0, 26365
IAST [candidate] (26.253 ms) : 0, 26253
AppSec [baseline] (35.034 ms) : 0, 35034
AppSec [candidate] (35.781 ms) : 0, 35781
Debugger [baseline] (6.123 ms) : 0, 6123
Debugger [candidate] (6.086 ms) : 0, 6086
Remote Config [baseline] (627.906 µs) : 0, 628
Remote Config [candidate] (609.345 µs) : 0, 609
Telemetry [baseline] (8.709 ms) : 0, 8709
Telemetry [candidate] (8.465 ms) : 0, 8465
Flare Poller [baseline] (4.207 ms) : 0, 4207
Flare Poller [candidate] (4.123 ms) : 0, 4123
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.55.0-SNAPSHOT~66a1ee4867, baseline=1.55.0-SNAPSHOT~425276dceb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.023 s) : 0, 1022988
Total [baseline] (10.654 s) : 0, 10654203
Agent [candidate] (1.022 s) : 0, 1021610
Total [candidate] (10.801 s) : 0, 10800521
section appsec
Agent [baseline] (1.195 s) : 0, 1194868
Total [baseline] (11.128 s) : 0, 11128211
Agent [candidate] (1.217 s) : 0, 1217391
Total [candidate] (11.096 s) : 0, 11096107
section iast
Agent [baseline] (1.157 s) : 0, 1157214
Total [baseline] (11.113 s) : 0, 11113165
Agent [candidate] (1.152 s) : 0, 1151797
Total [candidate] (10.91 s) : 0, 10910340
section profiling
Agent [baseline] (1.161 s) : 0, 1161411
Total [baseline] (11.055 s) : 0, 11054977
Agent [candidate] (1.166 s) : 0, 1166099
Total [candidate] (11.136 s) : 0, 11135633
gantt
title petclinic - break down per module: candidate=1.55.0-SNAPSHOT~66a1ee4867, baseline=1.55.0-SNAPSHOT~425276dceb
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.482 ms) : 0, 1482
crashtracking [candidate] (1.47 ms) : 0, 1470
BytebuddyAgent [baseline] (697.439 ms) : 0, 697439
BytebuddyAgent [candidate] (696.625 ms) : 0, 696625
GlobalTracer [baseline] (243.073 ms) : 0, 243073
GlobalTracer [candidate] (242.587 ms) : 0, 242587
AppSec [baseline] (32.435 ms) : 0, 32435
AppSec [candidate] (33.009 ms) : 0, 33009
Debugger [baseline] (6.429 ms) : 0, 6429
Debugger [candidate] (6.438 ms) : 0, 6438
Remote Config [baseline] (704.367 µs) : 0, 704
Remote Config [candidate] (720.789 µs) : 0, 721
Telemetry [baseline] (9.182 ms) : 0, 9182
Telemetry [candidate] (9.1 ms) : 0, 9100
Flare Poller [baseline] (10.964 ms) : 0, 10964
Flare Poller [candidate] (10.371 ms) : 0, 10371
section appsec
crashtracking [baseline] (1.468 ms) : 0, 1468
crashtracking [candidate] (1.497 ms) : 0, 1497
BytebuddyAgent [baseline] (717.773 ms) : 0, 717773
BytebuddyAgent [candidate] (732.184 ms) : 0, 732184
GlobalTracer [baseline] (234.591 ms) : 0, 234591
GlobalTracer [candidate] (238.868 ms) : 0, 238868
IAST [baseline] (24.817 ms) : 0, 24817
IAST [candidate] (25.462 ms) : 0, 25462
AppSec [baseline] (175.796 ms) : 0, 175796
AppSec [candidate] (178.204 ms) : 0, 178204
Debugger [baseline] (6.166 ms) : 0, 6166
Debugger [candidate] (6.257 ms) : 0, 6257
Remote Config [baseline] (644.273 µs) : 0, 644
Remote Config [candidate] (670.292 µs) : 0, 670
Telemetry [baseline] (8.553 ms) : 0, 8553
Telemetry [candidate] (8.674 ms) : 0, 8674
Flare Poller [baseline] (3.898 ms) : 0, 3898
Flare Poller [candidate] (4.068 ms) : 0, 4068
section iast
crashtracking [baseline] (1.471 ms) : 0, 1471
crashtracking [candidate] (1.461 ms) : 0, 1461
BytebuddyAgent [baseline] (820.025 ms) : 0, 820025
BytebuddyAgent [candidate] (815.347 ms) : 0, 815347
GlobalTracer [baseline] (232.5 ms) : 0, 232500
GlobalTracer [candidate] (231.827 ms) : 0, 231827
IAST [baseline] (26.746 ms) : 0, 26746
IAST [candidate] (26.57 ms) : 0, 26570
AppSec [baseline] (35.107 ms) : 0, 35107
AppSec [candidate] (35.797 ms) : 0, 35797
Debugger [baseline] (6.148 ms) : 0, 6148
Debugger [candidate] (6.122 ms) : 0, 6122
Remote Config [baseline] (604.016 µs) : 0, 604
Remote Config [candidate] (628.42 µs) : 0, 628
Telemetry [baseline] (8.636 ms) : 0, 8636
Telemetry [candidate] (8.581 ms) : 0, 8581
Flare Poller [baseline] (4.367 ms) : 0, 4367
Flare Poller [candidate] (4.123 ms) : 0, 4123
section profiling
crashtracking [baseline] (1.429 ms) : 0, 1429
crashtracking [candidate] (1.436 ms) : 0, 1436
BytebuddyAgent [baseline] (720.296 ms) : 0, 720296
BytebuddyAgent [candidate] (722.021 ms) : 0, 722021
GlobalTracer [baseline] (217.93 ms) : 0, 217930
GlobalTracer [candidate] (218.776 ms) : 0, 218776
AppSec [baseline] (32.423 ms) : 0, 32423
AppSec [candidate] (33.387 ms) : 0, 33387
Debugger [baseline] (6.52 ms) : 0, 6520
Debugger [candidate] (6.525 ms) : 0, 6525
Remote Config [baseline] (784.541 µs) : 0, 785
Remote Config [candidate] (700.005 µs) : 0, 700
Telemetry [baseline] (16.251 ms) : 0, 16251
Telemetry [candidate] (16.604 ms) : 0, 16604
Flare Poller [baseline] (4.141 ms) : 0, 4141
Flare Poller [candidate] (4.343 ms) : 0, 4343
ProfilingAgent [baseline] (108.289 ms) : 0, 108289
ProfilingAgent [candidate] (109.107 ms) : 0, 109107
Profiling [baseline] (109.425 ms) : 0, 109425
Profiling [candidate] (110.221 ms) : 0, 110221
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 3 performance regressions! Performance is the same for 7 metrics, 12 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~66a1ee4867, baseline=1.55.0-SNAPSHOT~425276dceb
dateFormat X
axisFormat %s
section baseline
no_agent (37.268 ms) : 36967, 37569
. : milestone, 37268,
appsec (46.457 ms) : 46049, 46866
. : milestone, 46457,
code_origins (45.059 ms) : 44670, 45448
. : milestone, 45059,
iast (46.162 ms) : 45760, 46565
. : milestone, 46162,
profiling (46.639 ms) : 46192, 47087
. : milestone, 46639,
tracing (44.725 ms) : 44339, 45110
. : milestone, 44725,
section candidate
no_agent (38.072 ms) : 37763, 38382
. : milestone, 38072,
appsec (49.24 ms) : 48821, 49659
. : milestone, 49240,
code_origins (44.743 ms) : 44349, 45138
. : milestone, 44743,
iast (43.415 ms) : 43045, 43785
. : milestone, 43415,
profiling (45.718 ms) : 45279, 46158
. : milestone, 45718,
tracing (45.328 ms) : 44944, 45712
. : milestone, 45328,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~66a1ee4867, baseline=1.55.0-SNAPSHOT~425276dceb
dateFormat X
axisFormat %s
section baseline
no_agent (4.393 ms) : 4340, 4446
. : milestone, 4393,
iast (9.77 ms) : 9609, 9931
. : milestone, 9770,
iast_FULL (14.141 ms) : 13853, 14428
. : milestone, 14141,
iast_GLOBAL (10.558 ms) : 10373, 10744
. : milestone, 10558,
profiling (8.632 ms) : 8497, 8766
. : milestone, 8632,
tracing (7.864 ms) : 7752, 7977
. : milestone, 7864,
section candidate
no_agent (4.42 ms) : 4362, 4477
. : milestone, 4420,
iast (9.638 ms) : 9474, 9803
. : milestone, 9638,
iast_FULL (14.739 ms) : 14444, 15035
. : milestone, 14739,
iast_GLOBAL (11.194 ms) : 10992, 11396
. : milestone, 11194,
profiling (8.492 ms) : 8362, 8621
. : milestone, 8492,
tracing (7.434 ms) : 7328, 7540
. : milestone, 7434,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.55.0-SNAPSHOT~66a1ee4867, baseline=1.55.0-SNAPSHOT~425276dceb
dateFormat X
axisFormat %s
section baseline
no_agent (1.476 ms) : 1464, 1488
. : milestone, 1476,
appsec (3.74 ms) : 3520, 3959
. : milestone, 3740,
iast (2.212 ms) : 2149, 2275
. : milestone, 2212,
iast_GLOBAL (2.249 ms) : 2186, 2313
. : milestone, 2249,
profiling (2.061 ms) : 2010, 2113
. : milestone, 2061,
tracing (2.024 ms) : 1975, 2073
. : milestone, 2024,
section candidate
no_agent (1.477 ms) : 1466, 1489
. : milestone, 1477,
appsec (3.68 ms) : 3465, 3895
. : milestone, 3680,
iast (2.204 ms) : 2141, 2267
. : milestone, 2204,
iast_GLOBAL (2.252 ms) : 2188, 2316
. : milestone, 2252,
profiling (2.049 ms) : 1998, 2099
. : milestone, 2049,
tracing (2.04 ms) : 1990, 2090
. : milestone, 2040,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.55.0-SNAPSHOT~66a1ee4867, baseline=1.55.0-SNAPSHOT~425276dceb
dateFormat X
axisFormat %s
section baseline
no_agent (15.546 s) : 15546000, 15546000
. : milestone, 15546000,
appsec (15.178 s) : 15178000, 15178000
. : milestone, 15178000,
iast (18.767 s) : 18767000, 18767000
. : milestone, 18767000,
iast_GLOBAL (18.23 s) : 18230000, 18230000
. : milestone, 18230000,
profiling (15.994 s) : 15994000, 15994000
. : milestone, 15994000,
tracing (15.27 s) : 15270000, 15270000
. : milestone, 15270000,
section candidate
no_agent (15.75 s) : 15750000, 15750000
. : milestone, 15750000,
appsec (15.235 s) : 15235000, 15235000
. : milestone, 15235000,
iast (18.65 s) : 18650000, 18650000
. : milestone, 18650000,
iast_GLOBAL (18.331 s) : 18331000, 18331000
. : milestone, 18331000,
profiling (15.143 s) : 15143000, 15143000
. : milestone, 15143000,
tracing (15.15 s) : 15150000, 15150000
. : milestone, 15150000,
|
3245121
to
9c634ef
Compare
// fall-through and check for non-datadog span data | ||
} else if (OTEL_CONTEXT_ROOT_SPAN_KEY.equals(key.toString())) { | ||
AgentSpan span = AgentSpan.fromContext(delegate); | ||
if (span != 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.
Should we add the same span.isValid()
check here for consistency?
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 was unsure about this one but I'm happy that you reacted on this one. Perhaps yes if we want to have the caller method to be consistent about the returned value in all cases
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.
done in 1c997f9
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
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.
Nice!
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.
📝 notes: I tried to squeeze any data possible into an invalid span context to make sure we can't loose it from this change but OTel API will drop it before you get the chance to store it into a context. -- Methods like Span.wrap()
, PropagatedSpan.create()
will drop any invalid data info.
So the changes look good. Thanks for the fix @amarziali ! 👍
I pushed minor changes wrt to the test suites.
Updating PR description and attaching to the JIRA card |
Moving missing trace context to a dedicated test (not dependent on other propagation style as it concerns tracecontext only). Adding more tests related to Span.fromContextOrNull() on span activation test suite.
61accd3
to
66a1ee4
Compare
What Does This Do
This PR will stop returning span / tracing context when they are invalid.
Motivation
This will change the semantic of
Span.fromContextOrNull()
to finally returnnull
(even if an invalid context can still be considered as valid) and will save allocating OTel wrapper objects.Additional Notes
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any useful labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: APMS-17294