feat(controller-client): DcControllerClient adapter for DC /api/v1/plan - #15
Merged
Merged
Conversation
Adds `DcControllerClient` next to `HttpControllerClient` in
`drivers/query/controller_client.rs`. It implements the same
`ControllerClient` trait but translates each capability-miss
notification into a DataCollector `QuerySpec` and POSTs to DC's
`/api/v1/plan` endpoint. Gated by a new `--controller-protocol=
{generic,datacollector}` CLI flag (defaulting to `generic` so existing
deployments are unchanged).
Translation rules:
- QueryRequirements.metric -> QuerySpec.metric_name
- QueryRequirements.grouping_labels -> QuerySpec.group_by_labels
- QueryRequirements.data_range_ms -> QuerySpec.time_window ("{N}ms"),
falls back to "60s" when None
- QueryRequirements.statistics -> QuerySpec.aggregations:
Quantile -> "quantile"
Cardinality -> "cardinality"
Count|Sum|... -> "frequency"
- accuracy_sla -> 0.95 (configurable)
Tests: five new unit tests (field projection, instant-query fallback,
quantile/cardinality mapping, round-trip against an axum mock server
asserting every field is present, and non-success HTTP status
handling). Plus one live validation on a real DC controller at
:18080, which is the whole point of Stage B:
curl /api/v1/query?query=sum(nonexistent_metric)
→ backend SimpleEngine capability miss
→ DcControllerClient translates and POSTs
→ DC controller handle_plan runs cost model
→ GET /api/v1/plan/nonexistent_metric returns a plan with
sketch_type=countsketch
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
DcControllerClientnext to the existingHttpControllerClient. It implements the sameControllerClienttrait but translates each capability-miss notification into a DataCollectorQuerySpecand POSTs to DC's/api/v1/planendpoint. Gated by a new--controller-protocol={generic,datacollector}CLI flag (defaulting togeneric, so existing deployments are unchanged).Motivation
PR G (#11) wired the backend to notify "the controller" on capability misses via the
ControllerClienttrait. The concrete impl shipped wasHttpControllerClient, which POSTs a backend-nativeCapabilityMissPayloadJSON to whatever URL is given via--controller-endpoint. That payload looks like:{ "kind": "capability_miss", "metric": "http_requests_total", "statistics": ["Sum"], "data_range_ms": 60000, "grouping_labels": ["service"], "spatial_filter_normalized": "status=\"200\"" }However, the DataCollector
controller/binary's plan endpoint (POST /api/v1/plan) expects a completely different shape — aQuerySpec:{ "metric_name": "http_requests_total", "aggregations": ["frequency"], "group_by_labels": ["service"], "time_window": "60000ms", "accuracy_sla": 0.95 }Result: if you point
--controller-endpointat DC's/api/v1/plan, every notification gets a 422 because DC can't deserializeCapabilityMissPayloadasQuerySpec. The field names, nesting, and semantics are all different.This PR introduces
DcControllerClient— a translation adapter that convertsQueryRequirements→ DCQuerySpecon the wire. It's selected via a new--controller-protocolCLI flag:--controller-protocolvaluesgeneric(default)CapabilityMissPayloadJSON directly. No translation.POST /api/v1/capability-miss), or any third-party controller that speaks the backend's format. Today nothing actually consumes this format — it exists to preserve the original PR G behavior as the default so existing--controller-endpointusers aren't broken.datacollectorQueryRequirements→ DCQuerySpecand POSTs to/api/v1/plan.In practice: if you pass
--controller-endpoint=http://controller:8080/api/v1/planwithout--controller-protocol=datacollector, the backend will POST the wrong JSON shape and get 422s from DC.Translation rules
QueryRequirementsfieldQuerySpecfieldmetricmetric_namegrouping_labels.labelsgroup_by_labelsdata_range_mstime_window"{N}ms"(e.g.60000→"60000ms"); defaults to"60s"whenNone(instant queries with no range)statistics: [Quantile]aggregations: ["quantile"]statistics: [Cardinality]aggregations: ["cardinality"]statistics: [Count|Sum|Increase|Rate|Min|Max|Topk]aggregations: ["frequency"]accuracy_sla: 0.95DcControllerConfig; DC rejects values outside [0,1]The
accuracy_slaand default time window are both onDcControllerConfig, so they can be tuned later without another CLI flag.What happens after the POST
When
DcControllerClientPOSTs to/api/v1/plan, the DC controller:QuerySpecvia itsAnalyzerGET /api/v1/plan/:metric)The backend's fire-and-forget call completes as soon as the POST returns 200. The query that triggered the miss has already returned (via the §5.2 fallback). Future queries benefit once the plan propagates.
Tests
Six new unit tests in
drivers/query/controller_client.rs:dc_build_query_spec_projects_statistics_to_aggregations— happy-path field projectiondc_build_query_spec_defaults_time_window_for_instant_query— instant-query fallbackdc_build_query_spec_maps_quantile_statistic— quantile → "quantile"dc_build_query_spec_maps_cardinality_statistic— cardinality → "cardinality"dc_client_round_trips_against_mock_plan_endpoint— full POST to an axum mock server asserting everyQuerySpecfield is present and correctly shapeddc_client_reports_non_success_status— clean error propagation on 4xx/5xxPlus live validation against a real DC
controller/binary at:18080(Stage B's actual validation goal):Test plan
cargo test -p query_engine_rust --lib controller_client→ 11 passedcargo fmt/cargo clippy -p query_engine_rust -- -D warningscleanprotocol=genericremains the default, existing--controller-endpointusers are not affectedDepends on
Stacks cleanly on #14 (the CountMinState envelope decoder fix). Can merge in either order; they touch different files.
🤖 Generated with Claude Code