Skip to content

proto: add *_ENCODING_MSGPACK variants to sketch encoding enums - #154

Merged
zzylol merged 1 commit into
mainfrom
docs/proto-msgpack-encoding
Apr 14, 2026
Merged

proto: add *_ENCODING_MSGPACK variants to sketch encoding enums#154
zzylol merged 1 commit into
mainfrom
docs/proto-msgpack-encoding

Conversation

@zzylol

@zzylol zzylol commented Apr 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Adds _ENCODING_MSGPACK = 3 and _ENCODING_MSGPACK_DELTA = 4 to the five per-sketch encoding enums in the modified OTLP proto patch, aligning DataCollector with the consumer and producer sides of the cross-language MessagePack sketch contract.

Why

Before this PR the three sides of the contract had mismatched enum names:

Side Had MSGPACK names? Had MSGPACK support at runtime?
ASAPQuery-backend (Rust consumer, #9) ✅ Yes ✅ Dispatcher decodes tag 3 → per-sketch from_msgpack_bytes
sketchlib-go (Go producer, #50, merged) N/A (no proto deps in wire pkg) ✅ wire/asapmsgpack.Marshal* produces matching bytes
DataCollector (this PR) ❌ No ❌ Processors had to hard-code integer tag 3

Because protobuf enums are just integer tags, the runtime contract already works across all three — this is a pure readability / discoverability win. No generated code or runtime behavior changes until a follow-up wires a *processor component to actually emit the new tag (and the merged sketchlib-go wire/asapmsgpack package is now directly available as a Go import for that follow-up).

What's in scope

Affected enums in opentelemetry-proto-patch/opentelemetry/proto/metrics/v1/metrics.proto:

  • DDSketchEncoding — adds DDSKETCH_ENCODING_MSGPACK = 3 + _MSGPACK_DELTA = 4
  • KLLSketchEncoding — adds KLL_SKETCH_ENCODING_MSGPACK = 3 + _MSGPACK_DELTA = 4 (reserved; see note below)
  • CountSketchEncoding — adds COUNT_SKETCH_ENCODING_MSGPACK = 3 + _MSGPACK_DELTA = 4
  • CountMinSketchEncoding — adds COUNT_MIN_SKETCH_ENCODING_MSGPACK = 3 + _MSGPACK_DELTA = 4
  • HLLSketchEncoding — adds HLL_SKETCH_ENCODING_MSGPACK = 3 + _MSGPACK_DELTA = 4

KLL note: sketchlib-go's KLL does not share a byte-level backend with ASAPQuery-backend's datasketches-rs-backed KllSketch, so KLL_SKETCH_ENCODING_MSGPACK is not usable for KLL today — sketchlib-go #50 intentionally omits a KLL marshaller for this reason. The enum value is reserved for consistency; producers emitting KLL should use _PROTO (the Rust side does lossy statistical reconstruction within KLL's rank-error bound).

No changes to .proto message shapes, no reserved-range shifts, no impact on existing _PROTO = 1 / _PROTO_DELTA = 2 / _DELTA = 2 producers and consumers.

What's out of scope

  • Generated .pb.go — regenerated from the patch file via restore_otel_proto_patches.sh at build time; not committed
  • opentelemetry-proto submodule — touched only via the restore script; not committed
  • *processor wire-up — a follow-up can add a config option on each processor (countminsketchprocessor, countsketchprocessor, ddsketchprocessor, hllprocessor) to emit _ENCODING_MSGPACK instead of _ENCODING_PROTO, calling sketchlib-go's now-merged wire/asapmsgpack.Marshal* on the sketch state. This is the next hop for the producer side of the contract.
  • _MSGPACK_DELTA — still deferred until sketch-core grows an apply_delta API; the enum entries exist only to reserve the tag numbers consistently with the Rust side

🤖 Generated with Claude Code

@zzylol @claude
proto: add *_ENCODING_MSGPACK / _MSGPACK_DELTA variants for all five …
4010d95 
…sketches

Adds `_ENCODING_MSGPACK = 3` and `_ENCODING_MSGPACK_DELTA = 4` to the
five per-sketch encoding enums in the modified OTLP proto patch —
matching the variants landed in ASAPQuery-backend PR #9
(ProjectASAP/ASAPQuery-backend#9) and implemented
on the producer side in sketchlib-go PR #50
(ProjectASAP/sketchlib-go#50).

Before this PR the three sides of the cross-language MessagePack contract
had mismatched proto enum names:

  * ASAPQuery-backend (Rust consumer) — already had `_MSGPACK = 3` in its
    vendored metrics.proto, ready to decode msgpack-encoded sketch bytes
    from the hot path dispatcher
  * sketchlib-go (Go producer) — already has `wire/asapmsgpack.Marshal*`
    producing bytes the Rust consumer deserializes byte-for-byte
  * DataCollector — lacked the enum names entirely, so processors
    emitting msgpack bytes had to hard-code the integer tag `3`

This patch aligns the DataCollector proto with the other two, closing
the naming gap. Because protobuf enums are just integer tags, the change
is forward-compatible: producers and consumers that already know about
tag `3` at runtime continue to interoperate whether or not the name is
declared on their side of the wire. So this commit is a pure readability
/ discoverability win; no generated code or runtime behavior changes
until a follow-up wires a processor component to actually emit the new
tag.

Affected enums:
  * `DDSketchEncoding`        — adds MSGPACK=3, MSGPACK_DELTA=4
  * `KLLSketchEncoding`       — adds MSGPACK=3, MSGPACK_DELTA=4 (reserved;
                                sketchlib-go KLL doesn't share a byte-level
                                backend with ASAPQuery-backend's KLL, so
                                MSGPACK is not usable for KLL today — use
                                PROTO instead)
  * `CountSketchEncoding`     — adds MSGPACK=3, MSGPACK_DELTA=4
  * `CountMinSketchEncoding`  — adds MSGPACK=3, MSGPACK_DELTA=4
  * `HLLSketchEncoding`       — adds MSGPACK=3, MSGPACK_DELTA=4

No changes to .proto message shapes, no reserved-range shifts, no
impact on existing `_PROTO = 1` / `_PROTO_DELTA = 2` / `_DELTA = 2`
producers and consumers.

The generated `.pb.go` files and the opentelemetry-proto submodule
checkout are NOT committed — they're regenerated from this patch file
via `restore_otel_proto_patches.sh` at build time.

Follow-ups:
  * `*processor` components in DataCollector that today emit only
    `_ENCODING_PROTO` can gain a config option to emit `_ENCODING_MSGPACK`
    instead, calling sketchlib-go's `wire/asapmsgpack.Marshal*` on the
    sketch state
  * `_DELTA` variants still deferred until sketch-core grows an
    `apply_delta` API — these enum entries exist only to reserve the
    tag numbers consistently with the Rust side

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zzylol zzylol mentioned this pull request Apr 14, 2026
Merged
@zzylol
zzylol merged commit 799475c into main Apr 14, 2026
This was referenced Apr 14, 2026
Merged
Merged
zzylol added a commit that referenced this pull request Apr 15, 2026
@zzylol @claude
pmetric: add *_ENCODING_MSGPACK / _MSGPACK_DELTA constants for all 5 …
9f16539 
…sketches

Hand-adds the MessagePack encoding variants to both the internal
generated enum files and the pmetric wrappers so any future DataCollector
processor PR can reference `pmetric.*SketchEncodingMsgpack` directly,
matching the cross-language wire contract with ASAPQuery-backend and
sketchlib-go.

The patch file header says "Code generated by pdatagen/main.go — DO NOT
EDIT", but these files live in `opentelemetry-collector-patch/pdata/` —
the DataCollector repo's manual override of the upstream collector's
pdata package. PR #154 added the MSGPACK variants to the proto in
`opentelemetry-proto-patch/opentelemetry/proto/metrics/v1/metrics.proto`,
but the Go enums derived from that proto were never regenerated. This
PR closes that gap.

## Why

sketchlib-go PR #51 landed `SerializeMsgpack()` methods on each sketch
type. ASAPQuery-backend PR #9 decodes the msgpack wire format. PR #154
added the proto enum names. The only missing piece for a DataCollector
sketch processor to emit `Metric.data = *Sketch{encoding: MSGPACK,
sketch: ...}` is the Go enum constant the processor code can assign to
`dp.SetEncoding(...)`. Without this PR, a processor author has to run
`make genpdata` in the vendored collector-patch tree (which nobody
documents), regenerate several files, and commit the result. This PR
does that step once so follow-up processor wire-up PRs become
mechanical.

## Files touched (5 pairs = 10 files)

Each pair adds two new values (= 3, = 4) to the existing PROTO / DELTA
constants and extends the name/value maps + `String()` switch.

  * internal/generated_enum_countminsketchencoding.go
    + pmetric/countminsketch_encoding.go
  * internal/generated_enum_countsketchencoding.go
    + pmetric/countsketch_encoding.go
  * internal/generated_enum_ddsketchencoding.go
    + pmetric/ddsketch_encoding.go
  * internal/generated_enum_hllsketchencoding.go
    + pmetric/hllsketch_encoding.go
  * internal/generated_enum_kllsketchencoding.go
    + pmetric/kllsketch_encoding.go

## Per-sketch notes

  * **CountMin / CountSketch / HLL**: MSGPACK / MSGPACK_DELTA wired
    straight through; sketchlib-go has concrete `SerializeMsgpack`
    methods for these three.

  * **DDSketch**: constant added, but the docstring warns that the
    DataDog library's internal state (gamma-based mapping, bucket
    store) does not map directly to sketchlib-go's cross-language
    wire format. A processor using this encoding needs to convert
    gamma → alpha and extract buckets explicitly. The sketchlib-go
    `DDSketch.SerializeMsgpack()` method from PR #51 only works on
    sketchlib-go's own `*DDSketch` struct, not on the DataDog
    `*ddsketch.DDSketch` that `ddsketchprocessor` currently uses.
    Wiring this will require a conversion layer or a processor
    rewrite that uses sketchlib-go's DDSketch.

  * **KLL**: constant added but the docstring warns this is **not
    implementable end-to-end today** because sketchlib-go's KLL and
    ASAPQuery-backend's sketch-core KLL do not share a byte-level
    backend. Producers should keep using PROTO. The enum is
    reserved for parity with the other sketches and for future use
    once a shared backend exists. See sketchlib-go PR #50 §out-of-
    scope and PR #51's KLL omission note.

## What's NOT in this PR

The actual processor wire-up — adding a config option to each
`*sketchprocessor` and calling `SerializeMsgpack` instead of the
existing proto path — is a separate follow-up. Surfaced scope
during implementation:

  1. **countminsketchprocessor / countsketchprocessor / hllprocessor**
     currently emit sketches as **Gauge data points with
     `sketch_payload` byte attributes**, NOT as typed
     `*SketchDataPoint` messages. The ASAPQuery-backend's
     modified-OTLP decoder only consumes typed data points, so
     these processors need a structural refactor (switch their
     emission path from Gauge-with-attributes to
     `metric.SetEmptyCountMinSketch().DataPoints().AppendEmpty()`
     and set the bytes via `SetSketch` + encoding via `SetEncoding`)
     before the msgpack encoding option becomes meaningful.

  2. **ddsketchprocessor** already emits typed `DDSketchDataPoint`
     messages via `SetSketch` + `SetEncoding` (see lines 282-283
     in its processor.go), so it's the closest to ready — but its
     `github.com/DataDog/sketches-go` dependency doesn't provide
     the cross-language msgpack format. It needs a conversion path
     or a switch to sketchlib-go's DDSketch.

Both gaps are tracked as follow-up PRs. They need a design call on
whether to refactor in place or migrate each processor to
sketchlib-go's sketch types first. This PR unblocks that work by
making the enum constants available; without it every such follow-up
would have to repeat the codegen step.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zzylol zzylol mentioned this pull request Apr 15, 2026
Merged
zzylol added a commit that referenced this pull request Apr 15, 2026
@zzylol @claude
pmetric: add *_ENCODING_MSGPACK / _MSGPACK_DELTA constants for all 5 …
384675c 
…sketches (#157)

Hand-adds the MessagePack encoding variants to both the internal
generated enum files and the pmetric wrappers so any future DataCollector
processor PR can reference `pmetric.*SketchEncodingMsgpack` directly,
matching the cross-language wire contract with ASAPQuery-backend and
sketchlib-go.

The patch file header says "Code generated by pdatagen/main.go — DO NOT
EDIT", but these files live in `opentelemetry-collector-patch/pdata/` —
the DataCollector repo's manual override of the upstream collector's
pdata package. PR #154 added the MSGPACK variants to the proto in
`opentelemetry-proto-patch/opentelemetry/proto/metrics/v1/metrics.proto`,
but the Go enums derived from that proto were never regenerated. This
PR closes that gap.

## Why

sketchlib-go PR #51 landed `SerializeMsgpack()` methods on each sketch
type. ASAPQuery-backend PR #9 decodes the msgpack wire format. PR #154
added the proto enum names. The only missing piece for a DataCollector
sketch processor to emit `Metric.data = *Sketch{encoding: MSGPACK,
sketch: ...}` is the Go enum constant the processor code can assign to
`dp.SetEncoding(...)`. Without this PR, a processor author has to run
`make genpdata` in the vendored collector-patch tree (which nobody
documents), regenerate several files, and commit the result. This PR
does that step once so follow-up processor wire-up PRs become
mechanical.

## Files touched (5 pairs = 10 files)

Each pair adds two new values (= 3, = 4) to the existing PROTO / DELTA
constants and extends the name/value maps + `String()` switch.

  * internal/generated_enum_countminsketchencoding.go
    + pmetric/countminsketch_encoding.go
  * internal/generated_enum_countsketchencoding.go
    + pmetric/countsketch_encoding.go
  * internal/generated_enum_ddsketchencoding.go
    + pmetric/ddsketch_encoding.go
  * internal/generated_enum_hllsketchencoding.go
    + pmetric/hllsketch_encoding.go
  * internal/generated_enum_kllsketchencoding.go
    + pmetric/kllsketch_encoding.go

## Per-sketch notes

  * **CountMin / CountSketch / HLL**: MSGPACK / MSGPACK_DELTA wired
    straight through; sketchlib-go has concrete `SerializeMsgpack`
    methods for these three.

  * **DDSketch**: constant added, but the docstring warns that the
    DataDog library's internal state (gamma-based mapping, bucket
    store) does not map directly to sketchlib-go's cross-language
    wire format. A processor using this encoding needs to convert
    gamma → alpha and extract buckets explicitly. The sketchlib-go
    `DDSketch.SerializeMsgpack()` method from PR #51 only works on
    sketchlib-go's own `*DDSketch` struct, not on the DataDog
    `*ddsketch.DDSketch` that `ddsketchprocessor` currently uses.
    Wiring this will require a conversion layer or a processor
    rewrite that uses sketchlib-go's DDSketch.

  * **KLL**: constant added but the docstring warns this is **not
    implementable end-to-end today** because sketchlib-go's KLL and
    ASAPQuery-backend's sketch-core KLL do not share a byte-level
    backend. Producers should keep using PROTO. The enum is
    reserved for parity with the other sketches and for future use
    once a shared backend exists. See sketchlib-go PR #50 §out-of-
    scope and PR #51's KLL omission note.

## What's NOT in this PR

The actual processor wire-up — adding a config option to each
`*sketchprocessor` and calling `SerializeMsgpack` instead of the
existing proto path — is a separate follow-up. Surfaced scope
during implementation:

  1. **countminsketchprocessor / countsketchprocessor / hllprocessor**
     currently emit sketches as **Gauge data points with
     `sketch_payload` byte attributes**, NOT as typed
     `*SketchDataPoint` messages. The ASAPQuery-backend's
     modified-OTLP decoder only consumes typed data points, so
     these processors need a structural refactor (switch their
     emission path from Gauge-with-attributes to
     `metric.SetEmptyCountMinSketch().DataPoints().AppendEmpty()`
     and set the bytes via `SetSketch` + encoding via `SetEncoding`)
     before the msgpack encoding option becomes meaningful.

  2. **ddsketchprocessor** already emits typed `DDSketchDataPoint`
     messages via `SetSketch` + `SetEncoding` (see lines 282-283
     in its processor.go), so it's the closest to ready — but its
     `github.com/DataDog/sketches-go` dependency doesn't provide
     the cross-language msgpack format. It needs a conversion path
     or a switch to sketchlib-go's DDSketch.

Both gaps are tracked as follow-up PRs. They need a design call on
whether to refactor in place or migrate each processor to
sketchlib-go's sketch types first. This PR unblocks that work by
making the enum constants available; without it every such follow-up
would have to repeat the codegen step.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the docs/proto-msgpack-encoding branch May 9, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@zzylol