test(trace stats): assert trace stats aggregation values via fake-intake#1256
Merged
Conversation
|
lucaspimentel
changed the title
lucaspimentel
force-pushed
the
lpimentel/stats-aggregation-tests
branch
from
June 12, 2026 23:20
a17f441 to
f706f60
Compare
lucaspimentel
changed the base branch from
main
to
lpimentel/fix-cold-start-test-runtime
June 12, 2026 23:21
lucaspimentel
marked this pull request as ready for review
June 12, 2026 23:22
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end, value-level assertions for trace stats aggregation by routing real spans through bottlecap’s trace/stats pipeline and validating the decoded ClientGroupedStats captured by the in-process fake-intake. This strengthens integration coverage beyond “payload exists/roundtrips” by checking that the computed aggregates (hits/errors/duration) and grouping keys match expectations.
Changes:
- Generalizes the existing test helper into
run_processor_pipeline_with_traces(.., traces)and keeps a single-span wrapper for existing_dd.compute_statstests. - Introduces deterministic, aggregate-value tests (hits/errors/duration and grouping by resource,
span.kind, andhttp.status_code) using concrete spans sent throughSendingTraceProcessor → StatsConcentratorService → StatsFlusher → fake-intake. - Centralizes a fixed span start timestamp (
STATS_SPAN_START_NS) for deterministic bucketing behavior in tests.
Comment on lines
+403
to
+406
| /// Fixed span start well in the past relative to the concentrator's clock, so all spans | ||
| /// fold into the single oldest bucket and a `force_flush` returns exactly one bucket. | ||
| /// That keeps the aggregation assertions deterministic without controlling time. | ||
| const STATS_SPAN_START_NS: i64 = 1_700_000_000_000_000_000; |
litianningdatadog
approved these changes
Jun 15, 2026
Add E2E tests that route concrete spans through the real SpanConcentrator and assert on the computed aggregate values (hits, errors, duration sum, resource grouping) that reach the intake, rather than only on stats presence/suppression. Generalize run_processor_pipeline to accept arbitrary traces; keep a single-span wrapper for the existing _dd.compute_stats tests. 🤖 Co-Authored-By: Claude Code <[email protected]>
Add two more aggregation tests: spans differing only in span.kind or http.status_code must land in separate ClientGroupedStats, each counted independently. 🤖 Co-Authored-By: Claude Code <[email protected]>
…estamp Name the fixed past timestamp used to fold stats spans into one bucket, so its intent is self-documenting at each use site. 🤖 Co-Authored-By: Claude Code <[email protected]>
lucaspimentel
force-pushed
the
lpimentel/stats-aggregation-tests
branch
from
June 15, 2026 17:47
f706f60 to
6b29ef1
Compare
This file contains hidden or 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
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.
Note: stacked on fix(test): repair broken cold-start tests from #1254 #1255. Merge that PR first.Overview
Extends the in-process fake-intake APM tests (added in #1194) with value-level coverage of trace stats aggregation. The existing stats tests assert serialization fidelity (a hand-built
ClientStatsPayloadsurvives the roundtrip) and stats presence/suppression (_dd.compute_statstruth table). None assert that the computed aggregates reaching the intake are correct.This PR routes concrete spans through the real pipeline (
SendingTraceProcessor→StatsConcentratorService→StatsFlusher→ fake-intake) and asserts on the decodedClientGroupedStats:e2e_stats_count_aggregates_identical_spans— 3 identical top-level spans collapse into one group withhits == 3,top_level_hits == 3.e2e_stats_counts_errors_separately_from_hits—errorscounts onlyerror != 0;hitscounts all.e2e_stats_sums_span_durations—durationis the nanosecond sum of aggregated spans.e2e_stats_groups_by_resource— distinct resources stay in separate groups.e2e_stats_groups_by_span_kind— distinctspan.kindstay in separate groups.e2e_stats_groups_by_http_status_code— distincthttp.status_codestay in separate groups.These verify bottlecap's wiring and serialization end-to-end (a span entering the trace path comes out the intake correctly aggregated), which libdatadog's concentrator unit tests cannot see. The aggregation math itself is owned and exhaustively tested upstream in
libdd-trace-stats.To support multiple spans,
run_processor_pipelinewas generalized intorun_processor_pipeline_with_traces(.., traces), with a thin single-span wrapper preserving the existing_dd.compute_statstests unchanged.Out of scope: time-bucketing
Deliberately not added. Bucket assignment lives entirely inside
SpanConcentratorand is thoroughly tested inlibdd-trace-stats(test_concentrator_stats_countsover multiple buckets,test_concentrator_oldest_timestamp_cold/hot,test_force_flush). bottlecap's wiring does not touch bucketing, so a test here would duplicate upstream coverage and require a production clock seam (the concentrator seedsoldest_timestampfrom wall-clock) for no unique coverage.Testing
This PR is only adding more tests. If they pass, we're good.
Ran locally:
cargo test --test apm_integration_testcargo fmt --all -- --checkcargo clippy --workspace --all-targets --features default