fix(http): consult per-metric BackendStorageRouting on every query (issue #46 ⑤) - #89
Merged
Merged
Conversation
The Phase-5/6 EngineRouter wiring (PR #87) sourced the per-metric StorageBackend axis from `StreamingConfig::storage_backend()` — a single field that applies to the entire streaming config. In production deploys the YAML loader (`StreamingConfig::from_yaml_data`) constructs via `Self::new(...)` and always defaults `storage_backend` to `SketchWarmTier`, so the HTTP handler always took the `SimpleEngine`-direct-dispatch branch and the EngineRouter was effectively bypassed for every query — `data_source: gorilla_archive` never landed on cold-archive responses (issue #46 v2 criterion 5 PARTIAL). Per the design correction: the streaming engine never sees Gorilla data on its OTLP-ingest path (the agent's `gorillas3processor` writes chunks directly to S3), so there is nothing for `StreamingConfig::from_yaml_data` to learn. The fix lives in a separate per-metric routing layer: - New `BackendStorageRouting` data type (`{metric_name: StorageBackend}` map) loaded once at startup from `--backend-storage-routing` YAML (or its `ASAP_BACKEND_STORAGE_ROUTING` env-var alias). Wired on both the legacy `query_engine_rust` binary and the deployed `precompute_engine` binary so the Docker image picks it up. - HTTP handler's `process_query_request` extracts the metric name from the PromQL AST (via `promql_parser`) and consults the routing table; falls back to the streaming-config single axis only when no routing table is wired (preserves pre-Phase-5 behaviour). - Three new unit tests exercise the **production code path** (routing table loaded, streaming-config default unchanged) — distinct from the existing tests that mock the dispatch by pinning `streaming_cfg.storage_backend` directly. Co-Authored-By: Claude Opus 4.7 (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
Closes the issue-46 v2 criterion ⑤ partial-pass:
data_source: gorilla_archivenever landed on cold-archive responses because the HTTP query handler bypassedEngineRouterfor every query.Root cause (per the v2 root-cause analysis on issue #46): the Phase-5 dispatch read the per-metric
StorageBackendfromStreamingConfig::storage_backend()— a single field. The deployedprecompute_enginebinary loads the streaming config viaStreamingConfig::from_yaml_file → from_yaml_data, which constructs viaSelf::new(...)and always defaults the axis toSketchWarmTier. So theEngineRouterwas effectively never consulted in production.Per the user's architecture correction: the streaming engine never sees Gorilla data on its OTLP-ingest path (the agent's
gorillas3processorwrites chunks directly to S3), so there is nothing forStreamingConfig::from_yaml_datato learn. The fix lives in a separate per-metric routing layer.What changed
BackendStorageRoutingdata type — a{metric_name: StorageBackend}map loaded from YAML at startup. Schema:--backend-storage-routingCLI flag (aliasASAP_BACKEND_STORAGE_ROUTINGenv var) on both theprecompute_enginebinary (used by the deployed Docker image) and the legacyquery_engine_rustbinary.HttpServer::with_backend_storage_routing(...)wires the routing table intoAppState.process_query_requestnow consults the routing table on every PromQL query: parses the query, extracts the first metric name from the AST viapromql_parser, looks up the storage backend, and dispatches throughEngineRouterfor any per-metric override. Falls back to the streaming-config single axis when no routing table is wired (preserves pre-Phase-5 behaviour).Test plan
BackendStorageRoutingunit tests (empty router, YAML round-trip, defaults).http.rs::tests:http_production_path_routes_archive_metric_via_routing_table— streaming-config stays default (SketchWarmTier); routing table flipshttp_requests_total→gorilla_s3_archive; the response carriesdata_source: gorilla_archiveand the mock GorillaQueryEngine is hit exactly once.http_production_path_unlisted_metric_falls_back_to_warm_tier— non-listed metric still hitsSimpleEnginedirect path.http_production_path_default_axis_routes_all_metrics—default: gorilla_s3_archive(no per-metric overrides) routes everything.http_routes_archive_metric_to_gorilla_engine, etc.).cargo test -p query_engine_rust --lib drivers::query::servers::http: 25 tests pass.cargo build -p query_engine_rust --bins: clean.🤖 Generated with Claude Code