chore(ddsketch): consume DDSketch wire format without metric scalars - #345
Merged
Merged
Conversation
The DDSketch wire format dropped its DataPoint-level METRIC scalars
(count/sum/min/max) in ProjectASAP/sketchlib-go#243 / asap_sketchlib#57.
Update the backend to consume the trimmed format:
- proto: DDSketchDelta keeps only `buckets` (1); tags 2-7 reserved.
- DDSketchAccumulator:
- from_sketchlib_proto_bytes / sketch_reducer / delta_apply now call
DdSketch::from_raw(alpha, store_counts, store_offset) (3-arg) and
derive count from the bucket store via total_count().
- apply_proto_delta_bytes applies bucket deltas only; count is
recomputed from the merged buckets.
- serialize_to_json / edge_runtime_adapter drop the removed scalars.
- query_statistic STRICT policy:
- Quantile -> sketch-derived (unchanged).
- Count -> derived from buckets (sum of store counts).
- Sum / Min / Max -> return the unavailable-statistic error; these
move to controller-provisioned exact Sum / MinMax aggregations.
Default aux_stats() is empty for DDSketch so the query path falls
through to query_statistic and propagates the error gracefully.
- tests: assert Quantile + Count still work and that Sum/Min/Max return
the unavailable-statistic error (not a panic / 0); fixtures rebuilt
from buckets only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 26, 2026
zzylol
added a commit
that referenced
this pull request
May 26, 2026
Align the backend's delta-apply with asap_sketchlib's updated wire format:
- HLL delta is now a varint-packed (index_delta, value) blob (was repeated
per-register sub-messages). Decode + apply via HllSketch::apply_delta_bytes
(single source of truth) instead of the vendored nested decode; vendored
hll_delta.proto updated to bytes packed_updates; HLL round-trip tests build
the packed blob.
- CountMinSketchDelta gained an hh_keys field upstream; pass an empty set on
the CountMin apply path (no TopK to rebuild here), mirroring CountSketch.
- Update the CMS/CountSketch zero-dims decode tests to assert the current
validate_sketch_dims message ('degenerate dims') surfaced by the bump.
DDSketch alignment (scalar-less wire) is already on main (#345/#346).
cargo build --workspace clean; cargo test -p data_plane --lib: 782 passed.
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.
What
The DDSketch wire format dropped its DataPoint-level METRIC scalars (
count/sum/min/max) in ProjectASAP/sketchlib-go#243 / asap_sketchlib#57. This updatesASAPQuery-backendto consume the trimmed format.Changes
crates/asap_otel_proto/proto/sketchlib_delta/ddsketch_delta.proto):DDSketchDeltakeeps onlybuckets(1); tags 2-7 (d_count,d_sum,new_min,new_max,min_changed,max_changed) are nowreserved.build.rsregenerates the prost bindings.DDSketchAccumulator(dd_sketch_accumulator.rs):from_sketchlib_proto_bytescalls the new 3-argDdSketch::from_raw(alpha, store_counts, store_offset);countis derived from the bucket store viatotal_count().apply_proto_delta_bytesapplies bucket deltas only;countis recomputed from the merged buckets.serialize_to_jsondrops the removed scalars (keeps bucket-derivedcount).sketch_reducer.rs/delta_apply.rsand theedge_runtime_adapter.rsenvelope encoder updated for the 3-fieldDdSketchStateandtotal_count().query_statistic— STRICT policy (as implemented)Statistic::Quantile(q)→ sketch-derived (unchanged).Statistic::Count→ derived from buckets (sum of store counts, exact).Statistic::Sum/Statistic::Min/Statistic::Max→ return the unavailable-statistic error (aBox<dyn Error>string error, not a panic, not 0). These statistics move to the controller-provisioned exactSum/MinMaxaggregations.The query path handles this gracefully:
query_statisticreturns aResult, andDDSketchAccumulatoruses the default emptyaux_stats(), so Sum/Min/Max are not short-circuited by aux and the error propagates to the caller (which already treats it as a fallible result).Tests
query_statisticpolicy tests: Quantile + Count succeed; Sum/Min/Max return the unavailable-statistic error.Dependency
Depends on ProjectASAP/asap_sketchlib#57 — CI green once that merges and the
asap_sketchlibdep picks up the removed fields. Verified locally by temporarily repointing theasap_sketchlibpath to thechore/ddsketch-drop-metric-scalarsworktree (reverted before commit; the committedCargo.tomlkeeps the normal../asap_sketchlibpath).Test results (local, against the trimmed asap_sketchlib)
cargo build(lib + benches + examples + tests): green.edge_runtime_consumes_precompute_rs,e2e_controller_plans_and_backend_serves,e2e_modified_otlp_sketch_path): green.*_zero_dims_rejectedlib tests fail against the new asap_sketchlib branch because that branch also changed CMS error text — out of scope for this DDSketch PR; not touched here.🤖 Generated with Claude Code