Repository navigation
feat(DDSketch): NewFromState / SerializeStateProtoBytes constructors - #52
Merged
Merged
Conversation
Unblocks the DataCollector ddsketchprocessor migration from
DataDog/sketches-go to sketchlib-go. The processor needs a way to
both emit and consume raw `DDSketchState` proto bytes (no envelope
wrapper) so it can round-trip sketches through the modified-OTLP
`DDSketchDataPoint.Sketch` field, which is what ASAPQuery-backend's
`sketch_core::dd_sketch::DdSketchAccumulator::from_sketchlib_proto_bytes`
consumes.
## Why not just SerializePortable?
`SerializePortable()` wraps the `DDSketchState` in an
`envpb.SketchEnvelope` for cross-sketch dispatch. When the caller
already knows the sketch type — e.g. a `DDSketchDataPoint` on the
modified-OTLP wire is already tagged as DDSketch — the envelope
overhead is wasted and incompatible with ASAPQuery-backend's
decoder, which expects raw `DdSketchState::decode(bytes)`, not an
envelope. This PR adds the unwrapped variant.
## New API
### `(*DDSketch).SerializeStateProtoBytes() ([]byte, error)`
Returns prost-compatible bytes of the inner `DDSketchState` message
directly — derives alpha from gamma, copies the bucket slice, and
emits a flat proto payload.
### `NewFromState(state *ddpb.DDSketchState) (*DDSketch, error)`
Rebuilds a `DDSketch` from a decoded state proto. Recomputes the
`IndexMapping` from `state.Alpha`, reattaches the bucket store
using the provided `StoreCounts` / `StoreOffset`, and seeds empty
sketches with +Inf/-Inf min/max sentinels so subsequent `Add()`
calls behave like a fresh sketch.
Rejects:
* nil state
* alpha outside (0, 1)
### `NewFromStateProtoBytes(data []byte) (*DDSketch, error)`
Convenience wrapper: `proto.Unmarshal` + `NewFromState`. This is
the symmetric reverse of `SerializeStateProtoBytes` and is what
the DataCollector ddsketchprocessor will call when it receives an
incoming `DDSketchDataPoint` whose bytes were emitted by another
sketchlib-go peer.
## Tests
Five new tests in `portable_test.go`:
* `TestSerializeStateProtoBytes_RoundTrip` — emit bytes, decode
as raw `DDSketchState` (no envelope) to pin the wire shape,
then reconstruct via `NewFromStateProtoBytes` and verify count,
sum, and p50 quantile match within alpha tolerance.
* `TestNewFromState_RejectsInvalidAlpha` — 4 alpha values
outside (0, 1) each produce an error.
* `TestNewFromState_NilReturnsError` — defensive nil check.
* `TestNewFromState_EmptySketchRoundTrip` — empty sketch
round-trips, and the reconstructed sketch accepts further
`Add()` calls (verifies the min/max sentinel seeding).
* `TestNewFromState_NonEmptyBucketsRoundTrip` — non-empty
sketch round-trips, and further `Add()` calls continue to
update the bucket store (verifies the `storage.Vector1DFromVec`
reattachment is live, not a snapshot view).
All 5 pass. `gofmt -l` clean.
## Follow-up
A DataCollector PR will land right after this one that migrates
`ddsketchprocessor` from `github.com/DataDog/sketches-go` to this
package, using `NewFromStateProtoBytes` for the input decode path
and `SerializeStateProtoBytes` for the output emission path.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
to ProjectASAP/ASAPCollector
that referenced
this pull request
Apr 15, 2026
Adds an `encoding: msgpack` config option to countminsketchprocessor, countsketchprocessor, and hllprocessor so operators can opt into the cross-language MessagePack wire format as an alternative to the default sketchlib proto path. This makes real MessagePack traffic flow on the modified-OTLP data plane — ASAPQuery-backend already accepts it (PRs #9, pmetric constants in #157), sketchlib-go already emits it (PR #51), but no processor selected it until now. ## Scope Three of four sketch processors are wired in this PR: * `countminsketchprocessor` — calls `ws.cms.SerializeMsgpack()` when `encoding: msgpack`, tags the data point with `CountMinSketchEncodingMsgpack`. * `countsketchprocessor` — calls `ws.cs.SerializeMsgpack()`, tags with `CountSketchEncodingMsgpack`. * `hllprocessor` — calls `series.sketch.SerializeMsgpack()` via a new `serializeHLLSketch(sketch, enc)` helper that returns `(payload, encodingTag, err)`, tags with `HLLSketchEncodingMsgpack`. `ddsketchprocessor` is intentionally **deferred** — it uses `github.com/DataDog/sketches-go`, not sketchlib-go, so it can't call `SerializeMsgpack` directly. Enabling MSGPACK for ddsketchprocessor requires a conversion shim that walks the DataDog sketch's buckets, builds a sketchlib-go `DDSketchState` proto (the same shape sketchlib-go [PR #52](ProjectASAP/sketchlib-go#52) introduced via `NewFromStateProtoBytes`), and calls `SerializeMsgpack` on the reconstructed sketchlib-go sketch. Tracked as a separate follow-up because (a) the conversion code is ~100 lines of bucket flattening, (b) DataDog's proto doesn't carry Sum/Min/Max so msgpack emission from that source is lossy, and (c) the long-term fix is a full ddsketchprocessor migration to sketchlib-go internally, which is a much bigger refactor. ## Delta transmission stays proto-only All three processors keep delta transmission on the proto path when `delta_transmission: true` is set. Sketchlib-go has `SerializeMsgpack` for full sketch state but no matching delta wire format — tracked upstream until sketchlib-go grows an `apply_delta` API parallel to its proto one. The net effect: `encoding: msgpack` + `delta_transmission: true` emits proto deltas for sparse windows and never falls back to msgpack-full for those. ## Config shape Each processor's `Config` gains: ```yaml encoding: msgpack # "proto" (default) or "msgpack" ``` New `SketchEncoding` string type + `EncodingProto` / `EncodingMsgpack` constants per processor. `Validate()` rejects unknown values with a clear error rather than silently falling back. ## Validation Same pre-existing `go.opentelemetry.io/collector/processor/selfmonitor` module-resolution issue as the earlier typed-DP refactor PRs blocks local `go build`. gofmt is clean on all 6 modified files. All API methods used (`SerializeMsgpack` on each sketch type, `*SketchEncodingMsgpack` on the pmetric patch) are already available: * sketchlib-go `CountMinSketch.SerializeMsgpack` — PR #51 * sketchlib-go `CountSketch.SerializeMsgpack` — PR #51 * sketchlib-go `HyperLogLog.SerializeMsgpack` — PR #51 * pmetric `CountMinSketchEncodingMsgpack` — PR #157 * pmetric `CountSketchEncodingMsgpack` — PR #157 * pmetric `HLLSketchEncodingMsgpack` — PR #157 ## Follow-ups * `ddsketchprocessor` MSGPACK option via a DataDog→sketchlib-go conversion shim (or a full internal migration). Tracked. * Delta msgpack wire format (requires sketchlib-go upstream work). * Full ddsketchprocessor migration from DataDog/sketches-go to sketchlib-go DDSketch — bigger refactor, not in this PR's scope. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
to ProjectASAP/ASAPCollector
that referenced
this pull request
Apr 15, 2026
#162) Adds an `encoding: msgpack` config option to countminsketchprocessor, countsketchprocessor, and hllprocessor so operators can opt into the cross-language MessagePack wire format as an alternative to the default sketchlib proto path. This makes real MessagePack traffic flow on the modified-OTLP data plane — ASAPQuery-backend already accepts it (PRs #9, pmetric constants in #157), sketchlib-go already emits it (PR #51), but no processor selected it until now. ## Scope Three of four sketch processors are wired in this PR: * `countminsketchprocessor` — calls `ws.cms.SerializeMsgpack()` when `encoding: msgpack`, tags the data point with `CountMinSketchEncodingMsgpack`. * `countsketchprocessor` — calls `ws.cs.SerializeMsgpack()`, tags with `CountSketchEncodingMsgpack`. * `hllprocessor` — calls `series.sketch.SerializeMsgpack()` via a new `serializeHLLSketch(sketch, enc)` helper that returns `(payload, encodingTag, err)`, tags with `HLLSketchEncodingMsgpack`. `ddsketchprocessor` is intentionally **deferred** — it uses `github.com/DataDog/sketches-go`, not sketchlib-go, so it can't call `SerializeMsgpack` directly. Enabling MSGPACK for ddsketchprocessor requires a conversion shim that walks the DataDog sketch's buckets, builds a sketchlib-go `DDSketchState` proto (the same shape sketchlib-go [PR #52](ProjectASAP/sketchlib-go#52) introduced via `NewFromStateProtoBytes`), and calls `SerializeMsgpack` on the reconstructed sketchlib-go sketch. Tracked as a separate follow-up because (a) the conversion code is ~100 lines of bucket flattening, (b) DataDog's proto doesn't carry Sum/Min/Max so msgpack emission from that source is lossy, and (c) the long-term fix is a full ddsketchprocessor migration to sketchlib-go internally, which is a much bigger refactor. ## Delta transmission stays proto-only All three processors keep delta transmission on the proto path when `delta_transmission: true` is set. Sketchlib-go has `SerializeMsgpack` for full sketch state but no matching delta wire format — tracked upstream until sketchlib-go grows an `apply_delta` API parallel to its proto one. The net effect: `encoding: msgpack` + `delta_transmission: true` emits proto deltas for sparse windows and never falls back to msgpack-full for those. ## Config shape Each processor's `Config` gains: ```yaml encoding: msgpack # "proto" (default) or "msgpack" ``` New `SketchEncoding` string type + `EncodingProto` / `EncodingMsgpack` constants per processor. `Validate()` rejects unknown values with a clear error rather than silently falling back. ## Validation Same pre-existing `go.opentelemetry.io/collector/processor/selfmonitor` module-resolution issue as the earlier typed-DP refactor PRs blocks local `go build`. gofmt is clean on all 6 modified files. All API methods used (`SerializeMsgpack` on each sketch type, `*SketchEncodingMsgpack` on the pmetric patch) are already available: * sketchlib-go `CountMinSketch.SerializeMsgpack` — PR #51 * sketchlib-go `CountSketch.SerializeMsgpack` — PR #51 * sketchlib-go `HyperLogLog.SerializeMsgpack` — PR #51 * pmetric `CountMinSketchEncodingMsgpack` — PR #157 * pmetric `CountSketchEncodingMsgpack` — PR #157 * pmetric `HLLSketchEncodingMsgpack` — PR #157 ## Follow-ups * `ddsketchprocessor` MSGPACK option via a DataDog→sketchlib-go conversion shim (or a full internal migration). Tracked. * Delta msgpack wire format (requires sketchlib-go upstream work). * Full ddsketchprocessor migration from DataDog/sketches-go to sketchlib-go DDSketch — bigger refactor, not in this PR's scope. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 of 3 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
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.
Unblocks the DataCollector
ddsketchprocessormigration fromgithub.laiyagushi.com/DataDog/sketches-goto sketchlib-go. The processor needs a way to both emit and consume rawDDSketchStateproto bytes (no envelope wrapper) so it can round-trip sketches through the modified-OTLPDDSketchDataPoint.Sketchfield, which is what ASAPQuery-backend'ssketch_core::dd_sketch::DdSketchAccumulator::from_sketchlib_proto_bytesconsumes.Why not just
SerializePortable?SerializePortable()wraps theDDSketchStatein anenvpb.SketchEnvelopefor cross-sketch dispatch. When the caller already knows the sketch type — e.g. aDDSketchDataPointon the modified-OTLP wire is already tagged as DDSketch — the envelope overhead is wasted and incompatible with ASAPQuery-backend's decoder, which expects rawDdSketchState::decode(bytes), not an envelope.New API
(*DDSketch).SerializeStateProtoBytes() ([]byte, error)— returns prost-compatible bytes of the innerDDSketchStatemessage directly.NewFromState(state *ddpb.DDSketchState) (*DDSketch, error)— rebuilds aDDSketchfrom a decoded state proto. RecomputesIndexMappingfromstate.Alpha, reattaches the bucket store, seeds empty sketches with +Inf/-Inf sentinels.NewFromStateProtoBytes(data []byte) (*DDSketch, error)— convenience wrapper:proto.Unmarshal+NewFromState.Tests
Five new tests in
portable_test.go:TestSerializeStateProtoBytes_RoundTrip— emit bytes, decode as rawDDSketchState(no envelope) to pin the wire shape, reconstruct viaNewFromStateProtoBytes, verify count/sum/p50 match within alpha toleranceTestNewFromState_RejectsInvalidAlpha— 4 values outside (0, 1) each produce an errorTestNewFromState_NilReturnsError— defensive nil checkTestNewFromState_EmptySketchRoundTrip— empty sketch round-trips; reconstructed sketch accepts furtherAdd()calls (min/max sentinel seeding)TestNewFromState_NonEmptyBucketsRoundTrip— non-empty sketch round-trips; furtherAdd()calls keep working (bucket store reattachment is live, not a snapshot view)All 5 pass.
gofmt -lclean.Follow-up
A DataCollector PR will land right after this one that migrates
ddsketchprocessorfrom DataDog/sketches-go to sketchlib-go, usingNewFromStateProtoBytesfor input decode andSerializeStateProtoBytesfor output emission.🤖 Generated with Claude Code