Commit 51f0fec
Use bitmask SpanKindFilter for per-span eligibility in metrics aggregator (#11380)
Trim per-span work on metrics aggregator publish path
ConflatingMetricsAggregator.publish does a handful of redundant operations on
every span. None individually is large; together they show as ~2.5% on the
existing JMH benchmark once the benchmark actually exercises span.kind.
- dedup span.isTopLevel(): publish() reads it into a local, then shouldComputeMetric
read it again. Pass the cached value in.
- resolve spanKind to String once: master called toString() twice per span (once
inside spanKindEligible, once at the getPeerTags call site) and used HashSet
contains on a CharSequence (which routes through equals on String). Normalize
to String up front and reuse.
- lazy-allocate the peer-tag list: getPeerTags() always allocated an ArrayList
sized to features.peerTags() even when the span had none of those tags set.
Defer allocation until the first match; return Collections.emptyList() when
none hit. MetricKey already treats null/empty peerTags as emptyList, so no
behavior change.
Drop the spanKindEligible helper — the HashSet.contains call inlines fine in
shouldComputeMetric.
Update the JMH benchmark to set span.kind=client on every span. Without it the
filter path short-circuits before the peer-tag and toString work, so the wins
above aren't measurable. With it:
baseline 6.755 us/op (CI [6.560, 6.950], stdev 0.129)
optimized 6.585 us/op (CI [6.536, 6.634], stdev 0.033)
2 forks x 5 iterations x 15s. ~2.5% mean improvement and much tighter variance
fork-to-fork.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add SpanKindFilter and CoreSpan.isKind for bitmask-based kind checks
Introduce SpanKindFilter -- a tiny builder-built immutable filter whose state
is an int bitmask indexed by the span.kind ordinals already cached on
DDSpanContext. Each include* on the builder sets one bit (1 << ordinal); the
runtime check is a single AND against (1 << span's ordinal).
CoreSpan.isKind(SpanKindFilter) is the new entry point. DDSpan overrides it
to do the bit-test directly against the cached ordinal -- no virtual call,
no tag-map lookup. The two existing test-only CoreSpan impls (SimpleSpan
and TraceGenerator.PojoSpan, the latter in two source sets) implement isKind
by reading the span.kind tag and delegating to SpanKindFilter.matches(String),
which converts via DDSpanContext.spanKindOrdinalOf and does the same AND.
Refactor: DDSpanContext.setSpanKindOrdinal(String) now delegates to a new
package-private static spanKindOrdinalOf(String) so the same string-to-ordinal
mapping serves both the tag interceptor path and SpanKindFilter.matches.
This is groundwork -- nothing in the codebase calls isKind yet. The next
commit will replace the HashSet-based eligibility checks in
ConflatingMetricsAggregator with SpanKindFilter instances.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use SpanKindFilter in ConflatingMetricsAggregator
Replace the two ELIGIBLE_SPAN_KINDS_FOR_* HashSet<String> constants and the
SPAN_KIND_INTERNAL.equals check with three SpanKindFilter instances:
METRICS_ELIGIBLE_KINDS, PEER_AGGREGATION_KINDS, INTERNAL_KIND. Eligibility
checks now go through span.isKind(filter), which on DDSpan is a volatile
byte read against the already-cached span.kind ordinal plus a single bit-test.
Also defer the span.kind tag read: previously read at the top of the publish
loop and threaded through both shouldComputeMetric and the inner publish.
isKind no longer needs the string, so the read can move down into the inner
publish where it's still needed for the SPAN_KINDS cache key / MetricKey.
Supporting changes:
- DDSpanContext.spanKindOrdinalOf(String) is now public so non-DDSpan CoreSpan
impls can compute the ordinal at tag-write time.
- SpanKindFilter gains a public matches(byte) fast-path overload that callers
with a pre-computed ordinal use directly.
- SimpleSpan caches the ordinal in setTag(SPAN_KIND, ...), mirroring what
TagInterceptor does for DDSpanContext, and its isKind now hits the byte
fast path. Without this, the JMH benchmark (which uses SimpleSpan) would
re-derive the ordinal on every isKind call and overstate the cost.
Benchmark on the bench updated last commit (kind=client on every span,
4 forks x 5 iter x 15s):
prior commit 6.585 ± 0.049 us/op
this commit 6.903 ± 0.096 us/op
The slight regression is a SimpleSpan-via-groovy-dispatch artifact -- the
interface call to isKind through CoreSpan, then through SimpleSpan, then
through SpanKindFilter.matches, doesn't fold as aggressively as a HashSet
contains on a static field. In production DDSpan.isKind inlines to a context
field read + ordinal byte read + bit-test, so the production path is faster
than the prior HashSet approach. A DDSpan-based benchmark would show this;
the existing SimpleSpan-based one doesn't.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add DDSpan-based variant of ConflatingMetricsAggregator JMH benchmark
The existing ConflatingMetricsAggregatorBenchmark uses SimpleSpan, a groovy
mock. That's enough for measuring queue/CHM/MetricKey work, but it conceals
the production cost of CoreSpan.isKind: SimpleSpan's isKind goes through
groovy interface dispatch into SpanKindFilter.matches, while DDSpan.isKind
inlines to a context byte-read + bit-test.
This new benchmark uses real DDSpan instances created through a CoreTracer
(with a NoopWriter so finishing doesn't reach the agent). Same shape as the
SimpleSpan bench (64-span trace, span.kind=client, peer.hostname set).
Numbers (2 forks x 5 iter x 15s):
master: 6.428 +- 0.189 us/op (HashSet eligibility checks)
this branch: 6.343 +- 0.115 us/op (SpanKindFilter bitmask)
About 1.3% faster on the production path. The SimpleSpan benchmark in the
same conditions shows a ~2.2% slowdown -- the mock's dispatch shape gives a
misleading signal.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tighten SpanKindFilter encapsulation
Make SpanKindFilter.kindMask and its constructor private now that DDSpan.isKind
no longer needs direct field access -- it delegates to SpanKindFilter.matches(byte).
The Builder.build() in the same outer class still constructs instances via the
private constructor.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merge branch 'master' into dougqh/conflating-metrics-producer-wins
Clear span.kind ordinal cache on ledger removal
DDSpanContext.setAllTags(TagMap.Ledger) removed tags via
unsafeTags.remove(...) without clearing the cached spanKindOrdinal.
A builder that set span.kind and then nulled it on the same
SpanBuilder (withTag(SPAN_KIND, "client").withTag(SPAN_KIND, null))
left the cached ordinal stale at CLIENT, so the new bitmask
eligibility checks counted spans the previous tag-map-based code
correctly skipped.
Mirror what removeTag(String) already does: when the removal targets
SPAN_KIND, reset the cached ordinal to SPAN_KIND_UNSET before
forwarding the remove to unsafeTags.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply review feedback on SpanKindFilter rollout
- CoreSpan.isKind: now a default method that reads span.kind via
unsafeGetTag and delegates to SpanKindFilter.matches(String). The
three test-only implementations (SimpleSpan + two PojoSpans) no
longer need their own copies, and SimpleSpan's spanKindOrdinal
cache can go away too.
- DDSpan.isKind: keeps the fast path (cached ordinal + bit-test) and
now carries an explicit @OverRide.
- DDSpanContext.spanKindOrdinalOf: package-private now that the only
remaining caller is SpanKindFilter (same package).
- SpanKindFilter: class-level javadoc spelling out the recognized
span.kind values and that arbitrary custom strings collapse to
SPAN_KIND_CUSTOM and never match — by design.
- ConflatingMetricsAggregator: static-import Collections.emptyList /
singletonList / singletonMap per project conventions.
- ConflatingMetricsAggregatorDDSpanBenchmark: record the rollout
result (~1.3% faster on the DDSpan path) in the class javadoc.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merge branch 'master' into dougqh/conflating-metrics-producer-wins
Remove unused Tags import in TraceGenerator
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merge branch 'master' into dougqh/conflating-metrics-producer-wins
Merge branch 'master' into dougqh/conflating-metrics-producer-wins
Remove unused Tags import from traceAgentTest TraceGenerator
codenarcTraceAgentTest flagged the bootstrap.instrumentation.api.Tags
import as never referenced. Same fix as f9822a5 applied earlier to
the test/ copy of TraceGenerator.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>1 parent 02117e3 commit 51f0fec
8 files changed
Lines changed: 267 additions & 48 deletions
File tree
- dd-trace-core/src
- jmh/java/datadog/trace/common/metrics
- main/java/datadog/trace
- common/metrics
- core
- test/java/datadog/trace/core
Lines changed: 3 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
| 5 | + | |
4 | 6 | | |
5 | 7 | | |
6 | 8 | | |
| |||
52 | 54 | | |
53 | 55 | | |
54 | 56 | | |
| 57 | + | |
55 | 58 | | |
56 | 59 | | |
57 | 60 | | |
| |||
Lines changed: 109 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
Lines changed: 36 additions & 38 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
14 | | - | |
15 | 10 | | |
16 | 11 | | |
17 | 12 | | |
18 | 13 | | |
19 | 14 | | |
20 | 15 | | |
21 | 16 | | |
22 | | - | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
23 | 20 | | |
24 | 21 | | |
25 | 22 | | |
| |||
36 | 33 | | |
37 | 34 | | |
38 | 35 | | |
| 36 | + | |
39 | 37 | | |
40 | 38 | | |
41 | 39 | | |
42 | | - | |
43 | | - | |
44 | | - | |
45 | 40 | | |
46 | 41 | | |
47 | 42 | | |
| |||
50 | 45 | | |
51 | 46 | | |
52 | 47 | | |
53 | | - | |
54 | 48 | | |
55 | 49 | | |
56 | 50 | | |
| |||
60 | 54 | | |
61 | 55 | | |
62 | 56 | | |
63 | | - | |
| 57 | + | |
64 | 58 | | |
65 | 59 | | |
66 | 60 | | |
| |||
82 | 76 | | |
83 | 77 | | |
84 | 78 | | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
90 | 86 | | |
91 | | - | |
92 | | - | |
93 | | - | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
94 | 92 | | |
95 | 93 | | |
96 | 94 | | |
| |||
289 | 287 | | |
290 | 288 | | |
291 | 289 | | |
292 | | - | |
293 | | - | |
| 290 | + | |
294 | 291 | | |
295 | 292 | | |
296 | 293 | | |
297 | 294 | | |
298 | 295 | | |
299 | 296 | | |
300 | 297 | | |
301 | | - | |
| 298 | + | |
302 | 299 | | |
303 | 300 | | |
304 | 301 | | |
305 | 302 | | |
306 | 303 | | |
307 | 304 | | |
308 | 305 | | |
309 | | - | |
310 | | - | |
| 306 | + | |
| 307 | + | |
311 | 308 | | |
312 | 309 | | |
313 | 310 | | |
314 | 311 | | |
315 | 312 | | |
316 | | - | |
317 | | - | |
318 | | - | |
319 | | - | |
320 | | - | |
321 | | - | |
| 313 | + | |
322 | 314 | | |
323 | 315 | | |
324 | 316 | | |
| |||
335 | 327 | | |
336 | 328 | | |
337 | 329 | | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
338 | 333 | | |
339 | 334 | | |
340 | 335 | | |
| |||
347 | 342 | | |
348 | 343 | | |
349 | 344 | | |
350 | | - | |
| 345 | + | |
351 | 346 | | |
352 | 347 | | |
353 | 348 | | |
| |||
382 | 377 | | |
383 | 378 | | |
384 | 379 | | |
385 | | - | |
386 | | - | |
| 380 | + | |
| 381 | + | |
387 | 382 | | |
388 | | - | |
| 383 | + | |
389 | 384 | | |
390 | 385 | | |
391 | 386 | | |
392 | 387 | | |
393 | 388 | | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
394 | 392 | | |
395 | 393 | | |
396 | 394 | | |
397 | 395 | | |
398 | 396 | | |
399 | 397 | | |
400 | | - | |
401 | | - | |
| 398 | + | |
| 399 | + | |
402 | 400 | | |
403 | 401 | | |
404 | 402 | | |
405 | 403 | | |
406 | 404 | | |
407 | | - | |
| 405 | + | |
408 | 406 | | |
409 | 407 | | |
410 | 408 | | |
411 | 409 | | |
412 | 410 | | |
413 | | - | |
| 411 | + | |
414 | 412 | | |
415 | 413 | | |
416 | 414 | | |
| |||
Lines changed: 6 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
| |||
80 | 81 | | |
81 | 82 | | |
82 | 83 | | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
83 | 89 | | |
84 | 90 | | |
85 | 91 | | |
| |||
Lines changed: 5 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
959 | 959 | | |
960 | 960 | | |
961 | 961 | | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
962 | 967 | | |
963 | 968 | | |
964 | 969 | | |
| |||
0 commit comments