feat: vendor DataCollector's modified OTLP proto (PR A, Phase 1) - #5
Merged
Merged
Conversation
Adds a new workspace crate `asap_otel_proto` that ships Rust bindings
for DataCollector's modified opentelemetry-proto schema, and switches
asap-query-engine over to it. This is the first piece of the
end-to-end sketch path roadmap (Phase 1, PR A in
docs/pipeline-query-catalog.md §10 in DataCollector#153).
What's vendored
---------------
The four .proto files copied verbatim from
DataCollector/opentelemetry-proto/opentelemetry/proto/:
- common/v1/common.proto
- resource/v1/resource.proto
- metrics/v1/metrics.proto (modified upstream)
- collector/metrics/v1/metrics_service.proto (modified upstream)
The modified metrics.proto extends Metric.data with first-class sketch
variants on tags 13–17:
Metric.data oneof {
...
DDSketch ddsketch = 13;
KLLSketch kllsketch = 14;
CountSketch countsketch = 15;
CountMinSketch countminsketch = 16;
HLLSketch hllsketch = 17;
}
Each variant has a typed `*SketchDataPoint` message with `attributes`
(labels), `start_time_unix_nano` / `time_unix_nano`, per-window
`count` / `sum` / `min` / `max`, sketch bytes, and a per-type
`encoding` enum with `*_ENCODING_PROTO` and `*_ENCODING_PROTO_DELTA`
variants. Every existing data point also gains a `series_id` field
with the invariant "exactly one of (series_id != 0, attributes
populated) is true". The collector metrics service response gains a
`series_assignments` field for distributing series_id assignments
back to the agent.
Crate layout
------------
asap-common/dependencies/rs/asap_otel_proto/
Cargo.toml # path-dep'd from asap-query-engine
build.rs # tonic-build, protoc from protoc-bin-vendored
src/lib.rs # re-exports as asap_otel_proto::tonic::*
proto/opentelemetry/proto/{common,resource,metrics}/v1/*.proto
proto/opentelemetry/proto/collector/metrics/v1/metrics_service.proto
The module structure under `asap_otel_proto::tonic::*` mirrors
`opentelemetry_proto::tonic::*` so call sites only need to swap the
crate name, not the inner module path.
Build dependencies
------------------
- prost-build = "0.13"
- tonic-build = "0.12"
- protoc-bin-vendored = "3"
The vendored protoc is required because the host's system protoc on
many distros is < 3.15 (the minimum that accepts proto3 `optional`
keyword used in opentelemetry-proto). build.rs sets `PROTOC` to the
vendored binary so the build is self-contained on any platform
protoc-bin-vendored supports (linux x86_64 / aarch64 / x86_32 /
ppcle_64 / s390_64, macos x86_64 / aarch_64, win32).
asap-query-engine adoption
--------------------------
- Cargo.toml: dropped `opentelemetry-proto = "0.28"` from crates.io,
added `asap_otel_proto = { path = "../asap-common/dependencies/rs/asap_otel_proto" }`.
- drivers/ingest/otel.rs: replaced every `opentelemetry_proto::` import
with `asap_otel_proto::` (11 sites). Module paths under
`tonic::collector::metrics::v1::*`, `tonic::common::v1::*`,
`tonic::metrics::v1::*` are unchanged because the new crate's
`lib.rs` mirrors the upstream layout.
- ExportMetricsServiceResponse construction: the response message
gained a `series_assignments` field in the modified proto. Set to
`Vec::new()` for now — PR B / G will populate it when the backend
starts minting series_id descriptors.
- otlp_to_record_count: added match arms for the five new sketch
variants so the data-point counter sees them.
- otlp_to_metric_points_and_sketches: added stub match arms for the
five new sketch variants that emit a `debug!` log indicating the
decoder is PR B's work. Sketches are not silently dropped —
they're acknowledged and traced. PR B will replace these stubs
with real per-variant decoders that build the matching concrete
accumulator and route via WorkerMessage::AccumulatorInput.
Validation
----------
- cargo build -p asap_otel_proto: clean. Verified the generated
output at target/debug/build/asap_otel_proto-*/out contains
Data::Ddsketch / Data::Kllsketch / Data::Countsketch /
Data::Countminsketch / Data::Hllsketch enum variants and
per-sketch encoding enums (DdsketchEncodingProto /
DdsketchEncodingProtoDelta etc.).
- cargo check -p query_engine_rust --all-targets: clean.
- cargo clippy -p query_engine_rust --all-targets -- -D warnings: clean.
- cargo fmt --check: clean.
- cargo test -p query_engine_rust --lib: 435 passed, 0 failed,
5 ignored. No regressions from the dep swap.
What's next
-----------
PR B (task #7) implements the per-variant handlers in otel.rs,
replacing the debug-log stubs with real decoders that read the
typed proto fields (attributes, time_unix_nano, count/sum/min/max,
sketch bytes, encoding) and route via WorkerMessage::AccumulatorInput.
PR C (task #8) audits and fills any missing concrete accumulator
types in precompute_operators/. PR D (task #9) is the e2e
integration test that anchors correctness for Phase 1.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Apr 14, 2026
Adds the first real end-to-end correctness test for the modified-OTLP sketch path landed in PR A (#5, vendoring) + PR B (#6, routing + CountMin decoder). The test exercises every public surface in the Phase 1 hot path: 1. Build a `StreamingConfig` with a single `AggregationConfig` for `http_requests_total` with `aggregation_type=CountMinSketch`, `grouping_labels=[service]`, 1-second tumbling windows. 2. Spawn a real `PrecomputeEngine` (workers + router + ingest state). 3. Spawn a real `OtlpReceiver::with_ingest_state(...)` wired to the same engine — modeling the production wiring `main.rs` does. 4. POST a real protobuf-encoded `ExportMetricsServiceRequest` over OTLP HTTP at `/v1/metrics`. The request carries a single `Metric.data = CountMinSketch{ data_points: [ CountMinSketchDataPoint { sketch: <CountMinState bytes>, encoding: COUNT_MIN_SKETCH_ENCODING_PROTO, attributes: [{service: "auth"}], time_unix_nano: 100ms } ] }` payload with a known 2x4 matrix (counts: row 0 = [1,2,3,4], row 1 = [5,6,7,8]). 5. POST a second OTLP request timestamped 2 s past epoch (past the 1 s window end) so the precompute engine's watermark advances and closes window 0. 6. Wait for the periodic flush (100ms interval) to fire. 7. Drain the `CapturingOutputSink`, find the entry for window 0, downcast the `Box<dyn AggregateCore>` to `CountMinSketchAccumulator`, read `inner.sketch()`, and assert each row matches the expected `Vec<f64>`. What this validates end-to-end: - PR A: the vendored `asap_otel_proto` crate's tonic bindings expose `Metric.data::Countminsketch` and the typed `CountMinSketchDataPoint` fields, and the prost build pipeline produces decoders that the test crate can use directly to construct a request. - PR B routing: `route_modified_otlp_sketches_to_precompute` walks the `Metric.data` oneof, flattens each per-variant data point into `ModifiedOtlpSketchDp`, matches the metric against `ingest_state.agg_configs` by name, computes the group key from the `service` attribute via `IngestState::extract_group_key_for`, and emits `WorkerMessage::AccumulatorInput` correctly. - PR B decoder: `CountMinSketchAccumulator::from_sketchlib_proto_bytes` decodes the `CountMinState` proto, picks the int64 counter path, reshapes the flat counts into `Vec<Vec<f64>>`, and constructs the underlying `CountMinSketch` via `from_legacy_matrix` — round-trip matches the original matrix exactly. - Precompute engine sketch-pane merge: the `WorkerMessage::AccumulatorInput` reaches the right worker via the (agg_id, group_key) hash, lands in the `sketch_panes` of the matching `GroupState`, and survives the watermark-driven window close to be emitted via the `OutputSink`. - Window close + sink: after the watermark advances past the 1 s window end, the worker's flush emits a `(PrecomputedOutput, Box<dyn AggregateCore>)` tuple with `start_timestamp=0` and `end_timestamp=1000` to the `CapturingOutputSink`. - Round-trip correctness: the matrix that comes out of the sink is bit-identical to what went in over OTLP, proving there is no data loss in the routing/decoder/merge path for a single-input window. Test layout ----------- - New file `asap-query-engine/tests/e2e_modified_otlp_sketch_path.rs` - Helpers: `make_count_min_agg_config(...)` — builds a tumbling-window CountMinSketch `AggregationConfig` with a `service` group `engine_config(...)` — builds a `PrecomputeEngineConfig` with a 100ms flush interval so the test does not have to wait long `build_count_min_state(...)` — builds a `CountMinState` proto from a known matrix `build_export_request(...)` — wraps a `CountMinSketchDataPoint` in a fully-formed `ExportMetricsServiceRequest` `post_otlp_http(...)` — sends a protobuf body to the OTLP HTTP endpoint at `localhost:port/v1/metrics` and asserts 2xx Validation ---------- - `cargo check -p query_engine_rust --tests`: clean - `cargo clippy -p query_engine_rust --tests -- -D warnings`: clean - `cargo fmt --check`: clean - `cargo test -p query_engine_rust --test e2e_modified_otlp_sketch_path`: `test e2e_count_min_sketch_modified_otlp_path ... ok` `test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured` - `cargo test -p query_engine_rust --lib`: 439 passed, 0 failed, 5 ignored What's still PR C territory --------------------------- - KLL / DDSketch / CountSketch / HLL e2e coverage. PR C (task #8) is re-scoped as smaller per-sketch-type follow-ups (PR C-CountSketch, PR C-HLL, PR C-KLL, PR C-DDSketch); each adds a per-type decoder and an analogous test arm to this file. - Delta transmission (`*_ENCODING_PROTO_DELTA`). PR C-delta adds per-series baseline tracking and a delta-merge codepath; this test file gets a `delta` test case once that lands. - MessagePack encoding parity (`*_ENCODING_MSGPACK` variants). PR I (task #14) adds those; this test file gets a four-way `(format, mode)` correctness assertion at that point. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 14, 2026
4 tasks
zzylol
added a commit
that referenced
this pull request
May 5, 2026
…ill) (#77) Rewrites docs/proofs.md to a per-proof structure (Statement / Setup- Lemmas / Proof / Caveats / Code anchors) for paper-blocker #5: - §2 combine_statistic correctness across schema-timeline segments: additive stats sum-bound, idempotent stats max-bound, non-combinable stats return Partial. - §3 Write-barrier safety: post-force_expire samples never reach any query, by chasing the sample through route_decoded_samples -> is_writable -> dropped. - §4 Backfill determinism: under the three §10.5 invariants enforced by BackfillRegistry::create_checked, build_backfilled_accumulator produces bit-identical bytes to a live SumAccumulator-style path (induction on sample-stream length). Each proof cites the runtime functions it relies on by symbol name (line-number-stable). Marks TODO.md item 5 done with a pointer to proofs.md; no source files touched. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
zzylol
added a commit
that referenced
this pull request
May 5, 2026
Under concurrent producer write + reader scan (the e2e harness shape), the cold-fallback reader saw a part-file whose final line was a partial JSON record and errored the whole scan, manifesting as 10 s cold-fallback timeouts on `sum(http_requests_total)` in E0 (PR #261, 34/300 replay queries). Tolerate the torn last line specifically: if a parse error hits the *final* line *and* the blob has no terminating newline (the producer-mid-flush shape per design-sketch-db.md §5.2), warn + drop instead of failing the part. Mid-file parse errors still hard-error — that signals real corruption, not a concurrent write. Threads `Option<&Path>` through a new `parse_jsonl_at` helper so the warn log includes the offending file + a 200-char line preview; existing `parse_jsonl(bytes, start, end)` callers (`local_fs.rs`) unchanged. Pins both shapes in tests: - parse_jsonl_ignores_torn_trailing_line - parse_jsonl_errors_on_mid_file_corruption Unblocks PROGRESS.md "Outstanding follow-ups #5" and the E0 exit-criterion (1) cold-routed query family. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 task
zzylol
added a commit
that referenced
this pull request
May 13, 2026
Post-M2.3 reorg #5 of 8. Creates a new `sketch_db::lifecycle` module and moves `SchemaEvictionService` (and friends) into `lifecycle/eviction.rs`. The sid-level lifecycle FIELDS and methods on `SketchInstanceMetadata` / `SketchStore` stay in `index/` next to the data they gate — only the schedule-driven *service* moves here. `schema/mod.rs` re-exports the eviction types under their legacy path (`sketch_db::schema::SchemaEvictionService`) so existing consumers compile unchanged. Canonical home is now `sketch_db::lifecycle::*`. This is the structural skeleton for the upcoming sub-PRs: - #6 will add `lifecycle::reconcile_from_streaming_config`, reimplementing schema/'s reconcile semantics over the sid catalog. - #7 will then delete `schema/` once the only resident is the thin re-export. 783/783 lib tests pass. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 13, 2026
zzylol
added a commit
that referenced
this pull request
May 13, 2026
#185) Schema retirement #3. Repoint `ASAPQueryEngine::timeline_for_query` from `SchemaRegistry::timeline_for_metric` to the sid-level `storage_engines::sketch_db::query::timeline::timeline_for_metric` landed in #183. The cross-reconfigure dispatcher (`try_handle_query_promql_via_timeline`) now reads its segments from the sid catalog rather than from `SchemaRegistry`. The sid-level timeline populates `TimelineSegment.agg_id` with a content-hash of `(metric, agg_kind, group_by_keys)` rather than a `StreamingConfig.aggregation_id`. Until schema retirement #5 ports the per-segment dispatch to sid-level evaluation, the segment-→-aggregation_config lookup inside the dispatcher is best-effort: when no segment resolves to an in-config aggregation the dispatcher returns `None` so the caller falls back to the default single-agg path instead of regressing cross-reconfigure queries to empty-result-plus-warnings. The schema retirement plan keeps the `schema_registry` field on `ASAPQueryEngine` alive for now — it's still referenced by the ingest barrier and the swap-handler driver. Both go away in retirements #4 + #5. Two tests in `tests/schema_timeline_dispatch_tests.rs` are marked `#[ignore]`: they build two distinct `AggregationConfig`s with identical content (same metric / Sum / `host` grouping). In the sid catalog those collapse to one signature group → one segment, so the dispatcher can no longer reproduce the schema-boundary-stitch scenario from a SchemaRegistry-shaped fixture. The third single-schema regression test still passes unchanged. Re-enabling these is part of retirement #5 (sid-level dispatch) or a fixture rewrite that uses two genuinely distinct signatures. 787/787 lib tests pass; 5 ignored. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
zzylol
added a commit
that referenced
this pull request
May 13, 2026
Schema retirement #4. Add the sid-level mirror of `SchemaRegistry::reconcile` under `sketch_db/lifecycle/reconcile.rs`: pub fn reconcile_from_streaming_config( store: &SketchStore, config: &StreamingConfig, retention: Duration, ) -> SidReconcileSummary Iterates every sid in `store`, computes its content signature `(metric_name, agg_kind, group_by_keys)`, and force-retires any sid whose signature is not represented in the new config. Mirrors the "retire orphans" half of the schema-version reconcile; the "add new ids" half is implicit in the sid model (sids are minted lazily by the ingest path on first write). Wired alongside the existing `SchemaRegistry::reconcile` at every reconcile call site: - `route_otlp_to_precompute` in `drivers/ingest/otel.rs` (raw-OTLP ingest path) - `route_modified_otlp_sketches_to_precompute` in `drivers/ingest/otel.rs` (modified-OTLP sketch path) - `streaming-config` swap handler in `drivers/query/servers/http.rs` (event-driven entry) Both registries run in parallel for now: the §6.3 ingest barrier `ingest_state.schemas.is_writable(agg_id)` still depends on `SchemaRegistry` lifecycle state, so we keep the schema reconcile alive until retirement #5 deletes the registry and replaces the barrier with a sid-level check. Six new unit tests cover empty-config, matching-signature, different-signature, idempotency (already-retired sid not re-retired), distinct-metrics isolation, and different-agg-type discrimination. Sketch-typed agg-configs are not yet covered (the `AggregationConfig` shape doesn't carry sketch params today — parallel capability-routing channel handles that lifecycle). 793/793 lib tests pass; 5 ignored. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 13, 2026
zzylol
added a commit
that referenced
this pull request
May 17, 2026
…chema-retirement #5 step 1) (#273) The modern execute() trait path and execute_range_promql_modern resolved ASAP-tier candidates to sids via `sids_for_policy(fp)` only, on the assumption that every production sid registration would populate `policy_fp` (per the comment retired by an earlier PR). That assumption breaks for sketches arriving via OTLP: `derive_sketch_policy_fp` only returns `Some(fp)` when a streaming-config policy's `grouping_labels` EXACTLY matches the wire DP's `group_by_keys`. When the agent emits sketches with the full wire-attr set (no upstream attribute reduction — the common case when the controller's OpAMP-pushed runtime config doesn't take effect, see ASAPCollector#381), `find_policy_by_content` returns `None`, all sids land in the catalog with `policy_fp = UNSET`, and `sids_for_policy(streaming_config_fp)` returns empty even though the data is sitting in the SketchStore right there. Fix: union the `instances_matching(metric, group_by_keys)` catalog walk into the sid set. `instances_matching` is the more general primitive: it returns sids whose `group_by_keys` is a SUPERSET of the candidate's asked grouping, which subsumes the policy-fp reverse-index hit (an ExactAgg-style sid minted via `ingest_precompute_for_agg_config` has `group_by_keys == streaming_config.grouping_labels` so it satisfies the subset check) AND covers the full-attr sketch case the prior path missed. End-to-end verified via the single-node MVP smoke test (`/mydata/mvp-smoke-test/`): $ curl --data-urlencode 'query=quantile_over_time(0.99, http_requests_total_latency_ms[5m])' \ http://localhost:19091/api/v1/query {"accuracy":{"delta":0.0,"epsilon":0.01,"kind":"relative_quantile"}, "data":{"result":[{"metric":{"zone":""}, "value":[1779043268.182,"90.93548893834691"]}], "resultType":"vector"}, "infos":["accuracy: ε=0.01, δ=0, kind=relative_quantile", ...]} Pre-fix this returned `{"data":null,"error":"No result for query"}`. Smoke-test `D` axis already showed 51 sids registered for the metric; the gap was purely the query path's sid-resolution step. Test plan: * `cargo test -p data_plane --lib` 756/756 green (756 pass / 5 ignored) * `cargo test -p control_plane --lib` 691/691 green * New regression test `full_attr_sketch_sid_findable_via_subset_grouping` in the `asap_tier_classify_tests` module pins the smoke-test scenario at unit level — a sketch sid with `policy_fp = UNSET` and full wire-attr `group_by_keys` must be findable by a query whose `group_by_keys` is a subset. * End-to-end smoke test (`bash /mydata/mvp-smoke-test/run_smoke.sh`) Axis C `quantile_over_time` now returns a real DDSketch quantile; pre-fix it returned "No result for query". Scope: this is step 1 of [#272 (schema-retirement #5)](#272). The ingest bucketing (`otel.rs:561`, `WorkerMessage::AccumulatorInput`), precompute output sink, and backfill processor still key on `config.aggregation_id()`; those are separate per-subsystem retirements tracked in the same issue. The legacy `handle_query` / `execute_context` path also still routes through agg_id and the "No precomputed outputs found for metric: X, aggregation_id: Y" error message — its retirement is the next step. Closes the sid-resolution sub-step of #272. Related: issue #271 (MVP demo axis C) — sister fix path 2. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 17, 2026
…ema-retirement #5 step 2) (#274) * fix(query): union instances_matching into ASAP-tier sid resolution (schema-retirement #5 step 1) The modern execute() trait path and execute_range_promql_modern resolved ASAP-tier candidates to sids via `sids_for_policy(fp)` only, on the assumption that every production sid registration would populate `policy_fp` (per the comment retired by an earlier PR). That assumption breaks for sketches arriving via OTLP: `derive_sketch_policy_fp` only returns `Some(fp)` when a streaming-config policy's `grouping_labels` EXACTLY matches the wire DP's `group_by_keys`. When the agent emits sketches with the full wire-attr set (no upstream attribute reduction — the common case when the controller's OpAMP-pushed runtime config doesn't take effect, see ASAPCollector#381), `find_policy_by_content` returns `None`, all sids land in the catalog with `policy_fp = UNSET`, and `sids_for_policy(streaming_config_fp)` returns empty even though the data is sitting in the SketchStore right there. Fix: union the `instances_matching(metric, group_by_keys)` catalog walk into the sid set. `instances_matching` is the more general primitive: it returns sids whose `group_by_keys` is a SUPERSET of the candidate's asked grouping, which subsumes the policy-fp reverse-index hit (an ExactAgg-style sid minted via `ingest_precompute_for_agg_config` has `group_by_keys == streaming_config.grouping_labels` so it satisfies the subset check) AND covers the full-attr sketch case the prior path missed. End-to-end verified via the single-node MVP smoke test (`/mydata/mvp-smoke-test/`): $ curl --data-urlencode 'query=quantile_over_time(0.99, http_requests_total_latency_ms[5m])' \ http://localhost:19091/api/v1/query {"accuracy":{"delta":0.0,"epsilon":0.01,"kind":"relative_quantile"}, "data":{"result":[{"metric":{"zone":""}, "value":[1779043268.182,"90.93548893834691"]}], "resultType":"vector"}, "infos":["accuracy: ε=0.01, δ=0, kind=relative_quantile", ...]} Pre-fix this returned `{"data":null,"error":"No result for query"}`. Smoke-test `D` axis already showed 51 sids registered for the metric; the gap was purely the query path's sid-resolution step. Test plan: * `cargo test -p data_plane --lib` 756/756 green (756 pass / 5 ignored) * `cargo test -p control_plane --lib` 691/691 green * New regression test `full_attr_sketch_sid_findable_via_subset_grouping` in the `asap_tier_classify_tests` module pins the smoke-test scenario at unit level — a sketch sid with `policy_fp = UNSET` and full wire-attr `group_by_keys` must be findable by a query whose `group_by_keys` is a subset. * End-to-end smoke test (`bash /mydata/mvp-smoke-test/run_smoke.sh`) Axis C `quantile_over_time` now returns a real DDSketch quantile; pre-fix it returned "No result for query". Scope: this is step 1 of [#272 (schema-retirement #5)](#272). The ingest bucketing (`otel.rs:561`, `WorkerMessage::AccumulatorInput`), precompute output sink, and backfill processor still key on `config.aggregation_id()`; those are separate per-subsystem retirements tracked in the same issue. The legacy `handle_query` / `execute_context` path also still routes through agg_id and the "No precomputed outputs found for metric: X, aggregation_id: Y" error message — its retirement is the next step. Closes the sid-resolution sub-step of #272. Related: issue #271 (MVP demo axis C) — sister fix path 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(query): port resolve_sketch_metric_alias to modern execute() (schema-retirement #5 step 2) Continuing #272 retirement work. The agent-side `_quantile` / `_hll` INGEST-time metric renames (DDSketch+KLL append `_quantile`, HLL appends `_hll`) used to be handled only at the entry of the legacy `handle_query_promql` path. With #273 making modern `execute()` and `execute_range_promql_modern` the producing paths for the common sketch-backed-query case, those paths also need the rename rewrite — otherwise a user query `quantile_over_time(0.99, http_latency[5m])` that lands in modern (via the legacy→modern fallback in `process_via_simple_engine`) misses the suffixed series the ASAP tier actually holds. Port: call `self.resolve_sketch_metric_alias(query).unwrap_or_else(...)` at the top of both modern execute() trait impl and `execute_range_promql_modern`. The helper is already a pure function (parse + classify shape + replace metric token); no refactor needed, just an additional caller. Tested with the smoke test (`/mydata/mvp-smoke-test/run_smoke.sh`) — the bare-metric case still works because the smoke fake-exporter emits `http_requests_total_latency_ms` literally (no suffix), and the streaming-config metric name matches, so `resolve_sketch_metric_alias` correctly no-ops via `bare_present`. Once the agent's DDSketch processor's `metric_suffix: "_quantile"` takes effect (when ASAPCollector#381 is resolved and OpAMP-pushed runtime config applies), modern's PromQL queries will need this rename to bind back to the suffixed series — this PR pre-stages it. Why not reorder process_via_simple_engine to call modern first now: the `http_capability_miss_feedback_loop_closes_over_http` test relies on the capability-miss notify side-effect inside `find_compatible_aggregation_with_miss_notify` (which only legacy calls); reordering also surfaces a pre-existing time=0 underflow bug at engine.rs:792. Modern needs its own capability-miss notify before that flip is safe — staged as the next sub-PR of #272. Test plan: * `cargo test -p data_plane --lib` 756/756 pass * `cargo test -p control_plane --lib` 691/691 pass Related: #272 (schema-retirement #5 umbrella), #273 (step 1 — sid resolution fallback). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 17, 2026
…#275) * fix(query): union instances_matching into ASAP-tier sid resolution (schema-retirement #5 step 1) The modern execute() trait path and execute_range_promql_modern resolved ASAP-tier candidates to sids via `sids_for_policy(fp)` only, on the assumption that every production sid registration would populate `policy_fp` (per the comment retired by an earlier PR). That assumption breaks for sketches arriving via OTLP: `derive_sketch_policy_fp` only returns `Some(fp)` when a streaming-config policy's `grouping_labels` EXACTLY matches the wire DP's `group_by_keys`. When the agent emits sketches with the full wire-attr set (no upstream attribute reduction — the common case when the controller's OpAMP-pushed runtime config doesn't take effect, see ASAPCollector#381), `find_policy_by_content` returns `None`, all sids land in the catalog with `policy_fp = UNSET`, and `sids_for_policy(streaming_config_fp)` returns empty even though the data is sitting in the SketchStore right there. Fix: union the `instances_matching(metric, group_by_keys)` catalog walk into the sid set. `instances_matching` is the more general primitive: it returns sids whose `group_by_keys` is a SUPERSET of the candidate's asked grouping, which subsumes the policy-fp reverse-index hit (an ExactAgg-style sid minted via `ingest_precompute_for_agg_config` has `group_by_keys == streaming_config.grouping_labels` so it satisfies the subset check) AND covers the full-attr sketch case the prior path missed. End-to-end verified via the single-node MVP smoke test (`/mydata/mvp-smoke-test/`): $ curl --data-urlencode 'query=quantile_over_time(0.99, http_requests_total_latency_ms[5m])' \ http://localhost:19091/api/v1/query {"accuracy":{"delta":0.0,"epsilon":0.01,"kind":"relative_quantile"}, "data":{"result":[{"metric":{"zone":""}, "value":[1779043268.182,"90.93548893834691"]}], "resultType":"vector"}, "infos":["accuracy: ε=0.01, δ=0, kind=relative_quantile", ...]} Pre-fix this returned `{"data":null,"error":"No result for query"}`. Smoke-test `D` axis already showed 51 sids registered for the metric; the gap was purely the query path's sid-resolution step. Test plan: * `cargo test -p data_plane --lib` 756/756 green (756 pass / 5 ignored) * `cargo test -p control_plane --lib` 691/691 green * New regression test `full_attr_sketch_sid_findable_via_subset_grouping` in the `asap_tier_classify_tests` module pins the smoke-test scenario at unit level — a sketch sid with `policy_fp = UNSET` and full wire-attr `group_by_keys` must be findable by a query whose `group_by_keys` is a subset. * End-to-end smoke test (`bash /mydata/mvp-smoke-test/run_smoke.sh`) Axis C `quantile_over_time` now returns a real DDSketch quantile; pre-fix it returned "No result for query". Scope: this is step 1 of [#272 (schema-retirement #5)](#272). The ingest bucketing (`otel.rs:561`, `WorkerMessage::AccumulatorInput`), precompute output sink, and backfill processor still key on `config.aggregation_id()`; those are separate per-subsystem retirements tracked in the same issue. The legacy `handle_query` / `execute_context` path also still routes through agg_id and the "No precomputed outputs found for metric: X, aggregation_id: Y" error message — its retirement is the next step. Closes the sid-resolution sub-step of #272. Related: issue #271 (MVP demo axis C) — sister fix path 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(query): port resolve_sketch_metric_alias to modern execute() (schema-retirement #5 step 2) Continuing #272 retirement work. The agent-side `_quantile` / `_hll` INGEST-time metric renames (DDSketch+KLL append `_quantile`, HLL appends `_hll`) used to be handled only at the entry of the legacy `handle_query_promql` path. With #273 making modern `execute()` and `execute_range_promql_modern` the producing paths for the common sketch-backed-query case, those paths also need the rename rewrite — otherwise a user query `quantile_over_time(0.99, http_latency[5m])` that lands in modern (via the legacy→modern fallback in `process_via_simple_engine`) misses the suffixed series the ASAP tier actually holds. Port: call `self.resolve_sketch_metric_alias(query).unwrap_or_else(...)` at the top of both modern execute() trait impl and `execute_range_promql_modern`. The helper is already a pure function (parse + classify shape + replace metric token); no refactor needed, just an additional caller. Tested with the smoke test (`/mydata/mvp-smoke-test/run_smoke.sh`) — the bare-metric case still works because the smoke fake-exporter emits `http_requests_total_latency_ms` literally (no suffix), and the streaming-config metric name matches, so `resolve_sketch_metric_alias` correctly no-ops via `bare_present`. Once the agent's DDSketch processor's `metric_suffix: "_quantile"` takes effect (when ASAPCollector#381 is resolved and OpAMP-pushed runtime config applies), modern's PromQL queries will need this rename to bind back to the suffixed series — this PR pre-stages it. Why not reorder process_via_simple_engine to call modern first now: the `http_capability_miss_feedback_loop_closes_over_http` test relies on the capability-miss notify side-effect inside `find_compatible_aggregation_with_miss_notify` (which only legacy calls); reordering also surfaces a pre-existing time=0 underflow bug at engine.rs:792. Modern needs its own capability-miss notify before that flip is safe — staged as the next sub-PR of #272. Test plan: * `cargo test -p data_plane --lib` 756/756 pass * `cargo test -p control_plane --lib` 691/691 pass Related: #272 (schema-retirement #5 umbrella), #273 (step 1 — sid resolution fallback). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * revert+retire(query): delete dead resolve_sketch_metric_alias rewrite Revert PR #274 (and remove the legacy `handle_query_promql` call site plus the function itself and its test module). The `_quantile` / `_hll` metric-suffix rewrite is obsolete dead code post the 2026-05 processor refactor. Per `opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/shim_helpers.go:189`: // Refactor-2026-05: metric name is PRESERVED from the input metric. // MetricSuffix is intentionally NOT applied — the sketch type is // carried by the OTLP pdata.Metric variant tag (DDSketch), so the // downstream backend can identify the encoding without a name suffix // and PromQL queries fired against the raw input metric name resolve // directly against the stored sketch state. (See also `deploy/mvp-singlenode/configs/backend-streaming.yaml`'s header note documenting the same refactor.) The wire-side input metric name IS the served metric name; the per-processor `metric_suffix` config field is now a no-op in production paths. The backend's `resolve_sketch_metric_alias` was the query-side counterpart to that long-since-retired emit-side suffix. With the suffix never applied, the resolver's `bare_present` check always short-circuits to `None` — making it pure overhead (one PromQL parse + one streaming-config snapshot scan per query) and worse, misleading: the doc comments + the in-tree `mod sketch_alias_resolver_tests` keep an architectural story alive that the runtime no longer matches. This PR removes: * The 3 call sites of `resolve_sketch_metric_alias`: - `handle_query_promql` (line 2038 — pre-existing) - `QueryEngine::execute()` trait impl (line ~3219 — added by #274) - `execute_range_promql_modern` (line ~3720 — added by #274) * The `resolve_sketch_metric_alias` method itself (~90 LOC + ~30 LOC doc comment) * The `replace_metric_token` helper (only caller was the resolver) * The `utf8_char_len` helper (only caller was `replace_metric_token`) * The `sketch_alias_resolver_tests` module (~160 LOC, 8 tests, all asserting behavior that's been a no-op for months) Net: -381 LOC, no additions. Test plan: * `cargo test -p data_plane --lib` → 748 pass / 0 fail / 5 ignored (756 → 748 reflects the 8 deleted alias-resolver tests) * `cargo test -p control_plane --lib` → 691 pass / 0 fail Follow-ups (out of scope for this PR): * `docs/design-controller-into-backend.md` still has two `metric_suffix: "_quantile"` references that should be rewritten in a docs-only PR — the live config (`gateway-aggregate-from-raw.yaml`) no longer carries the suffix, so the doc table is stale. Apologies for the original PR #274 — I called it a "port" without checking that the function being ported is dead code. Thanks for the catch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 18, 2026
…g crate (B7.5, schema-retirement #5 step 4) (#278) After #273 (modern execute() handles sketches via instances_matching) and #277 (saturating_sub for time=0 + dangling-callers cleanup), the legacy `handle_query` / `handle_query_promql` family no longer has features the modern `QueryEngine::execute()` trait can't handle. This PR deletes the whole legacy tree and the orphan `crates/promql_utilities/src/ast_matching/` module that only the legacy path consumed. Deleted: * `ASAPQueryEngine::handle_query`, `ASAPQueryEngine::handle_query_promql`, `ASAPQueryEngine::try_handle_query_promql_via_timeline` (instant-query entry points) * `ASAPQueryEngine::handle_range_query_promql`, `build_range_query_execution_context_promql`, `execute_range_query_pipeline`, `handle_binary_expr_range_promql`, `build_arm_range_context`, `apply_range_binary_op` (range-query entry point + helpers) * `execute_context`, `execute_query_pipeline`, `execute_store_query`, `execute_and_merge_store_queries` (legacy dispatcher chain) * `build_query_execution_context_promql*`, `build_promql_execution_context_tail`, `parse_and_match_promql`, `resolve_agg_info_promql`, `agg_info_from_forced_id`, `find_compatible_aggregation_with_miss_notify`, `resolve_metric_labels`, `calculate_query_timestamps_promql`, `calculate_start_timestamp_promql`, `validate_and_align_end_timestamp`, `extract_quantile_param_promql`, `extract_topk_param`, `build_query_kwargs_promql`, `create_keys_query_params`, `create_store_query_plan`, `collect_all_results`, `merge_precomputed_outputs`, `merge_accumulators`, `collect_results_separate_keys`, `collect_results_same_aggregation`, `limit_keys_for_topk`, `validate_range_query_params`, `format_final_results`, `build_query_requirements_promql`, `query_precompute_for_statistic` (legacy helpers) * `QueryExecutionContext`, `QueryMetadata`, `QueryTimestamps`, `StoreQueryParams`, `StoreQueryPlan`, `RangeQueryParams`, `RangeQueryExecutionContext` (legacy types) * `control_plane_patterns` field, the `PromQLPatternBuilder` setup in `new_with_hot_reload`, `QueryPatternType` enum * `crates/promql_utilities/src/ast_matching/` (4 files; only consumer was the legacy path) * `crates/promql_utilities/src/query_logics/parsing.rs` helpers (`get_metric_and_spatial_filter`, `get_statistics_to_compute`, `get_spatial_aggregation_output_labels`) * Test modules tied to the deleted surface: `range_query_tests`, `sketch_query_tests`, `e2e_feedback_loop_tests`, `forced_agg_id_tests`, `hll_count_query_tests`, `kll_quantile_query_tests`, `cms_rate_capability_tests`, `analyzer_parity_tests`, `calculate_start_timestamp_promql_tests`, `aux_pushdown_tests`, the whole `capability_matching_tests.rs` file and `tests/test_utilities/comparison.rs` Updated: * `data_plane/src/drivers/query/servers/http.rs::process_via_simple_engine` now calls modern `execute()` only. The capability-miss notify side-effect that used to live in `find_compatible_aggregation_with_miss_notify` is moved to the modern path's sid-resolution error branches AND to the "no-sketch-index attached" branch (HttpServer attaches its own `SketchStore` but the `ASAPQueryEngine` builder it hands off does not `.with_sketch_index(...)` — the e2e test `http_capability_miss_feedback_loop_closes_over_http` pins exactly that wiring). * `handle_range_query` now calls modern `execute_range_promql_modern` only — no legacy fallback. * `handle_precompute_job` routes through modern `execute()`. * `tests/schema_timeline_dispatch_tests.rs::single_schema_query_falls_through_to_default_path` moved to `#[ignore]` — its premise no longer has a callsite. Modern-path coverage lives in `asap_tier_classify_tests` and `e2e_modified_otlp_sketch_path`. Test plan: * `cargo test -p data_plane --lib`: 707 pass, 0 failed, 5 ignored (down from 754 — 47 tests deleted with the legacy code they exercised). * `cargo test -p data_plane --lib capability_miss_http_e2e`: 1 pass, 1 ignored — the feedback-loop test that pins the notify side-effect still passes. * `cargo test -p control_plane --lib`: 699 pass, 0 failed. The `try_handle_query_promql_via_timeline` cross-reconfigure dispatch path is gone too. Its functionality (per-segment dispatch across schema boundaries) was scheduled for a sid-level rewrite in the schema-retirement #5 follow-up; deferred to a separate PR since no current test exercises a multi-segment reconfigure boundary (the two `#[ignore]`d tests in `tests/schema_timeline_dispatch_tests.rs` documented as needing a sid-level rewrite anyway). Closes step 4 of #272. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
zzylol
added a commit
that referenced
this pull request
May 18, 2026
… to sid (B7.6) (#284) Schema retirement #5 step 6: the precompute engine's per-bucket state used to be keyed by `(agg_id, group_key)`. The grouping label values already fold into the sid via `SeriesIdResolver`'s `(metric, attrs_fingerprint, agg_kind_canonical)` identity contract, so the tuple collapses to a single u64 — `sid` is now the bucket key throughout the ingest → router → worker path. Scope: - `WorkerMessage::GroupSamples` / `AccumulatorInput` carry `sid: u64, policy_fp: PolicyFingerprint, group_key: String` instead of `agg_id + group_key`. `policy_fp` is the source-config handle the worker uses to look up `AggregationConfig` from the hot-reload snapshot; `group_key` still travels for emit-time `KeyByLabelValues` rendering. This is a BREAKING change to `WorkerMessage`, but the enum is private to the data_plane crate. - `SeriesRouter::route_group_batch` hashes by `sid` alone (`worker_for_sid`). Same sid always lands on the same worker; bucket state stays single-owner. - `Worker::group_states: HashMap<u64, GroupState>` (was `HashMap<(u64, String), GroupState>`). `GroupState` gains `policy_fp` and `group_key` fields so `evict_orphaned_groups` can check policy liveness and the emit path can render labels without re-keying the bucket. - `process_group_samples` / `process_accumulator_input` take `(sid, policy_fp, group_key, ...)`. All in-crate call sites updated. - OTLP ingest helper `resolve_bucket_sid_for_agg_config(state, config, point_labels) → (sid, policy_fp)`: derives the bucket sid for one `(config, DP)` pair by resolving against `(metric, grouping-label- values, ExactAgg-of-config)`. Used by all three OTLP dispatch paths (raw points, opaque SketchEnvelope, modified-OTLP first-class sketches). Crucially, "attrs" for sid purposes is the GROUPING-LABEL projection of wire labels — not the full label set — so distinct `(rack, node, pod)` tuples under a `grouping_labels=[zone]` policy still roll up into one bucket per zone (the GROUP-BY semantic). - Regression test added: `drivers::ingest::otel::sid_bucketing_tests:: raw_otlp_buckets_by_sid_with_distinct_group_keys` drives `route_otlp_to_precompute` end-to-end with two `zone` values × two DPs each, asserts exactly two `GroupSamples` are emitted with distinct non-zero sids that round-trip through `SeriesIdResolver::lookup`. The test docstring documents a pre-existing `format_series_key`/`parse_labels_from_series_key` inconsistency that makes `extract_group_key_for` return "" for OTLP inputs; B7.6 bucketing is unaffected because it reads `point.labels` directly (HashMap lookup), not the joined series_key. Files touched (3): - `data_plane/src/precompute_engine/series_router.rs` — message shape + routing hash + test rename. - `data_plane/src/precompute_engine/worker.rs` — `GroupState` / `Worker.group_states` retyped, `get_or_create_group_state` / `process_group_samples` / `process_accumulator_input` / `evict_orphaned_groups` / `flush_all` reworked, 30+ test call sites updated to pass `(sid, PolicyFingerprint, group_key)`. - `data_plane/src/drivers/ingest/otel.rs` — three OTLP dispatch paths switched to sid-bucketing via new `resolve_bucket_sid_for_agg_config` helper; added `sid_bucketing_tests` mod with the regression test. Test plan: - `cargo build -p data_plane` — clean. - `cargo test -p data_plane --lib` — 713 passed / 2 ignored, no new failures vs. main. - `cargo test -p data_plane` integration suite — same 2 pre-existing failures as origin/main (`controller_plan_to_query_full_roundtrip_ ddsketch` / `_kll`); verified by re-running on origin/main HEAD. Unrelated to B7.6. NOT in scope (left for B7.7): - `output_sink.rs` already reads `output.policy_fp` (no `agg_id`); no changes needed there. - `backfill/processor.rs` still uses `(agg_id, group_key)` internally — retired by B7.7. - `AggregationConfig::aggregation_id()` accessor remains; retiring it is deferred until B7.6 + B7.7 both land (per task brief). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
zzylol
added a commit
that referenced
this pull request
May 18, 2026
…_id to sid (B7.7) (#286) Third sub-step of schema-retirement #5 (issue #272), following B7.6's ingest-side rekey. The backfill processor's per-window grouping is now keyed by `sid: u64` instead of `group_key: String`, matching B7.6's sid-bucketed worker state. The SketchStore exposes a new sid-direct write path (`ingest_precompute_with_sid`) so callers that already hold the bucket sid skip the resolver round-trip inside the mint-driven `ingest_precompute_for_agg_config` wrapper. ## Sites rekeyed - `data_plane/src/storage_engines/sketch_db/backfill/processor.rs` - `process_window` now groups raw samples into a `HashMap<u64, SidBucket>` (sid-keyed) instead of `HashMap<String, Vec<RawSample>>` (group_key-keyed). - New helper `resolve_backfill_bucket_sid` mirrors `resolve_bucket_sid_for_agg_config` from `drivers/ingest/otel.rs` so backfill and live ingest mint the SAME sid for the same `(metric, grouping-values, agg_kind)` tuple. This is the invariant that lets backfill writes land in the same store row live ingest already populated for `[created_at, ∞)`. - Per-bucket writes go through the new `ingest_precompute_with_sid` path; the mint-driven sibling is no longer called from this file. - Resolver-less fallback (legacy / registry-only test setups) keeps a stable per-`group_key` bucket id so accumulator builds still preserve sample ordering — but the write itself is skipped in that branch anyway (no resolver ⇒ no precompute write, matching pre-B7.7 behaviour). - `data_plane/src/storage_engines/sketch_db/index/mod.rs` - New `pub fn ingest_precompute_with_sid(sid, agg_cfg, output, accumulator)` takes the bucket sid directly. The existing `ingest_precompute_for_agg_config` is refactored into a thin mint-driven wrapper that delegates to the new entry point — callers that don't yet hold the sid (the live `SketchStoreSink`) keep working unchanged. - Extracted `build_attrs_fp_and_label_map` shared by both methods so the mint-driven path (B7.6) and the sid-direct path (B7.7) stay byte-identical on the values they hand to the index. ## Tests added - `process_window_buckets_by_sid_via_resolver` — drives `process_window` end-to-end with two distinct svc values × two samples each, asserts exactly two sids land in the SketchStore, both `classify()` as `Hit`, and registry provenance is one entry per window. - `backfill_sid_matches_live_ingest_sid_for_same_grouping_values` — locks the live-vs-backfill sid namespace invariant: the sid the backfill helper computes for `(cfg, "latency{svc=a,zone=z0}")` must equal what the live ingest path's `resolve_bucket_sid_for_agg_config` mirror computes for the same `(metric, grouping-values, agg_kind)` tuple via the SAME resolver. ## Not in scope (deferred follow-ups) - `output_sink.rs` production code already consumes `output.policy_fp` (PR #284's report: "no changes needed there"). Its only `aggregation_id()` site is in a test that builds a `StreamingConfig` map keyed by policy_fp.as_u64() — the legitimate policy-registry use, not a bucket key. - `worker.rs` / `series_router.rs` / `drivers/ingest/otel.rs` are B7.6's domain (already merged) — not touched. - Remaining `aggregation_id()` accessor sites are all test-side `StreamingConfig` map-key uses (the map IS keyed by policy_fp.as_u64()) — those stay until the accessor itself is retired after #4 (re-enable ignored tests). ## Test plan - `cargo build -p data_plane` — clean - `cargo test -p data_plane --lib` — 715 passed / 2 ignored, no regressions vs origin/main - `cargo test -p data_plane` integration suite — same 2 pre-existing failures `controller_plan_to_query_full_roundtrip_ddsketch` / `_kll` PR #284 confirmed are pre-existing - Both new regression tests pass Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
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.
First PR in the end-to-end sketch path roadmap. Vendors DataCollector's modified opentelemetry-proto into a new workspace crate
asap_otel_protoand switches asap-query-engine over to it. See commit body for the full design rationale, vendoring layout, validation steps, and what's next.This is PR A in the implementation plan tracked in
docs/pipeline-query-catalog.md§10 on the DataCollector side. Once it lands, PR B can replace the debug-log stubs indrivers/ingest/otel.rswith real per-variant decoders that route viaWorkerMessage::AccumulatorInput.Validation
cargo build -p asap_otel_proto: cleancargo check -p query_engine_rust --all-targets: cleancargo clippy -p query_engine_rust --all-targets -- -D warnings: cleancargo fmt --check: cleancargo test -p query_engine_rust --lib: 435 passed, 0 failed, 5 ignored🤖 Generated with Claude Code