Skip to content

Retire the collector-facing emission pipeline - #725

Merged
zzylol merged 3 commits into
mainfrom
refactor/703-collector-emit-v2
Sep 14, 2026
Merged

zzylol merged 3 commits into
mainfrom
refactor/703-collector-emit-v2

Conversation

@zzylol

@zzylol zzylol commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Final step of #703 item 18, after #713, #715 and #718. Replaces #719, which was rebuilt on the current tree rather than replayed through #714's renames.

−23,503 / +801.

Why this lands as one commit

With the legacy planning endpoints gone (#718), the only remaining entry into the collector config chain was GET /api/v1/collector-config/agent, the HTTP config provider for collector bootstrap. Removing it retires the whole chain, because every part exists to serve that entry:

workloads.yaml startup pre-population
  → Analyzer / DeploymentPlanCompiler
  → PlanStore / WorkloadStore
  → Replanner (expiry + SLA-violation + backend-repost tickers)
  → Scraper (SLA violation detection)
  → OpAMP on-connect / on-disconnect hooks
  → colored_dag stage allocation + three-stage split
  → per-runtime YAML/TOML emitters

Splitting it would leave intermediate states that do not compile.

Deleted

emit/stage_config.rs (7,410), physical/colored_dag/ (2,572), replan.rs (1,350), emit/backend_push.rs (1,043), physical/workload_planner.rs (980), pipeline.rs (843), emit/{agent,otap,telegraf}.rs (1,857), registered_workload.rs (569), store/, monitor/, physical/{plan_cache,stage_split,topology}.rs, deployment_cost/{delta,sketch_capability}.rs plus its scoring layer, and the tests covering them.

registered_workload.rs is new since the first attempt — #714 introduced it as the unified planner workload, but it only ever served the legacy chain, so it retires with it.

Also gone: the USE_TYPED_STAGE_SPLIT kill switch (#703 item 12 — its "off" branch was the legacy emitter), RealizationProvider::stages (item 10, no callers), physical/topology.rs (item 11, a pure re-export).

Extracted, not deleted

  • physical::backend_stage (87 lines) keeps BackendStageConfig, BackendAggregation, BackendReadout, AggregationInput. PhysicalPlanCompiler builds these; backend_plan::from_stage_config consumes them. Their collector-side neighbours (EdgeStageConfig, GatewayStageConfig, ExportTarget, StageId) do not come along.
  • emit::backend_wire (390 lines) keeps the storage-routing classifier and build_backend_aggregation_json. That builder stays the single source of the aggregation wire shape, so BackendPlan materializations and the JSON format keep one PolicyFingerprint identity space — the invariant from_stage_config's module doc calls load-bearing.

deployment_cost keeps only what still has consumers: online (/api/v1/cost-model), tco (/api/v1/tco), wire (post-ASAP cost model). SketchCosts and the benchmark table move into online, its only remaining caller.

Behaviour changes

  • /metrics no longer exposes asap_active_plan_id. The gauge read PlanStore, which was legacy plan history. Every asap_runtime_* gauge is unchanged.
  • GET /api/v1/collector-config/agent is gone. The OpAMP CollectorPlan channel stays — ASAPCollector's asapedgeprocessor already implements ReceiveCollectorPlan — but a deployment pointing a collector at --config=http://control_plane:8080/api/v1/collector-config/agent has to move to that channel.

The e2e test

data_plane/tests/e2e_controller_plans_and_backend_serves.rs built its config through the deleted chain. Its scaffolding is ported here — plan_materializations runs BackendLocalPlanningInput::into_physical_compilation_requestPhysicalPlanCompiler::compile_promql, and physical_fixture::artifact_from_materializations takes planner output directly, so the legacy StreamingConfig document is no longer an intermediate step.

The 12 tests are #[ignore]d, tracked by #723. Making them pass is a different problem from making them compile: they build their OTLP payloads from summary families the legacy path was told to use via sketch_type_override, which ASAPPlanner has no equivalent for. Measured, not assumed:

test old override planner's choice
..._roundtrip_kll KLL DDSketch
..._roundtrip_count_sketch CountSketch CountMinSketchWithHeap
..._roundtrip_count_min_sketch CountMinSketch CountMinSketchWithHeap

#723 carries the five remaining failure classes and their root causes. This is the work #703 item 2 already scoped separately.

Resulting surface

Production routes are the ASAPPlanner path only:

/api/v1/physical-plan/cost-manifests
/api/v1/metricsql/physical-plan/cost-manifests
/api/v1/physical-plan/compile-and-publish
/api/v1/metricsql/physical-plan/compile-and-publish
/api/v1/clickhouse-plan/compile-and-publish
/api/v1/cost-model
/api/v1/tco
/metrics

AppState is down to opamp, online_store, runtime_samples, active_summary_catalog, backend_client. OpampServer is constructed bare and retains only publish_collector_plans, which the physical path uses.

Testing

  • cargo +1.98.0 fmt -- --check — clean.
  • cargo +1.98.0 clippy --workspace --all-targets -- -D warnings — clean.
  • cargo +1.98.0 test --workspace -- --test-threads=1 — exit 0, zero failures.

🤖 Generated with Claude Code

@zzylol
zzylol merged commit 84109cb into main Sep 14, 2026
1 check passed
zzylol added a commit that referenced this pull request Sep 14, 2026
…#726)

The 12 tests were ignored by #725 because their payload construction
assumed the summary families the legacy path was told to use via
`sketch_type_override`. ASAPPlanner has no such override — it picks from
the accuracy target — so the tests now read back what it committed to.

Three things had to line up.

**Grouping.** The population key the backend materializes under has to
match the attributes the producer puts on the wire. The queries carried
no grouping, so the planner derived an empty `grouping_labels` while the
frames carried `service`/`zone`, and ingest rejected them with `active
schema resolves 0` or `frame has no matching transmission rule`. The
queries are now registered as `sum by (service) (...)`.

**Registration is not readout.** Wrapping the *query* in the same
aggregation returns no result: the stored summaries are already
per-service, so the inner form reads them and the result carries the
label. Registration decides how the materialization is keyed; readout
uses the original shape.

**Families follow the planner.** Measured, not assumed:

| query | old override | planner's choice |
|---|---|---|
| `quantile_over_time(0.5, request_size_bytes[1s])` | KLL | DDSketch (`alpha`) |
| `count_over_time(unique_users_per_min[1s])` | HLL | CountMinSketch (`w`, `d`) |
| `topk(3, count_over_time(top_endpoint_qps[1s]))` | CountSketch | CountMinSketch |

Payloads are built from `materializations[0].parameters`, so a future
planner change moves the fixture with it instead of breaking it.

Two harness fixes: the first two tests moved to `start_full_stack`
because `start_backend_http_server` never attached the physical-plan
handles, and the first test's window went from 60s to 1s to agree with
the pane layout the install pins.

The KLL and HLL builders, the plain `start_backend_http_server` harness
and the legacy streaming-config snapshot accessor go with them — every
test now installs through the same physical-plan path.

Closes #723.

Co-authored-by: Claude Opus 5 (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