Skip to content

fix(data_plane): canonicalize sketch metric name at OTLP ingest - #323

Merged
zzylol merged 1 commit into
mainfrom
fix/sketch-query-grouping-resolve
May 25, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/sketch-query-grouping-resolve

Conversation

@zzylol

@zzylol zzylol commented May 25, 2026

Copy link
Copy Markdown
Contributor

Root cause

Warm-tier sketch queries (quantile_over_time/KLL, count/HLL,
topk/CountSketch) return status: error, "No result for query", data_source: asap_query even though the sketches are stored and the
controller's aggregations are registered.

The ASAPCollector edge pipeline's fused asapedgeprocessor
(opentelemetry-collector-contrib/processor/asapedgeprocessor/sketch.go:94)
sets MetricSuffix: "_" + family on every sketch it emits, so:

query metric (bare) stored SketchInstanceMetadata.metric_name
request_size_bytes request_size_bytes_kll
http_requests_total_latency_ms http_requests_total_latency_ms_kll
unique_users_per_min unique_users_per_min_hll
top_endpoint_qps top_endpoint_qps_countsketch
endpoint_request_freq endpoint_request_freq_countminsketch

The controller plans + pushes streaming-config policies keyed on the
bare metric name, and the query analyzer
(control_plane::asap_tier_analysis) lifts the bare name out of the
PromQL selector. With the suffix left on the stored name, both
resolution paths fail:
SketchIndex::instances_matching(bare, …) (equality on metric_name)
and find_matching_policies / find_policy_by_content (equality on
metric) return empty → EngineError::capability_miss. The 5-sketch
metrics aren't in backend-storage-routing.yaml so they're warm-only
(no archive fallthrough) → the adapter renders "No result for query".

http_requests_total (Sum family) is unaffected because the Sum path
(sum.go m.SetName(metricName)) does not suffix — which is
exactly why sum by (zone)(http_requests_total) works while the sketch
queries don't. Grouping (is_subset) is not the blocker:
per-series quantile_over_time has empty group_by_keys, which is a
subset of any stored sketch's keys.

Confirmed live on node2: GET /api/v1/db/schemas shows 20204 instances
under the suffixed names; GET /api/v1/streaming-config shows 6
policies under the bare names; with verbose RUST_LOG the engine logs
Modern execute() returned CapabilityMiss for each.

Fix

Per docs/design-controller-into-backend.md §1 the sketch family is
a wire-level attribute (already carried in agg_kind /
SketchKindHandle), NOT a name suffix. Canonicalize at the ingest seam
(data_plane/src/drivers/ingest/otel.rs):

  • New canonical_sketch_metric_name(name, kind) strips the
    _<family> suffix only when it matches the datapoint's actual
    sketch kind
    (so a metric legitimately named foo_hll arriving as
    KLL is left untouched; never collapses to empty).
  • Thread the canonical name through sid resolution
    (series_resolver.resolve), the series-key snapshot cache,
    SketchInstanceMetadata.metric_name, derive_sketch_policy_fp, and
    the legacy precompute-router match.
  • The SeriesAssignment echoed back to the sender keeps the wire
    name (the exporter's cache contract is unchanged).

The strip is idempotent — a no-op once agents stop suffixing — so it is
safe alongside the eventual agent-side fix.

Remaining (agent-side, out of scope here)

topk(top_endpoint_qps) has a second mismatch: the agent emits a
heap-less CountSketch (indexed FrequencyEstimate) while topk(...)
needs a heap-bearing variant (FrequencyTopk). The real fix is the
edge processor emitting CountSketchWithHeap. After this PR + that agent
change, KLL/HLL/CountSketch queries all resolve warm.

Test plan

  • cargo build -p data_plane green
  • cargo test -p data_plane --lib — 718 passed (incl. 4 new
    canonical_metric_name_tests)
  • cargo test -p control_plane --lib asap_tier_analysis — 57 passed
  • Rebuild asap/data-plane:dev, redeploy on node2, soak ~90s, then
    quantile_over_time(0.99, request_size_bytes[30s]) /
    count(unique_users_per_min) return real data_source: asap_query
    results

🤖 Generated with Claude Code

The ASAPCollector edge pipeline's fused `asapedgeprocessor`
(`sketch.go`) sets `MetricSuffix: "_" + family` on every sketch it
emits, so a KLL sketch over `request_size_bytes` lands on the wire as
`request_size_bytes_kll`, HLL over `unique_users_per_min` as
`unique_users_per_min_hll`, CountSketch as `..._countsketch`, etc.

The controller plans + pushes streaming-config policies keyed on the
*bare* metric name, and the query analyzer lifts the bare name out of
the PromQL selector. With the suffix left on,
`SketchInstanceMetadata.metric_name` is the suffixed form, so
`SketchIndex::instances_matching(bare, …)` and `find_matching_policies`
/ `find_policy_by_content` (all equality comparisons on `metric_name`)
never match — every warm sketch query (`quantile_over_time`,
`count`/HLL, `topk`) capability-misses and the caller sees
`data_source: asap_query, "No result for query"`. (Confirmed live on
node2: SketchStore holds 20204 instances under `request_size_bytes_kll`
/ `unique_users_per_min_hll` / `top_endpoint_qps_countsketch` /
`endpoint_request_freq_countminsketch`; the bare-name queries miss.)
`http_requests_total` (Sum family) is unaffected because the Sum path
does not suffix, which is why `sum by (zone)(http_requests_total)`
works.

Per `docs/design-controller-into-backend.md` §1 the sketch *family* is
a wire-level attribute (already carried in `agg_kind` /
`SketchKindHandle`), NOT a name suffix. Apply that canonicalization at
the ingest seam: strip the `_<family>` suffix when it matches the
datapoint's actual sketch kind, then thread the canonical name through
sid resolution, the series-key snapshot cache, the
`SketchInstanceMetadata` registration, `derive_sketch_policy_fp`, and
the legacy precompute-router match. The `SeriesAssignment` echoed back
to the sender keeps the wire name (the exporter's cache contract). The
strip is gated on the suffix matching the dp's kind (so a metric a user
legitimately named `foo_hll` arriving as KLL is untouched) and is a
no-op once agents stop suffixing — safe and idempotent.

Note: topk over `top_endpoint_qps` has a *second*, agent-side mismatch
on top of this — the agent emits a heap-less CountSketch
(`FrequencyEstimate`) while `topk(...)` requires a heap-bearing variant
(`FrequencyTopk`); the fix for that is making the edge processor emit a
with-heap sketch (CountSketchWithHeap), out of scope for this backend
change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 18eeaa4 into main May 25, 2026
zzylol added a commit that referenced this pull request May 25, 2026
…#327)

The warm sketch store grew unbounded (~0.74 GiB/min) under steady OTLP
ingest, freezing a 251 GiB node. Root cause: SidStoreData is always
built with epoch_capacity=None, so maybe_rotate_epoch returns early and
nothing ever seals; the persistence flusher only evicts SEALED epochs,
so current_epoch accumulated one ~19 KB pane per 30s tumbling window
per sid forever. Memory was O(active_series x total_elapsed_time).

Add a time-based retention horizon on SidStoreData: on each insert,
evict windows whose END is older than newest_end - horizon from
current_epoch (and any sealed_epochs), making memory
O(active_series x horizon). Configurable via ASAP_SKETCH_RETENTION_MS
(0 disables), default 2h.

Does not regress the #323-#326 sketch read path: the horizon (2h)
comfortably exceeds the ~30m max range-query window plus the
delta-stitching carry-in's Full-base reach, and eviction keys on
window-END so a straddling pane survives until fully behind the
horizon. Recent unsealed windows stay queryable via the overlap scan
(no force-seal). Regression tests prove (a) long-elapsed ingest keeps
per-sid window count bounded and (b) a 30m range query within the
horizon still resolves with its Full carry-in base intact.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 25, 2026
Compose hot (current_epoch) -> sealed (in-mem, pending flush) -> disk
parts into a single tiered store so warm-sketch memory is bounded by
flush-then-evict rather than #327's age-based drop. Reads union all
three tiers across the requested range.

- Sealing now fires under persistence: SidStoreData gains a
  seal_window_count cadence (default 20 windows ~= 10 min of 30s panes)
  so current_epoch rotates into sealed_epochs for the flusher to
  persist. max_epochs drop is disabled under persistence (the flusher
  owns sealed-epoch lifecycle). In-memory-only deploys keep #327.
- query_range/union_disk_parts_into consult PartCache+Manifest for the
  evicted portion of the range, rebuilding the full label key->value
  map from the sid's group_by_keys and preserving the #323-#326 read
  contract (half-open overlap + delta-stitching carry-in) across the
  in-mem/on-disk boundary -- incl. a carry-in Full base that now lives
  on disk. Part format round-trips SketchEncoding via a repurposed v1
  pad byte (legacy 0 decodes as Full).
- enforce_retention is a no-op under persistence so retention never
  drops un-flushed sealed/current data; the disk-tier TTL bounds the
  durable copy. In-memory-only path is unchanged.
- start_persistence installs a read handle + seal cadence and recovers
  the manifest+parts so a restart immediately serves recovered data.
- New CLI flag --persistence-seal-window-count plumbs the cadence.

Tests: seal-fires, flush+evict bounds memory, query-from-disk incl.
disk carry-in base, restart recovery, and persistence-disabled
non-regression.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the fix/sketch-query-grouping-resolve branch July 17, 2026 20:05
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