Skip to content

feat(controller): Phase D — query_language L1 trait + language_logical_plan L2 + PromQL backend - #274

Merged
zzylol merged 1 commit into
mainfrom
feat/controller-phase-d-language-wrap
May 6, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/controller-phase-d-language-wrap

Conversation

@zzylol

@zzylol zzylol commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase D of the controller-design alignment per controller/docs/design.md §6: introduces L1 (query_language) + L2 (language_logical_plan) and wraps the existing PromQL parser behind a multi-language Language trait. PromQL is the only active backend; Sql, DataFusion, ElasticDsl are stubbed with ParseError::Unimplemented so the type system is uniform while the DC build ships PromQL only.

  • L1 (controller/src/query_language/): Language trait + LanguageAst sum + per-language sub-modules. PromQL backend is a thin adapter over query_parser::{parse_query, parse_query_expr} — no parsing logic duplicated.
  • L2 (controller/src/language_logical_plan/): LanguageLogicalPlan (one variant per language) + lower_to_logical_plan(LanguageAst) -> LanguageLogicalPlan pass. PromQL's L2 IS the existing QueryExpr tree (rebadged + paired with a flat LanguageLogicalPlanSummary projection).
  • Legacy query_parser/ is read-only — its public surface (parse_query, parse_query_expr, ParsedQuery) stays unchanged for back-compat with analyzer::Analyzer.

Module layout

controller/src/query_language/
  ├── language.rs          — Language trait + ParseError
  ├── language_ast.rs      — LanguageAst sum
  ├── promql/{mod,ast}.rs  — active PromQL backend (wraps query_parser)
  ├── sql/mod.rs           — stub (Unimplemented)
  ├── datafusion/mod.rs    — stub (Unimplemented)
  ├── elastic_dsl/mod.rs   — stub (Unimplemented)
  └── tests.rs             — 8 unit tests

controller/src/language_logical_plan/
  ├── lower.rs             — lower_to_logical_plan + LoweringError
  ├── plan.rs              — LanguageLogicalPlan + LanguageLogicalPlanSummary
  ├── mod.rs               — re-exports
  └── tests.rs             — 6 unit tests

Coordination with Phase B

controller/src/main.rs add: mod query_language; mod language_logical_plan; — may conflict with Phase B's mod intent_algebra;. Trivial 3-way merge. The crate has no lib.rs (bin-only), so module declarations live in main.rs instead — orchestrator should treat both phases' edits the same way regardless.

Pre-existing test breakage (NOT introduced here)

origin/main at be44752 (post-#273) has 9 pre-existing E0063 errors in test code in controller/src/config/{agent,asapquery_backend,precompute}.rs and controller/src/main.rs::api_tests — all reading missing field 'data_sink' in initializer of types::AgentCollectorConfig. #273 added data_sink to AgentCollectorConfig but missed those test sites. Per Phase D constraints I cannot touch types.rs, config/, or main.rs::api_tests, so I left them alone. Consequence:

  • cargo build --release -p controllerclean (my new modules add zero warnings).
  • cargo check --release --tests for query_language / language_logical_planclean (my new test code compiles).
  • cargo test --release -p controllerfails to build due to pre-existing feat(controller): additive QuerySpec types from design.md (QueryShape/DataShape/AccuracyTarget/QueryLanguage) #273 oversight, NOT due to anything in this PR.
  • cargo clippy --release -p controller --all-targets -- -D warningsfails on pre-existing KLL/HLL/etc. naming-acronym warnings in types.rs; zero clippy hits in this PR's new modules (cargo clippy --release filtered to query_language|language_logical_plan returns nothing).

Recommend a follow-up trivial fix-test PR (or amend #273) to populate data_sink in those constructors.

Test plan

  • After the data_sink follow-up lands: cargo test --release -p controller query_language — all 8 new tests green.
  • After the data_sink follow-up lands: cargo test --release -p controller language_logical_plan — all 6 new tests green.
  • After the data_sink follow-up lands: cargo test --release -p controller query_parser — existing tests not regressed (this PR does not modify query_parser/).
  • cargo build --release -p controller clean.
  • No new clippy warnings on query_language/ or language_logical_plan/.
  • Submodule pointers unchanged.

🤖 Generated with Claude Code

…l_plan L2 + PromQL backend

Introduces the L1 (query_language) and L2 (language_logical_plan)
modules per design.md §6. Wraps the existing PromQL parser behind a
multi-language Language trait without touching the legacy
query_parser surface; SQL, DataFusion, and ElasticDsl backends are
stubs returning ParseError::Unimplemented so the type system stays
uniform while the DC build ships PromQL only.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 68135e3 into main May 6, 2026
zzylol added a commit that referenced this pull request May 6, 2026
…nfig constructors (#276)

PR #204 added the `data_sink: AgentDataSink` field to
`AgentCollectorConfig` (production sites updated) but missed nine
test-fixture / test-code constructors, leaving
`cargo test --release -p controller` blocked at compile-time. This
broke the baseline for downstream PRs (#273, #274, #275) which could
not run their newly added unit tests.

Mechanical fill-in only, no semantic change to existing tests:
- 7 sites use `AgentDataSink::default()` (Otlp-to-backend) — the
  canonical default that PR #204 introduced for new pipelines.
- 2 sites (`config::agent::tests::ddsketch_cfg` and
  `main::api_tests::generated_agent_yaml_contains_opamp_extension`)
  pin `AgentDataSink::PrometheusScrape { endpoint: "0.0.0.0:8889" }`
  because their pre-existing assertions check for the legacy
  `prometheus` exporter on :8889. Pinning the sink keeps the test
  semantics intact rather than rewriting the asserts.

After the fix:
- `cargo build --release -p controller` clean.
- `cargo test --release -p controller --no-run` clean (was the
  blocker).
- `cargo test --release -p controller` runs 395 tests; 389 pass,
  6 pre-existing failures unrelated to `data_sink`:
    * 2 in `analyzer::tests` — float-precision asserts.
    * 4 in `opamp::tests` / `api_tests` — protobuf framing
      ("invalid tag value: 0") on `ServerToAgent` decode.
  These are tracked separately and out of scope for this PR.
- Newly-shipped tests now run end-to-end:
  `query_language` (8), `language_logical_plan` (6),
  `types_v2` (7), `algebra` (intent_algebra family, 6+).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the feat/controller-phase-d-language-wrap branch May 9, 2026 18:00
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