feat(ingest): §6.3 barrier observability — counter, debug log, tests - #45
Merged
Merged
Conversation
The write-side schema barrier at `ingest_handler.rs` already drops samples for retired/expired agg_ids (§6.3 of the sketch DB design), but the drop was silent: no counter, no log. The e2e after #42 tried to confirm the barrier fires by looking at the logs and, finding nothing, concluded the barrier wasn't wired — a false alarm that would have sent a bug report down the wrong path. Changes: - `IngestState::samples_blocked_by_schema_barrier: AtomicU64`. Incremented exactly when `is_writable(agg_id)` returns false for a matching agg. One ingest sample can contribute more than once when multiple agg configs match the same metric. - Per-batch `debug!` log after the routing loop: total_dropped + per-agg_id breakdown. Batched (not per-sample) so a hot ingest path doesn't flood the log. - `route_decoded_samples` promoted to `pub(crate)` so the test module can call it directly. - Three unit tests in `precompute_engine::ingest_handler::tests`: - Active schema → counter stays 0. - `force_expire` → counter increments by the matching sample count in the next batch. - Non-matching metrics → counter stays 0 even after expire. 726 lib tests pass (was 723; +3), clippy clean, fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Apr 20, 2026
Follow-up from #45: the barrier counter was only visible via the in-process AtomicU64 + debug log, so production deployments running at INFO had no graphable signal for silent drops. Changes: - New `stores::sketch_db::metrics` module with a `CounterVec` `queryengine_ingest_samples_blocked_by_schema_barrier_total`, keyed by `agg_id`, registered through the global `prometheus::default_registry()` so the existing `/metrics` handler (`handle_metrics` in drivers/query/servers/http.rs) scrapes it automatically. - Per-agg increment right alongside the atomic bump in `route_decoded_samples`, fed by the same per-batch `dropped_by_barrier` tally so the counter and the log stay consistent. - New unit test `barrier_prom_counter_increments_per_agg_label` that keys on a unique agg_id (9001) to get a deterministic baseline in the process-global registry. Why label by agg_id (not a scalar counter): lets operators alert on "drops on a specific agg while the registry still reports that agg Active" — the silent-drop regression the counter is meant to catch. 729 lib tests pass (+1), clippy clean, fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Apr 20, 2026
Follow-up from #45: the barrier counter was only visible via the in-process AtomicU64 + debug log, so production deployments running at INFO had no graphable signal for silent drops. Changes: - New `stores::sketch_db::metrics` module with a `CounterVec` `queryengine_ingest_samples_blocked_by_schema_barrier_total`, keyed by `agg_id`, registered through the global `prometheus::default_registry()` so the existing `/metrics` handler (`handle_metrics` in drivers/query/servers/http.rs) scrapes it automatically. - Per-agg increment right alongside the atomic bump in `route_decoded_samples`, fed by the same per-batch `dropped_by_barrier` tally so the counter and the log stay consistent. - New unit test `barrier_prom_counter_increments_per_agg_label` that keys on a unique agg_id (9001) to get a deterministic baseline in the process-global registry. Why label by agg_id (not a scalar counter): lets operators alert on "drops on a specific agg while the registry still reports that agg Active" — the silent-drop regression the counter is meant to catch. 729 lib tests pass (+1), clippy clean, fmt clean. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
zzylol
added a commit
that referenced
this pull request
Apr 20, 2026
) Follow-up to #45 / #47. The Prometheus `CounterVec` `queryengine_ingest_samples_blocked_by_schema_barrier_total` (keyed by `agg_id`) was only bumped by the Prometheus / VictoriaMetrics remote-write path. The three OTLP barrier sites in `drivers/ingest/otel.rs` still silently dropped. This PR completes the observability story — no matter which driver the DataCollector ships through, a drop increments the same counter. Changes: - **`IngestState::record_barrier_drop(agg_id, count)`** — single helper that bumps both the in-process atomic AND the Prometheus `CounterVec`. All five ingest drivers funnel through it, so the `/metrics` number is a unified sum. - **`ingest_handler.rs`** — refactored `route_decoded_samples` to call the helper instead of maintaining its own inline atomic-increment + Prometheus-increment loop. Same batched debug-log semantics; fewer moving parts. - **`otel.rs`** — new local `flush_barrier_drops(state, map, driver_tag)` helper; each of the three barrier sites (`otlp-raw`, `otlp-sketch-envelope`, `otlp-modified-proto`) tallies drops in a `HashMap<agg_id, count>` and calls flush at loop exit. One summary debug log per driver per batch. Also: a new unit test `record_barrier_drop_advances_atomic_and_prom_counter` that asserts both sides of the helper's contract (atomic delta == 7 and Prometheus CounterVec delta == 7 when called with count=7 on a fresh agg_id label). 738 lib tests pass (+1), clippy clean, fmt clean. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
The §6.3 write-side schema barrier at
ingest_handler.rs:156has been dropping samples for retired/expired agg_ids since Phase 2a, but the drop was silent (no counter, no log line). The e2e that followed #42 tried to confirm the barrier was firing by grepping logs, found nothing, and concluded the barrier wasn't wired — a false alarm that would have sent a bug fix down the wrong path.This PR is the observability patch: the barrier semantics don't change; we just make it provable that the barrier is doing its job.
What's in this PR
IngestState::samples_blocked_by_schema_barrier: AtomicU64. Incremented exactly whenis_writable(agg_id)returns false for a metric-matching agg config. (Note: one ingest sample can contribute >1 drop if multiple agg configs match the same metric name and more than one is non-Active.)debug!log after the routing loop: total dropped plus per-agg_id breakdown. Aggregated once per batch — a hot ingest path won't flood the log.route_decoded_samplesis nowpub(crate)so the new test module can exercise it directly without spinning up the full engine.Why this is a non-fix
The Explore audit confirmed the barrier is already wired on main at
ingest_handler.rs:156, both on the Prometheus-remote-write and OTLP paths. The e2e misdiagnosis stemmed from trusting log silence as evidence. Adding the counter means the next e2e can assert `samples_blocked_by_schema_barrier > 0` post-force-expire and know the barrier works, rather than guessing from logs.Test plan
cargo test -p query_engine_rust --lib— 726 pass (was 723; +3 new tests)cargo clippy --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleanbarrier_counter_stays_zero_when_schema_is_activebarrier_counter_increments_after_force_expirebarrier_counter_ignores_non_matching_metricsFollow-ups (out of scope here)
/api/v1/store/metricsor/metricsendpoint so operators can poll it directly.drivers/ingest/otel.rs(three barrier checks already live there at lines 375 / 437 / 646).🤖 Generated with Claude Code