Skip to content

[APMSVLS-497][APMSVLS-498] test: add fake-intake for APM payload-level tests#1194

Merged
lucaspimentel merged 15 commits into
mainfrom
lpimentel/fake-intake-phase-1
May 20, 2026
Merged

[APMSVLS-497][APMSVLS-498] test: add fake-intake for APM payload-level tests#1194
lucaspimentel merged 15 commits into
mainfrom
lpimentel/fake-intake-phase-1

Conversation

@lucaspimentel

@lucaspimentel lucaspimentel commented Apr 21, 2026

Copy link
Copy Markdown
Member

Summary of changes

Adds an in-process fake intake that lets bottlecap's integration tests decode and inspect real outgoing APM payloads.

The fake intake accepts both APM flush endpoints (/api/v0.2/stats and /api/v0.2/traces), decodes each request body on arrival, and exposes typed query methods so tests can assert on individual fields. Two new integration tests exercise the full StatsFlusher and TraceFlusher fake-intake round trip.

Reason for change

Allow integration tests to inspect APM payloads and catch regressions.

Implementation details

  • add an in-process axum server on a random local port that accepts POST /api/v0.2/stats and POST /api/v0.2/traces, decodes each body on arrival, and stores the results.
  • add two new integration tests covering the two APM flush paths end-to-end.
  • The module is deliberately free of bottlecap-internal types to allow extraction into a shared crate later.

Test coverage

Added 2 new tests that server that use the new fake intake and serve as POCs.

Other details

Implements APMSVLS-497 and APMSVLS-498.

After the fake intake is proven and the API is stable, phase 2 will extract fake_intake.rs into a shared crate consumed by both bottlecap and SCL.

"Finally, a way to ask our StatsPayloads how they're really feeling, and not just whether they showed up to work." — Claude 🤖

@lucaspimentel lucaspimentel changed the title [APMSVLS-494] test: add in-process fake-intake for APM payload-level tests [APMSVLS-494] test: add fake-intake for APM payload-level tests Apr 21, 2026
@lucaspimentel
lucaspimentel marked this pull request as ready for review April 21, 2026 03:12
@lucaspimentel
lucaspimentel requested a review from a team as a code owner April 21, 2026 03:12
@lucaspimentel
lucaspimentel requested review from Copilot and duncanista and removed request for Copilot April 21, 2026 03:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an in-process “fake Datadog intake” test harness for Bottlecap APM payload-level integration testing, enabling assertions on decoded stats/traces payload fields rather than only verifying that an HTTP request occurred.

Changes:

  • Add FakeIntake axum server that accepts /api/v0.2/stats and /api/v0.2/traces, decodes bodies, and stores pb::StatsPayload / pb::AgentPayload for tests.
  • Add new APM integration tests that exercise the stats and traces flush paths and assert on decoded payload fields.
  • Make StatsFlusher::new accept an explicit stats intake URL so tests can redirect it to the fake intake; update the production call site accordingly.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
bottlecap/tests/common/mod.rs Adds a shared tests/common module exposing fake_intake.
bottlecap/tests/common/fake_intake.rs Implements the in-process fake intake server with decoding and captured payload storage.
bottlecap/tests/apm_integration_test.rs Adds payload-level integration tests for stats and traces flush paths.
bottlecap/src/traces/stats_flusher.rs Accepts stats_url in the constructor and logs the actual endpoint used.
bottlecap/src/bin/bottlecap/main.rs Passes trace_stats_url(&config.site) into StatsFlusher::new.
bottlecap/Cargo.toml Adds dev-dependencies for msgpack and gzip decoding used by the fake intake.
bottlecap/Cargo.lock Updates lockfile for the new dev-dependencies.

Comment thread bottlecap/tests/common/fake_intake.rs Outdated
Comment thread bottlecap/tests/common/fake_intake.rs
Comment thread bottlecap/tests/common/fake_intake.rs Outdated
Comment thread bottlecap/tests/common/mod.rs Outdated
@lucaspimentel lucaspimentel changed the title [APMSVLS-494] test: add fake-intake for APM payload-level tests [APMSVLS-497][APMSVLS-498] test: add fake-intake for APM payload-level tests Apr 21, 2026
@lucaspimentel
lucaspimentel requested a review from a team as a code owner April 23, 2026 20:12

@duncanista duncanista left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the stats URL change is different to what this PR claims, do we really need them together?

@lucaspimentel
lucaspimentel force-pushed the lpimentel/fake-intake-phase-1 branch from 2b10774 to 2e77c12 Compare April 24, 2026 17:36
@lucaspimentel
lucaspimentel changed the base branch from main to lpimentel/stats-flusher-url-param April 24, 2026 17:36
@lucaspimentel

lucaspimentel commented Apr 24, 2026

Copy link
Copy Markdown
Member Author

@duncanista

Seems like the stats URL change is different to what this PR claims, do we really need them together?

I split that small refactor out into #1210 and stacked this PR on that one.

@lucaspimentel
lucaspimentel force-pushed the lpimentel/stats-flusher-url-param branch from 8f52b41 to 6fbe14b Compare April 24, 2026 21:07
@lucaspimentel
lucaspimentel force-pushed the lpimentel/fake-intake-phase-1 branch from 2e77c12 to 027f404 Compare April 24, 2026 21:11
@lucaspimentel
lucaspimentel force-pushed the lpimentel/stats-flusher-url-param branch from 6fbe14b to 56162d9 Compare April 28, 2026 19:52
@lucaspimentel
lucaspimentel force-pushed the lpimentel/fake-intake-phase-1 branch from f6f171f to c51ec4d Compare April 29, 2026 19:21
@lucaspimentel
lucaspimentel marked this pull request as draft May 1, 2026 16:41
@lucaspimentel
lucaspimentel force-pushed the lpimentel/stats-flusher-url-param branch from 56162d9 to 1850777 Compare May 5, 2026 17:10
Base automatically changed from lpimentel/stats-flusher-url-param to main May 5, 2026 20:27
@lucaspimentel
lucaspimentel force-pushed the lpimentel/fake-intake-phase-1 branch from c51ec4d to aff7b2f Compare May 5, 2026 21:01
@lucaspimentel
lucaspimentel marked this pull request as ready for review May 5, 2026 23:07
@lucaspimentel
lucaspimentel requested review from a team and shreyamalpani May 18, 2026 18:52
…tests

Spawns an axum server on a random local port that accepts POSTs on
/api/v0.2/stats (msgpack + gzip) and /api/v0.2/traces (protobuf, with
zstd/gzip/identity decompression), decodes each body on arrival, and
stores the decoded pb::StatsPayload / pb::AgentPayload for tests to
query via typed methods.

Two integration tests cover both flush paths:

- stats_payload_roundtrip_through_fake_intake: drives StatsFlusher::send
  directly and asserts on decoded hit counts, span kind, peer tags, and
  HTTP metadata.
- trace_payload_roundtrip_through_fake_intake: drives TraceFlusher::flush
  through the real aggregator and asserts on decoded trace/span IDs and
  service metadata.

Tests redirect StatsFlusher at the fake-intake by passing
FakeIntake::stats_url() to StatsFlusher::new, which was lifted to take
the URL as a parameter in a prior commit.

Prototype for APMSVLS-494 phase 1; extraction to a shared
datadog/apm-agent-parity-rs crate is planned for phase 2.

Co-Authored-By: Claude <[email protected]>
lucaspimentel and others added 13 commits May 18, 2026 15:06
service_source and span_derived_primary_tags were added in 2.0.0
(crates.io) but the workspace pins the git version (1.0.0).

🤖 Co-Authored-By: Claude Code <[email protected]>
- Return Result instead of panicking on bad decompression; axum
  converts handler panics to TCP aborts, not 500s, which produces
  opaque test failures
- Warn on unrecognized Content-Encoding instead of silently
  treating as identity
- Make base_url() private; stats_url()/traces_url() cover all uses

🤖 Co-Authored-By: Claude Code <[email protected]>
base_url() was private and never called; stats_url() and traces_url()
accessed the field directly. Removing it also makes the blanket
#![allow(dead_code)] in common/mod.rs unnecessary.

🤖 Co-Authored-By: Claude Code <[email protected]>
fake_intake was re-exported from tests/common/mod.rs, causing it to be
compiled into every test binary. logs_integration_test and
metrics_integration_test never use it, so the compiler emitted dead-code
errors under RUSTFLAGS=-D warnings.

Remove the re-export from common/mod.rs and include the module directly
in apm_integration_test via #[path]. Remove the now-unused `mod common;`
declarations from logs and metrics test files.

🤖 Co-Authored-By: Claude Code <[email protected]>
The Drop impl already sends the shutdown signal and aborts the task.
The explicit shutdown() method was never called in any test.

🤖 Co-Authored-By: Claude Code <[email protected]>
@lucaspimentel
lucaspimentel force-pushed the lpimentel/fake-intake-phase-1 branch from aff7b2f to fe18778 Compare May 18, 2026 19:07
@lucaspimentel
lucaspimentel merged commit 1d687bf into main May 20, 2026
53 of 55 checks passed
@lucaspimentel
lucaspimentel deleted the lpimentel/fake-intake-phase-1 branch May 20, 2026 16:21
lucaspimentel added a commit that referenced this pull request Jun 15, 2026
…ake (#1256)

- [x] ~Note: stacked on #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 `ClientStatsPayload`
survives the roundtrip) and stats presence/suppression
(`_dd.compute_stats` truth 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 decoded `ClientGroupedStats`:

- `e2e_stats_count_aggregates_identical_spans` — 3 identical top-level
spans collapse into one group with `hits == 3`, `top_level_hits == 3`.
- `e2e_stats_counts_errors_separately_from_hits` — `errors` counts only
`error != 0`; `hits` counts all.
- `e2e_stats_sums_span_durations` — `duration` is the nanosecond sum of
aggregated spans.
- `e2e_stats_groups_by_resource` — distinct resources stay in separate
groups.
- `e2e_stats_groups_by_span_kind` — distinct `span.kind` stay in
separate groups.
- `e2e_stats_groups_by_http_status_code` — distinct `http.status_code`
stay 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_pipeline` was generalized into
`run_processor_pipeline_with_traces(.., traces)`, with a thin
single-span wrapper preserving the existing `_dd.compute_stats` tests
unchanged.

### Out of scope: time-bucketing

Deliberately not added. Bucket assignment lives entirely inside
`SpanConcentrator` and is thoroughly tested in `libdd-trace-stats`
(`test_concentrator_stats_counts` over 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 seeds `oldest_timestamp` from 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_test`
- `cargo fmt --all -- --check`
- `cargo clippy --workspace --all-targets --features default`

> *"Six tests walk into a bucket. They all collapse into one group —
turns out they were identical the whole time."* — Claude 🤖
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants