Skip to content

feat(metrics): expose §6.3 barrier drops as Prometheus counter - #47

Merged
zzylol merged 1 commit into
mainfrom
feat/barrier-counter-metric
Apr 20, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/barrier-counter-metric

Conversation

@zzylol

@zzylol zzylol commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #45. The §6.3 write-side schema barrier had a counter (`IngestState::samples_blocked_by_schema_barrier`) and a batched debug log, but both were in-process only. Production deployments running at `INFO` had no graphable signal for silent drops — the exact scenario the counter was meant to make visible.

What's in this PR

  • New `stores::sketch_db::metrics` module, mirroring the `promsketch_store::metrics` pattern. Registers a Prometheus `CounterVec`:
    • name: `queryengine_ingest_samples_blocked_by_schema_barrier_total`
    • labels: `agg_id`
  • `route_decoded_samples` bumps the counter per-agg right alongside the existing atomic + debug log, fed by the same per-batch `dropped_by_barrier` tally.
  • New unit test `barrier_prom_counter_increments_per_agg_label` (keyed on a unique agg_id=9001) to get a deterministic baseline in the process-global `lazy_static` registry.

Why label by agg_id

Ops can alert on "drops on a specific agg_id while the registry still reports that agg Active" — the silent-drop regression the counter is meant to catch. A scalar counter (no labels) would hide which agg is leaking.

Validation

  • `cargo test -p query_engine_rust --lib` — 729 pass (was 726; +3 from feat(ingest): §6.3 barrier observability — counter, debug log, tests #45 + 1 here)
  • `cargo clippy --all-targets -- -D warnings` — clean
  • `cargo fmt --all -- --check` — clean
  • The existing `/metrics` handler uses `prometheus::gather()` on the default registry, so this counter appears on scrape without further plumbing.

Follow-ups

  • OTLP ingest paths (`drivers/ingest/otel.rs` lines 375 / 437 / 646) have the same barrier check, still unmetered — parallel patch when DataCollector sketch traffic flows through that path in earnest.

🤖 Generated with Claude Code

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
zzylol merged commit 826302d into main Apr 20, 2026
@zzylol
zzylol deleted the feat/barrier-counter-metric branch April 20, 2026 16:11
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>
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.

1 participant