Refactor agent spans and scopes #8321
Merged
+952
−791
Conversation
This file contains 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
PerfectSlayer
added
comp: core
PerfectSlayer
changed the title
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/BlackHoleSpan.java
Show resolved
Hide resolved
internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/BlackHoleSpan.java
Show resolved
Hide resolved
internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NoopSpan.java
Show resolved
Hide resolved
internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ExtractedSpan.java
Show resolved
Hide resolved
PerfectSlayer
changed the title
PerfectSlayer
force-pushed
the
bbujon/context-propagation-step3
branch
2 times, most recently
from
47a977e to
9d95fd3
Compare
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 59 metrics, 4 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.47.0-SNAPSHOT~466ff09dae, baseline=1.47.0-SNAPSHOT~e7dd598ab0
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.052 s) : 0, 1052324
Total [baseline] (8.631 s) : 0, 8631396
Agent [candidate] (1.053 s) : 0, 1052503
Total [candidate] (8.669 s) : 0, 8668768
section iast
Agent [baseline] (1.171 s) : 0, 1171136
Total [baseline] (9.185 s) : 0, 9185065
Agent [candidate] (1.175 s) : 0, 1175272
Total [candidate] (9.215 s) : 0, 9214822
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.17 s) : 0, 1170203
Total [baseline] (9.178 s) : 0, 9178219
Agent [candidate] (1.182 s) : 0, 1182032
Total [candidate] (9.198 s) : 0, 9198298
section iast_TELEMETRY_OFF
Agent [baseline] (1.182 s) : 0, 1181999
Total [baseline] (9.199 s) : 0, 9199373
Agent [candidate] (1.175 s) : 0, 1174876
Total [candidate] (9.212 s) : 0, 9212088
gantt
title insecure-bank - break down per module: candidate=1.47.0-SNAPSHOT~466ff09dae, baseline=1.47.0-SNAPSHOT~e7dd598ab0
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (722.482 ms) : 0, 722482
BytebuddyAgent [candidate] (725.054 ms) : 0, 725054
GlobalTracer [baseline] (244.665 ms) : 0, 244665
GlobalTracer [candidate] (244.56 ms) : 0, 244560
AppSec [baseline] (55.445 ms) : 0, 55445
AppSec [candidate] (55.352 ms) : 0, 55352
Remote Config [baseline] (727.835 µs) : 0, 728
Remote Config [candidate] (714.864 µs) : 0, 715
Telemetry [baseline] (13.705 ms) : 0, 13705
Telemetry [candidate] (11.495 ms) : 0, 11495
section iast
BytebuddyAgent [baseline] (834.158 ms) : 0, 834158
BytebuddyAgent [candidate] (837.683 ms) : 0, 837683
GlobalTracer [baseline] (232.831 ms) : 0, 232831
GlobalTracer [candidate] (233.372 ms) : 0, 233372
IAST [baseline] (22.791 ms) : 0, 22791
IAST [candidate] (22.86 ms) : 0, 22860
AppSec [baseline] (56.856 ms) : 0, 56856
AppSec [candidate] (56.694 ms) : 0, 56694
Remote Config [baseline] (608.609 µs) : 0, 609
Remote Config [candidate] (611.984 µs) : 0, 612
Telemetry [baseline] (8.634 ms) : 0, 8634
Telemetry [candidate] (8.755 ms) : 0, 8755
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (832.962 ms) : 0, 832962
BytebuddyAgent [candidate] (841.97 ms) : 0, 841970
GlobalTracer [baseline] (233.211 ms) : 0, 233211
GlobalTracer [candidate] (234.815 ms) : 0, 234815
IAST [baseline] (22.862 ms) : 0, 22862
IAST [candidate] (23.173 ms) : 0, 23173
AppSec [baseline] (56.628 ms) : 0, 56628
AppSec [candidate] (57.236 ms) : 0, 57236
Remote Config [baseline] (621.394 µs) : 0, 621
Remote Config [candidate] (636.146 µs) : 0, 636
Telemetry [baseline] (8.643 ms) : 0, 8643
Telemetry [candidate] (8.79 ms) : 0, 8790
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (843.119 ms) : 0, 843119
BytebuddyAgent [candidate] (836.641 ms) : 0, 836641
GlobalTracer [baseline] (234.777 ms) : 0, 234777
GlobalTracer [candidate] (234.625 ms) : 0, 234625
IAST [baseline] (25.504 ms) : 0, 25504
IAST [candidate] (26.313 ms) : 0, 26313
AppSec [baseline] (53.882 ms) : 0, 53882
AppSec [candidate] (52.708 ms) : 0, 52708
Remote Config [baseline] (622.193 µs) : 0, 622
Remote Config [candidate] (612.355 µs) : 0, 612
Telemetry [baseline] (8.614 ms) : 0, 8614
Telemetry [candidate] (8.59 ms) : 0, 8590
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.47.0-SNAPSHOT~466ff09dae, baseline=1.47.0-SNAPSHOT~e7dd598ab0
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.043 s) : 0, 1042662
Total [baseline] (10.459 s) : 0, 10459488
Agent [candidate] (1.064 s) : 0, 1064298
Total [candidate] (10.613 s) : 0, 10612874
section appsec
Agent [baseline] (1.184 s) : 0, 1184171
Total [baseline] (10.758 s) : 0, 10757579
Agent [candidate] (1.185 s) : 0, 1185385
Total [candidate] (10.724 s) : 0, 10724425
section iast
Agent [baseline] (1.178 s) : 0, 1177807
Total [baseline] (10.97 s) : 0, 10969944
Agent [candidate] (1.175 s) : 0, 1175245
Total [candidate] (10.948 s) : 0, 10948289
section profiling
Agent [baseline] (1.27 s) : 0, 1269720
Total [baseline] (10.884 s) : 0, 10884190
Agent [candidate] (1.266 s) : 0, 1266325
Total [candidate] (10.856 s) : 0, 10856496
gantt
title petclinic - break down per module: candidate=1.47.0-SNAPSHOT~466ff09dae, baseline=1.47.0-SNAPSHOT~e7dd598ab0
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (716.323 ms) : 0, 716323
BytebuddyAgent [candidate] (729.946 ms) : 0, 729946
GlobalTracer [baseline] (243.555 ms) : 0, 243555
GlobalTracer [candidate] (247.537 ms) : 0, 247537
AppSec [baseline] (55.386 ms) : 0, 55386
AppSec [candidate] (56.153 ms) : 0, 56153
Remote Config [baseline] (714.039 µs) : 0, 714
Remote Config [candidate] (730.236 µs) : 0, 730
Telemetry [baseline] (11.495 ms) : 0, 11495
Telemetry [candidate] (14.434 ms) : 0, 14434
section appsec
BytebuddyAgent [baseline] (732.787 ms) : 0, 732787
BytebuddyAgent [candidate] (733.357 ms) : 0, 733357
GlobalTracer [baseline] (239.996 ms) : 0, 239996
GlobalTracer [candidate] (240.482 ms) : 0, 240482
IAST [baseline] (21.693 ms) : 0, 21693
IAST [candidate] (21.831 ms) : 0, 21831
AppSec [baseline] (176.454 ms) : 0, 176454
AppSec [candidate] (176.522 ms) : 0, 176522
Remote Config [baseline] (657.138 µs) : 0, 657
Remote Config [candidate] (657.976 µs) : 0, 658
Telemetry [baseline] (8.257 ms) : 0, 8257
Telemetry [candidate] (8.278 ms) : 0, 8278
section iast
BytebuddyAgent [baseline] (838.715 ms) : 0, 838715
BytebuddyAgent [candidate] (836.865 ms) : 0, 836865
GlobalTracer [baseline] (234.321 ms) : 0, 234321
GlobalTracer [candidate] (234.105 ms) : 0, 234105
IAST [baseline] (23.014 ms) : 0, 23014
IAST [candidate] (22.841 ms) : 0, 22841
AppSec [baseline] (57.025 ms) : 0, 57025
AppSec [candidate] (56.802 ms) : 0, 56802
Remote Config [baseline] (623.458 µs) : 0, 623
Remote Config [candidate] (617.114 µs) : 0, 617
Telemetry [baseline] (8.806 ms) : 0, 8806
Telemetry [candidate] (8.753 ms) : 0, 8753
section profiling
BytebuddyAgent [baseline] (712.15 ms) : 0, 712150
BytebuddyAgent [candidate] (709.047 ms) : 0, 709047
GlobalTracer [baseline] (354.688 ms) : 0, 354688
GlobalTracer [candidate] (354.675 ms) : 0, 354675
AppSec [baseline] (55.35 ms) : 0, 55350
AppSec [candidate] (54.733 ms) : 0, 54733
Remote Config [baseline] (689.336 µs) : 0, 689
Remote Config [candidate] (699.962 µs) : 0, 700
Telemetry [baseline] (8.854 ms) : 0, 8854
Telemetry [candidate] (9.035 ms) : 0, 9035
ProfilingAgent [baseline] (95.46 ms) : 0, 95460
ProfilingAgent [candidate] (95.815 ms) : 0, 95815
Profiling [baseline] (95.484 ms) : 0, 95484
Profiling [candidate] (95.839 ms) : 0, 95839
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 17 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.47.0-SNAPSHOT~466ff09dae, baseline=1.47.0-SNAPSHOT~e7dd598ab0
dateFormat X
axisFormat %s
section baseline
no_agent (382.322 µs) : 363, 402
. : milestone, 382,
iast (504.934 µs) : 483, 527
. : milestone, 505,
iast_FULL (747.82 µs) : 726, 770
. : milestone, 748,
iast_GLOBAL (548.103 µs) : 527, 570
. : milestone, 548,
iast_HARDCODED_SECRET_DISABLED (504.018 µs) : 483, 525
. : milestone, 504,
iast_INACTIVE (462.481 µs) : 441, 484
. : milestone, 462,
iast_TELEMETRY_OFF (498.8 µs) : 475, 522
. : milestone, 499,
tracing (453.146 µs) : 432, 474
. : milestone, 453,
section candidate
no_agent (380.665 µs) : 361, 400
. : milestone, 381,
iast (501.583 µs) : 480, 523
. : milestone, 502,
iast_FULL (742.229 µs) : 720, 764
. : milestone, 742,
iast_GLOBAL (551.974 µs) : 530, 574
. : milestone, 552,
iast_HARDCODED_SECRET_DISABLED (510.019 µs) : 489, 532
. : milestone, 510,
iast_INACTIVE (468.318 µs) : 447, 489
. : milestone, 468,
iast_TELEMETRY_OFF (489.648 µs) : 467, 513
. : milestone, 490,
tracing (454.847 µs) : 434, 476
. : milestone, 455,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.47.0-SNAPSHOT~466ff09dae, baseline=1.47.0-SNAPSHOT~e7dd598ab0
dateFormat X
axisFormat %s
section baseline
no_agent (1.352 ms) : 1333, 1371
. : milestone, 1352,
appsec (1.737 ms) : 1713, 1761
. : milestone, 1737,
appsec_no_iast (1.767 ms) : 1744, 1789
. : milestone, 1767,
iast (1.52 ms) : 1496, 1544
. : milestone, 1520,
profiling (1.514 ms) : 1490, 1538
. : milestone, 1514,
tracing (1.506 ms) : 1482, 1530
. : milestone, 1506,
section candidate
no_agent (1.368 ms) : 1348, 1388
. : milestone, 1368,
appsec (1.749 ms) : 1726, 1771
. : milestone, 1749,
appsec_no_iast (1.762 ms) : 1739, 1785
. : milestone, 1762,
iast (1.515 ms) : 1491, 1538
. : milestone, 1515,
profiling (1.569 ms) : 1545, 1594
. : milestone, 1569,
tracing (1.502 ms) : 1478, 1526
. : milestone, 1502,
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.47.0-SNAPSHOT~466ff09dae, baseline=1.47.0-SNAPSHOT~e7dd598ab0
dateFormat X
axisFormat %s
section baseline
no_agent (1.473 ms) : 1461, 1484
. : milestone, 1473,
appsec (2.361 ms) : 2317, 2404
. : milestone, 2361,
iast (2.1 ms) : 2045, 2155
. : milestone, 2100,
iast_GLOBAL (2.155 ms) : 2100, 2211
. : milestone, 2155,
profiling (1.98 ms) : 1935, 2025
. : milestone, 1980,
tracing (1.94 ms) : 1898, 1982
. : milestone, 1940,
section candidate
no_agent (1.468 ms) : 1456, 1479
. : milestone, 1468,
appsec (2.357 ms) : 2314, 2400
. : milestone, 2357,
iast (2.105 ms) : 2050, 2159
. : milestone, 2105,
iast_GLOBAL (2.154 ms) : 2098, 2209
. : milestone, 2154,
profiling (2.464 ms) : 2275, 2653
. : milestone, 2464,
tracing (1.938 ms) : 1896, 1980
. : milestone, 1938,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.47.0-SNAPSHOT~466ff09dae, baseline=1.47.0-SNAPSHOT~e7dd598ab0
dateFormat X
axisFormat %s
section baseline
no_agent (14.931 s) : 14931000, 14931000
. : milestone, 14931000,
appsec (15.088 s) : 15088000, 15088000
. : milestone, 15088000,
iast (18.707 s) : 18707000, 18707000
. : milestone, 18707000,
iast_GLOBAL (18.215 s) : 18215000, 18215000
. : milestone, 18215000,
profiling (15.213 s) : 15213000, 15213000
. : milestone, 15213000,
tracing (14.927 s) : 14927000, 14927000
. : milestone, 14927000,
section candidate
no_agent (14.992 s) : 14992000, 14992000
. : milestone, 14992000,
appsec (15.109 s) : 15109000, 15109000
. : milestone, 15109000,
iast (19.103 s) : 19103000, 19103000
. : milestone, 19103000,
iast_GLOBAL (18.019 s) : 18019000, 18019000
. : milestone, 18019000,
profiling (15.12 s) : 15120000, 15120000
. : milestone, 15120000,
tracing (15.133 s) : 15133000, 15133000
. : milestone, 15133000,
|
PerfectSlayer
force-pushed
the
bbujon/context-propagation-step3
branch
3 times, most recently
from
2a4a5ad to
2bb3c30
Compare
PerfectSlayer
force-pushed
the
bbujon/context-propagation-step3
branch
from
2bb3c30 to
9b9e1ac
Compare
mcculls
reviewed
Feb 5, 2025
...p/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ConcurrentState.java
Outdated
Show resolved
Hide resolved
mcculls
reviewed
Feb 5, 2025
...-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java
Outdated
Show resolved
Hide resolved
mcculls
reviewed
Feb 5, 2025
| noopScopeWrapper = new OtelScope(AgentTracer.NoopAgentScope.INSTANCE); | ||
| noopSpanWrapper = new OtelSpan(noopSpan(), this); | ||
| noopContextWrapper = new OtelSpanContext(AgentSpanContext.noop()); | ||
| noopScopeWrapper = new OtelScope(AgentScope.noop()); |
There was a problem hiding this comment.
IMHO we should have distinct names so they can all be static imported... i.e. AgentScope.noopScope() and AgentSpanContext.noopSpanContext() - having the above mix looks inconsistent
There was a problem hiding this comment.
I used noop() to be consistent with the other components creating noop implementations.
But you're right, having consistency across the tracing API makes more sense here.
I will:
- rename them to contains the returned type
- move
noopSpan()toAgentSpanas static to increase consistency and removeJava8BytecodeBridge.
We will end up with:
AgentSpan.noopSpan()AgentSpanContext.noopSpanContext()AgentScope.noopScope()
All static methods returning a singleton object (not accessible outside the tracking package).
Black hole spans creation will still be left to the tracer.
PerfectSlayer
force-pushed
the
bbujon/context-propagation-step3
branch
5 times, most recently
from
0052c55 to
3c34dc6
Compare
|
All comments should be addressed now :) |
PerfectSlayer
force-pushed
the
bbujon/context-propagation-step3
branch
2 times, most recently
from
468c214 to
2bc6c39
Compare
mcculls
reviewed
Feb 7, 2025
| * Returns the noop span instance. | ||
| * | ||
| * <p>This instance will always be the same, and can be safely tested using object identity (ie | ||
| * {@code =}). |
There was a problem hiding this comment.
Suggested change
| * {@code =}). | |
| * {@code ==}). |
There was a problem hiding this comment.
same change for other javadoc below, i.e. s/=/==/
There was a problem hiding this comment.
I will fix the 3 occurrences
mcculls
reviewed
Feb 7, 2025
| return Context.INSTANCE; | ||
| } | ||
|
|
||
| public static final class Context extends NoopSpanContext { |
There was a problem hiding this comment.
Suggested change
| public static final class Context extends NoopSpanContext { | |
| static final class Context extends NoopSpanContext { |
AFAICT it should be ok to make this package-private
There was a problem hiding this comment.
Also wondering if we should call it InvalidSpanContext...?
mcculls
reviewed
Feb 7, 2025
internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/BlackHoleSpan.java
Show resolved
Hide resolved
mcculls
reviewed
Feb 7, 2025
| if (this.spanContext instanceof TagContext) { | ||
| Map<String, String> tags = ((TagContext) this.spanContext).getTags(); | ||
| //noinspection unchecked | ||
| return (Map<String, Object>) (Map<?, ?>) tags; |
There was a problem hiding this comment.
you can simplify this to return (Map) tags; if you use //noinspection unchecked, rawtypes
mcculls
approved these changes
Feb 7, 2025
…tracer Add dedicated noop methods for span, span context and scope Remove unused eligibleForDropping method Improve Javadoc
PerfectSlayer
force-pushed
the
bbujon/context-propagation-step3
branch
from
2bc6c39 to
466ff09
Compare
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 extracts and refactors span and scope from tracer.
It introduces the
ImmutableSpanimplementation as a stub implementation for span mutators (AgentSpaninherits fromMutableSpanfrom public API so it can't be changed), and parent abstract class forNoopSpan(that does nothing),BlackHoleSpan(that stops propagation), andExtractedSpan(that represents a remove span).Motivation
Additional Notes
This also contains typo / wording fixes from #8313
I hesitate about introducing an
isValidmethod that checks for span validity (tid != 0& sid != 0) to distinguish noop and black hole span implementations.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-293