fix(ingest): envelope-aware decoders for KLL / HLL / DDSketch / CountSketch - #26
Merged
Merged
Conversation
…Sketch PR #14 fixed `CountMinSketchAccumulator::from_sketchlib_proto_bytes` to try `SketchEnvelope::decode` first and fall back to bare `CountMinState`. DataCollector's sketchlib-go processors wrap every sketch state in a `SketchEnvelope` via `SerializePortableFO` + `proto.Marshal`, so the bare-proto decoder was producing "invalid wire type" errors and silently falling through to §5.2 for every real-world sketch data point. The same bug existed for the other four sketch accumulators. This PR applies the envelope-first-then-bare-fallback pattern verbatim to: * `CountSketchAccumulator::from_sketchlib_proto_bytes` * `DatasketchesKLLAccumulator::from_sketchlib_proto_bytes` * `HllSketchAccumulator::from_sketchlib_proto_bytes` * `DDSketchAccumulator::from_sketchlib_proto_bytes` On envelope decode success, dispatch on the oneof variant and extract the expected state. On non-matching variant, return a clear `SketchEnvelope contains non-<Name> sketch` error instead of silently producing garbage. On envelope decode failure or empty oneof, fall back to bare proto decode so unit tests (which encode states directly without the envelope) keep working. Tested end-to-end via 8 new unit tests: two per accumulator (envelope-wrapped happy path + wrong-sketch-type rejection). All 10 envelope tests pass (8 new + 2 pre-existing CMS), 578 lib tests total (up from 570); clippy + fmt clean. Before the fix, wiring any of these four processors in a real deployment would fail with a decode error on every data point. The CMS fix shipped in PR #14; this PR completes the pattern across the remaining sketch types. 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
1 participant
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
PR #14 fixed
CountMinSketchAccumulator::from_sketchlib_proto_bytesto decode theSketchEnvelopewrapper DataCollector's sketchlib-go processors emit, falling back to bare proto for direct-encoded tests. The same bug existed for the other four sketch types — wiring any of them in a real deployment would fail-silent with decode errors on every data point.This PR applies the envelope-first-then-bare-fallback pattern verbatim to:
CountSketchAccumulator::from_sketchlib_proto_bytesDatasketchesKLLAccumulator::from_sketchlib_proto_bytesHllSketchAccumulator::from_sketchlib_proto_bytesDDSketchAccumulator::from_sketchlib_proto_bytesOn envelope decode success, dispatch on the oneof variant; on non-matching variant return a clear
SketchEnvelope contains non-<Name> sketcherror; on decode failure / empty oneof, fall back to bare decode.Test plan
Impact
Unblocks real-world deployments wiring any of KLL / HLL / DDSketch / CountSketch processors from DataCollector. CMS already worked post-PR #14; this closes the pattern across the remaining sketch types.
🤖 Generated with Claude Code