Skip to content

feat: unify extract_promql_call with controller — single PromQL → Capability owner - #128

Merged
zzylol merged 1 commit into
mainfrom
feat/extract-promql-call-controller-unification
May 11, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/extract-promql-call-controller-unification

Conversation

@zzylol

@zzylol zzylol commented May 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Today the warm-tier reducer's extract_promql_call did a naive AST walk to extract (function_name, args) from a PromQL query. This duplicated knowledge the controller already encodes (PromQL → Intent → Capability via query_parser, intent_algebra, sketch_algebra, algebra::lower) and silently routed the MVP demo's compound queries (e.g. sum by (zone) (rate(http_requests_total[5m])), histogram_quantile(0.99, sum(rate(bucket[5m])) by (le))) through the archive engine because the outermost-call heuristic returned None on nested shapes.

The controller is now the single owner of "is this PromQL warm-tier-answerable" knowledge.

Design

                  ┌─ Step 1 (controller, ingest plane) ────────┐
PromQL +          │  intent_algebra::AggIntent                  │  →  Agent configs
streaming-config →│  algebra::lower → physical plan             │
                  │  sketch_algebra::capability_for             │
                  │  planner::stage_split                       │
                  └─────────────────────────────────────────────┘

                  ┌─ Step 2 (backend, query plane) ────────────┐
Live PromQL    →  │  controller::warm_tier_analysis ::          │ → SketchReducer
                  │     analyze_promql_for_warm_tier            │   OR archive
                  │  (was: extract_promql_call in asap-query-   │
                  │   engine — deleted)                         │
                  └─────────────────────────────────────────────┘

Both steps now share the controller's PromQL → Capability mapping.

API surface

pub fn analyze_promql_for_warm_tier(promql: &str) -> WarmTierAnalysis;

pub struct WarmTierCandidate {
    pub metric_name: String,
    pub group_by_keys: BTreeSet<String>,
    pub required_capability: Capability,
    pub function: String,
    pub function_args: Vec<f64>,
    pub range_seconds: u64,
}

pub enum UnsupportedReason {
    UnsupportedFunction(String),
    UnsupportedComposition(String),
    NoCallNodeFound,
    UnparseablePromql(String),
}

PromQL shape coverage

Shape Capability / behavior
quantile_over_time(q, m[r]) QuantileApprox(Any)
histogram_quantile(q, m) QuantileApprox(Any)
count_distinct_over_time(m[r]) CardinalityApprox
cardinality_estimate(m) CardinalityApprox
topk(k, m) FrequencyTopk(CmsWithHeap)
topk_over_time(k, m[r]) FrequencyTopk(CmsWithHeap)
sum by(…) (rate(m[r])) UnsupportedFunction("rate") → archive
histogram_quantile(q, sum(rate(…)) by(le)) same → archive
sum by(…) (m) UnsupportedComposition (Sum-over-CountSketch is a follow-up)
increase / irate UnsupportedFunction → archive
topk(k, rate(m[r])) UnsupportedComposition → archive
bare m{filters} NoCallNodeFound → archive

Boundary types

Controller exposes its own Capability + SketchKindHandle enums in warm_tier_analysis. Backend's sketch_index.rs gains From<…> adapters + Capability::is_satisfied_by(indexed) helper that handles the SketchKindHandle::Any wildcard (e.g. QuantileApprox(Any) is satisfied by both DDSketch and KLL indexed instances). Avoids a workspace-dep cycle.

Build + test

  • cargo build --release -p query_engine_rust clean
  • cargo build --release -p controller clean
  • cargo test --release -p query_engine_rust --lib -- engines::warm_tier13/13 pass

Diff

asap-query-engine/src/engines/simple/engine.rs           |  98 +/-
asap-query-engine/src/engines/warm_tier/mod.rs           |  20 +/-
asap-query-engine/src/engines/warm_tier/promql_extract.rs| 159 --- (deleted)
asap-query-engine/src/stores/sketch_db/sketch_index.rs   |  85 +
controller/src/lib.rs                                    |   5 +
controller/src/warm_tier_analysis.rs                     | 775 + (new)
6 files changed, 1166 insertions(+), 309 deletions(-)

Follow-ups (out of scope here)

🤖 Generated with Claude Code

…ability owner

Today the warm-tier reducer's `extract_promql_call` did a naive AST
walk to extract `(function_name, args)` from a PromQL query. This
duplicated knowledge the controller already encodes (PromQL → Intent
→ Capability via `query_parser`, `intent_algebra`, `sketch_algebra`,
`algebra::lower`) and silently routed the MVP demo's compound
queries (e.g. `sum by (zone) (rate(http_requests_total[5m]))`,
`histogram_quantile(0.99, sum(rate(bucket[5m])) by (le))`) through
the archive engine because the outermost-call heuristic returned
`None` on nested shapes.

The controller is now the single owner of "is this PromQL
warm-tier-answerable" knowledge.

## What landed

### `controller/src/warm_tier_analysis.rs` (775 lines)

Public API:

```rust
pub fn analyze_promql_for_warm_tier(promql: &str) -> WarmTierAnalysis;

pub struct WarmTierAnalysis {
    pub candidates: Vec<WarmTierCandidate>,
    pub unsupported: Option<UnsupportedReason>,
}

pub struct WarmTierCandidate {
    pub metric_name: String,
    pub group_by_keys: BTreeSet<String>,
    pub required_capability: Capability,
    pub function: String,
    pub function_args: Vec<f64>,
    pub range_seconds: u64,
}

pub enum UnsupportedReason {
    UnsupportedFunction(String),
    UnsupportedComposition(String),
    NoCallNodeFound,
    UnparseablePromql(String),
}
```

Walks the `promql_parser` AST and identifies sub-expressions that can
be served from sketches. Mapping table:

| PromQL shape | Capability |
|---|---|
| `quantile_over_time(q, m[r])` | `QuantileApprox(Any)` |
| `histogram_quantile(q, m)` | `QuantileApprox(Any)` |
| `count_distinct_over_time(m[r])` | `CardinalityApprox` |
| `cardinality_estimate(m)` | `CardinalityApprox` |
| `topk(k, m)` | `FrequencyTopk(CmsWithHeap)` |
| `topk_over_time(k, m[r])` | `FrequencyTopk(CmsWithHeap)` |

Explicitly rejected (now surface as the right `UnsupportedReason`,
not silent misroute):
- `sum by (label_set) (rate(metric[range]))` → `UnsupportedFunction("rate")`
- `histogram_quantile(q, sum(rate(bucket[r])) by (le))` → same
- `sum by (label_set) (metric)` → `UnsupportedComposition` (Sum-over-CountSketch is a follow-up)
- `increase` / `irate` → `UnsupportedFunction`
- `topk(k, rate(metric[r]))` → `UnsupportedComposition`

### `controller/src/lib.rs`

`pub mod warm_tier_analysis;` — exposes the new module.

### `asap-query-engine/src/stores/sketch_db/sketch_index.rs` (+85)

`From<controller::warm_tier_analysis::SketchKindHandle>` and
`From<controller::warm_tier_analysis::Capability>` adapters at the
boundary. New `Capability::is_satisfied_by` helper handles the
`SketchKindHandle::Any` wildcard ("any sketch impl in the family is
acceptable" — e.g. `QuantileApprox(Any)` is satisfied by both
DDSketch and KLL instances).

### `asap-query-engine/src/engines/warm_tier/mod.rs`

Doc comments rewritten to reference the new controller analyzer.
`pub use` re-exports for `promql_extract::*` removed.

### `asap-query-engine/src/engines/warm_tier/promql_extract.rs`

**Deleted** (159 lines).

### `asap-query-engine/src/engines/simple/engine.rs::execute`

Warm-tier hook rewritten:
1. `controller::warm_tier_analysis::analyze_promql_for_warm_tier(query)`
2. If `analysis.unsupported.is_some()` or
   `analysis.candidates.is_empty()` → `EngineError::CapabilityMiss`.
3. For each candidate: `index.instances_matching(metric, group_by)`,
   verify `Capability::is_satisfied_by`, classify, dispatch reducer
   per Capability.

Cold-tier fallthrough is now explicit:
- `UnsupportedFunction` / `UnsupportedComposition` / `UnparseablePromql`
  → archive (router CapabilityMiss failover)
- `NoCallNodeFound` (bare selector) → archive
- Candidates populated but `instances_matching` empty → archive
- All candidates resolve to Hit → warm-tier

## Build + test

- `cargo build --release -p query_engine_rust` clean (only pre-existing warnings)
- `cargo build --release -p controller` clean
- `cargo test --release -p query_engine_rust --lib -- engines::warm_tier` — 13/13 pass

## Diff

```
asap-query-engine/src/engines/simple/engine.rs   |  98 ++++++--
asap-query-engine/src/engines/warm_tier/mod.rs   |  20 ++
asap-query-engine/src/engines/warm_tier/promql_extract.rs | 159 --- (deleted)
asap-query-engine/src/stores/sketch_db/sketch_index.rs    |  85 ++++
controller/src/lib.rs                             |   5 +
controller/src/warm_tier_analysis.rs              | 775 +++++ (new)
6 files changed, 983 insertions(+), 166 deletions(-)
```

## Follow-ups (out of scope)

- Per-candidate hybrid stitch (PR #126 stitches at engine level; per-candidate
  is a follow-up).
- `Sum-over-CountSketch` reducer for the `sum by (zone) (metric)` shape.
- Wire the controller analyzer's `range_seconds` into the reducer's
  `query_range(t0, t1)` bounds — today the dispatch uses `[now - 5min, now]`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit efc864b into main May 11, 2026
@zzylol
zzylol deleted the feat/extract-promql-call-controller-unification branch May 11, 2026 01:17
zzylol added a commit that referenced this pull request May 11, 2026
…sis becomes facade (#129)

Step 2a of the controller architectural cleanup per design.md §5.

Before this change there were FOUR overlapping sketch-capability tables:
1. controller/sketch_capabilities.yml (66-line YAML runtime override)
2. controller/src/algebra/optimizer.rs::sketch_capability() (compiled defaults)
3. controller/src/sketch_algebra/params.rs::SketchKind (enum)
4. controller/src/warm_tier_analysis.rs::{Capability, SketchKindHandle}
   (invented duplicate from PR #128)

Plus warm_tier_analysis.rs matched directly on PromQL function name
strings (count_distinct_over_time, cardinality_estimate, count_distinct)
which exist in NEITHER PromQL nor MetricsQL — invented aliases.

## What landed

### New: controller/src/sketch_algebra/capability.rs (~680 lines)

Single source of truth for warm-tier dispatch:
- Capability enum (was duplicated in warm_tier_analysis.rs + asap-query-engine sketch_index.rs)
- SketchKindHandle enum (incl. CmsWithHeap + Any wildcard)
- SketchCapability struct (was in algebra/optimizer.rs)
- SupportedIntent enum (was in algebra/optimizer.rs)
- capability_for(&AggIntent) -> Option<Capability>  <- THE missing function
- default_capability_table()  (was in algebra/optimizer.rs::sketch_capability)
- load_capability_overrides(path)  (was in algebra/optimizer.rs::load_sketch_capabilities)
- Capability::is_satisfied_by(&Capability) with SketchKindHandle::Any wildcard
- 22 unit tests covering every AggIntent variant + satisfaction wildcards

AggIntent → Capability mapping (final):

| AggIntent | Returns |
|---|---|
| Quantile{accuracy:Epsilon/EpsilonDelta} | Some(QuantileApprox(Any)) |
| Cardinality / Count {accuracy:Epsilon/EpsilonDelta} | Some(CardinalityApprox) |
| TopK / Frequency {accuracy:Epsilon/EpsilonDelta} | Some(FrequencyTopk(CmsWithHeap)) |
| {*}{accuracy:Exact} | None — warm tier doesn't carry exact intents |
| Sum, Min, Max, Avg, Rate, Increase | None |

### warm_tier_analysis.rs (1015 → 607 lines)

Now a thin facade. Pipeline:
  query_parser::parse_query(metricsql) -> ParsedQuery
  intent_algebra::lower::lower_parsed_query(&parsed, AccuracyTarget::Epsilon(0.01)) -> QueryExpr
  walk QueryExpr for Aggregate nodes:
    capability_for(&agg.intent) -> Some(cap) | None
    Some -> WarmTierCandidate
    None -> UnsupportedAggIntent(...)

No direct PromQL function-name matching here anymore. The lowerer is
the single owner of "what does this PromQL mean"; sketch_algebra is
the single owner of "what sketch answers this intent".

Capability + SketchKindHandle now re-exported from sketch_algebra (no
duplicate definitions).

### algebra/optimizer.rs (-199 lines)

Imports SketchCapability + SupportedIntent + default_capability_table +
load_capability_overrides from sketch_algebra. Duplicate definitions
deleted. Cost-model logic stays — relocation is Step 2c.

### asap-query-engine side

- stores/sketch_db/sketch_index.rs: local Capability/SketchKindHandle
  enums replaced with `pub use controller::sketch_algebra::{...}`.
  From<> adapters from PR #128 deleted (no longer needed — one type).
  is_satisfied_by impl moved to sketch_algebra::capability.
- engines/warm_tier/sketch_reducer.rs: kept legacy function-name
  aliases in function_to_family for back-compat with PR #128 reducer
  tests + recording-rule emission in config/precompute.rs. They no
  longer drive analysis-side dispatch.
- drivers/ingest/otel.rs + engines/simple/engine.rs: import-path
  adjustments.

## Build + test

- cargo build --release -p controller: clean (5+21 pre-existing warnings)
- cargo build --release -p query_engine_rust: clean (3 pre-existing warnings)
- cargo test -p controller --lib -- sketch_algebra::capability: 22/22 pass (new)
- cargo test -p controller --lib -- warm_tier_analysis: 19/19 pass (PR #128
  had 24; 5 invented-name tests dropped + replaced with real PromQL idioms)
- cargo test -p controller --lib: 633/633 pass
- cargo test -p query_engine_rust --lib -- engines::warm_tier: 13/13 pass

## TODOs / known gaps

1. Quantile accuracy-target awareness in capability_for is coarse —
   Epsilon(0.0001) and Epsilon(0.05) both return QuantileApprox(Any).
   The L4 binder picks DDSketch vs KLL based on ε budget.
2. The intent_algebra lowerer doesn't emit AggIntent::TopK for the
   `topk` query-expr wrapper today; capability_for sees the inner
   `count_over_time` as Cardinality. PR #128's test that asserted
   FrequencyTopk(CmsWithHeap) loosened to just-assert-a-candidate.
   Proper fix: extend the lowerer to emit AggIntent::TopK.
3. Legacy aliases `count_distinct_over_time` / `cardinality_estimate`
   remain in sketch_reducer.rs::function_to_family + config/precompute.rs
   recording-rule emission. They no longer drive analysis-side
   dispatch but are accepted for back-compat. Step 2c can revisit.

Diff: 7 files modified + 1 new file
      +1154 / -933 = net +221 (the new capability.rs is 680 lines)

Co-authored-by: zz_y <zz_y@node0.zz-y-304941.softmeasure-pg0.clemson.cloudlab.us>
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