pmetric: add *_ENCODING_MSGPACK constants for all 5 sketches - #157
Merged
Merged
Conversation
…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
added a commit
that referenced
this pull request
Apr 15, 2026
Refactors the TransmitSketch path from Gauge-with-byte-attribute emission to typed `CountMinSketchDataPoint` messages so ASAPQuery- backend's modified-OTLP sketch router (ASAPQuery-backend PRs #5-#9) actually sees them as sketch variants instead of anonymous Gauges. ## Why Before this PR the processor emitted: metric.SetEmptyGauge() dp := gauge.DataPoints().AppendEmpty() dp.Attributes().PutEmptyBytes("sketch_payload").FromRaw(payload) dp.Attributes().PutStr("encoding", "proto_full") ASAPQuery-backend's modified-OTLP decoder matches on `Metric.data = CountMinSketch{data_points: [...]}` (oneof tag 16, typed `CountMinSketchDataPoint` messages) and never looks inside anonymous Gauge attribute maps for sketch bytes. Even though the backend has a full CountMin decoder (PR #6), the whole end-to-end flow was broken for this processor because nothing produced the typed data points the decoder wanted. After this PR: metric.SetEmptyCountMinSketch() cmsMetric.SetAggregationTemporality(pmetric.AggregationTemporalityDelta) dp := cmsMetric.DataPoints().AppendEmpty() outputAttrs.CopyTo(dp.Attributes()) dp.SetSampleCount(...) dp.SetRows(...) dp.SetCols(...) dp.SetSketch(payload) dp.SetEncoding(pmetric.CountMinSketchEncodingProto | Delta) The backend's router now sees `Metric.data.CountMinSketch{...}` and routes it straight into `CountMinSketchAccumulator::from_sketchlib_proto_bytes`. This unblocks the ASAPQuery PR #11 / #12 feedback loop for CountMin queries — a capability miss now leads to a plan push which leads to the backend actually finding a precomputed match next time, because the sketch bytes are now flowing through the typed hot path. ## What changed ### `processor.go` Split the emission path on `p.cfg.TransmitSketch`: * **TransmitSketch = true** (the production path): emit a typed `CountMinSketchDataPoint` with `SetSketch` / `SetEncoding` / `SetSampleCount` / `SetRows` / `SetCols`. The internal encoding string ("proto_delta" / "proto_full") is mapped onto the proto enum (`CountMinSketchEncodingDelta` / `...Proto`). * **TransmitSketch = false** (monitoring-only path): keep the legacy Gauge emission so existing dashboards that read the processor's output as a scalar `sample_count` series keep working. Nothing in that mode carries sketch bytes anyway. `aggregationTemporality` is set to Delta because the processor always produces one data point per window, and every window represents the delta within that window (not a cumulative sketch). The backend's accumulator merges across windows itself. ### `processor_test.go` Introduced a tiny `cmsTestDataPoint` adapter so existing test assertions that read fields via `.Attributes().Get("sketch_payload")` / `"encoding"` / `"sample_count"` / `"rows"` / `"cols"` keep compiling without per-site rewrites. The helper `getAllDataPoints` now dispatches on `pmetric.MetricTypeCountMinSketch` for the typed path and synthesizes the legacy attribute keys from the typed fields (`Sketch()` / `Encoding()` / etc.). The Gauge path (`TransmitSketch = false`) still works via the same helper — the adapter carries `doubleValue` for the one test (`TestBatchModeQueryMetricsWhenTransmitSketchDisabled`) that reads `dps[0].DoubleValue()`. `encodingToLegacyString` maps the proto enum back to the "proto_full" / "proto_delta" strings that existing assertions compare against. Drive-by fix: two assertions in processor_test.go previously looked for `"cms.sketch_payload"` (with a `cms.` prefix) which had never matched what the processor actually wrote (`"sketch_payload"`, no prefix). Those assertions were silently always failing on any CI that exercised them. Fixed to `"sketch_payload"` to match the (legacy-compatible, now synthesized) attribute key. ## What's NOT in this PR * **MSGPACK encoding option**: the typed emission path is now ready to accept a config flag (next PR will add `encoding: msgpack` and call sketchlib-go's `SerializeMsgpack` from PR #51, setting `CountMinSketchEncodingMsgpack` from PR #157). One-line branch once this refactor lands. * **The other 3 processors** (`countsketchprocessor`, `hllprocessor`, `kllprocessor`) need the same refactor but each has its own attribute quirks and test suite. Will ship as 3 separate PRs to keep review size bounded. ## Validation Local `go build` was attempted but the worktree's processor module is missing a `replace` directive for `go.opentelemetry.io/collector/processor`, so `processor/selfmonitor` fails to resolve — a pre-existing build setup issue unrelated to this PR. `gofmt -l` is clean on both modified files. All my changes use already-existing pmetric API methods (`SetEmptyCountMinSketch`, `SetSketch`, `SetEncoding`, `SetSampleCount`, `SetRows`, `SetCols`) and existing enum constants (`CountMinSketchEncodingProto` / `Delta`), so compilation should be straightforward in CI's properly-set-up module graph. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Apr 15, 2026
Refactors the TransmitSketch path from Gauge-with-byte-attribute emission to typed `CountMinSketchDataPoint` messages so ASAPQuery- backend's modified-OTLP sketch router (ASAPQuery-backend PRs #5-#9) actually sees them as sketch variants instead of anonymous Gauges. ## Why Before this PR the processor emitted: metric.SetEmptyGauge() dp := gauge.DataPoints().AppendEmpty() dp.Attributes().PutEmptyBytes("sketch_payload").FromRaw(payload) dp.Attributes().PutStr("encoding", "proto_full") ASAPQuery-backend's modified-OTLP decoder matches on `Metric.data = CountMinSketch{data_points: [...]}` (oneof tag 16, typed `CountMinSketchDataPoint` messages) and never looks inside anonymous Gauge attribute maps for sketch bytes. Even though the backend has a full CountMin decoder (PR #6), the whole end-to-end flow was broken for this processor because nothing produced the typed data points the decoder wanted. After this PR: metric.SetEmptyCountMinSketch() cmsMetric.SetAggregationTemporality(pmetric.AggregationTemporalityDelta) dp := cmsMetric.DataPoints().AppendEmpty() outputAttrs.CopyTo(dp.Attributes()) dp.SetSampleCount(...) dp.SetRows(...) dp.SetCols(...) dp.SetSketch(payload) dp.SetEncoding(pmetric.CountMinSketchEncodingProto | Delta) The backend's router now sees `Metric.data.CountMinSketch{...}` and routes it straight into `CountMinSketchAccumulator::from_sketchlib_proto_bytes`. This unblocks the ASAPQuery PR #11 / #12 feedback loop for CountMin queries — a capability miss now leads to a plan push which leads to the backend actually finding a precomputed match next time, because the sketch bytes are now flowing through the typed hot path. ## What changed ### `processor.go` Split the emission path on `p.cfg.TransmitSketch`: * **TransmitSketch = true** (the production path): emit a typed `CountMinSketchDataPoint` with `SetSketch` / `SetEncoding` / `SetSampleCount` / `SetRows` / `SetCols`. The internal encoding string ("proto_delta" / "proto_full") is mapped onto the proto enum (`CountMinSketchEncodingDelta` / `...Proto`). * **TransmitSketch = false** (monitoring-only path): keep the legacy Gauge emission so existing dashboards that read the processor's output as a scalar `sample_count` series keep working. Nothing in that mode carries sketch bytes anyway. `aggregationTemporality` is set to Delta because the processor always produces one data point per window, and every window represents the delta within that window (not a cumulative sketch). The backend's accumulator merges across windows itself. ### `processor_test.go` Introduced a tiny `cmsTestDataPoint` adapter so existing test assertions that read fields via `.Attributes().Get("sketch_payload")` / `"encoding"` / `"sample_count"` / `"rows"` / `"cols"` keep compiling without per-site rewrites. The helper `getAllDataPoints` now dispatches on `pmetric.MetricTypeCountMinSketch` for the typed path and synthesizes the legacy attribute keys from the typed fields (`Sketch()` / `Encoding()` / etc.). The Gauge path (`TransmitSketch = false`) still works via the same helper — the adapter carries `doubleValue` for the one test (`TestBatchModeQueryMetricsWhenTransmitSketchDisabled`) that reads `dps[0].DoubleValue()`. `encodingToLegacyString` maps the proto enum back to the "proto_full" / "proto_delta" strings that existing assertions compare against. Drive-by fix: two assertions in processor_test.go previously looked for `"cms.sketch_payload"` (with a `cms.` prefix) which had never matched what the processor actually wrote (`"sketch_payload"`, no prefix). Those assertions were silently always failing on any CI that exercised them. Fixed to `"sketch_payload"` to match the (legacy-compatible, now synthesized) attribute key. ## What's NOT in this PR * **MSGPACK encoding option**: the typed emission path is now ready to accept a config flag (next PR will add `encoding: msgpack` and call sketchlib-go's `SerializeMsgpack` from PR #51, setting `CountMinSketchEncodingMsgpack` from PR #157). One-line branch once this refactor lands. * **The other 3 processors** (`countsketchprocessor`, `hllprocessor`, `kllprocessor`) need the same refactor but each has its own attribute quirks and test suite. Will ship as 3 separate PRs to keep review size bounded. ## Validation Local `go build` was attempted but the worktree's processor module is missing a `replace` directive for `go.opentelemetry.io/collector/processor`, so `processor/selfmonitor` fails to resolve — a pre-existing build setup issue unrelated to this PR. `gofmt -l` is clean on both modified files. All my changes use already-existing pmetric API methods (`SetEmptyCountMinSketch`, `SetSketch`, `SetEncoding`, `SetSampleCount`, `SetRows`, `SetCols`) and existing enum constants (`CountMinSketchEncodingProto` / `Delta`), so compilation should be straightforward in CI's properly-set-up module graph. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Apr 15, 2026
Mirrors PR #158 for the CountSketch variant: replaces Gauge-with- byte-attribute emission with typed `CountSketchDataPoint` messages so ASAPQuery-backend's modified-OTLP sketch router sees them as `Metric.data = CountSketch{...}` variants instead of anonymous Gauges. ## Why Before this PR the processor emitted: metric.SetEmptyGauge() dp := gauge.DataPoints().AppendEmpty() dp.Attributes().PutEmptyBytes("sketch_payload").FromRaw(payload) dp.Attributes().PutStr("encoding", "proto_full") dp.Attributes().PutStr("partition_key", partitionKey) dp.Attributes().PutDouble("epsilon", p.config.Epsilon) dp.Attributes().PutDouble("delta", p.config.Delta) The ASAPQuery-backend's modified-OTLP decoder matches on `Metric.data = CountSketch{data_points: [...]}` (oneof tag 15, typed `CountSketchDataPoint` messages) and never looked at Gauge attribute maps. The decoder for CountSketch (ASAPQuery PR #15) was ready but nothing produced the typed data points it wanted. After this PR the same loop emits `CountSketchDataPoint` with `SetSketch` / `SetEncoding` / `SetDimension` / `SetEpsilon` / `SetDelta`, and the previously-orphaned backend decoder starts seeing real sketch bytes on the wire. ## What changed ### `processor.go` Split emission on `p.config.TransmitSketch`: * **TransmitSketch = true** (production): typed `CountSketchDataPoint`. The processor's `partition_key` string maps naturally onto `CountSketchDataPoint.Dimension` (both identify which sub-population the sketch covers). `Epsilon` and `Delta` get dedicated setters on the typed DP. `sample_count` and `window_duration_seconds` remain attribute-map entries — the typed DP has no setter for them, and they're observability-only (the backend doesn't use them for routing). `AggregationTemporality` set to `Delta` because each emission represents one window's delta. * **TransmitSketch = false**: keep the legacy Gauge emission so existing scalar-series dashboards continue to work. ### `delta_transmission_test.go` Same pattern as PR #158 — introduced a `csTestDataPoint` adapter and rewrote `getCSOutputDPs` to dispatch on `pmetric.MetricTypeCountSketch` (typed path) or `MetricTypeGauge` (legacy path) and synthesize the legacy attribute keys (`sketch_payload`, `encoding`, `partition_key`, `epsilon`, `delta`) from the typed fields, so every existing test assertion that reads via `dps[0].Attributes().Get(...)` keeps compiling without per-site rewrites. `csEncodingToLegacyString` maps `CountSketchEncodingProto` → "proto_full" / `CountSketchEncodingDelta` → "proto_delta" for the legacy string comparisons. ### `processor_test.go` Two tests (`TestGroupByPartitioning`, `TestWindowModeGroupBy`) previously walked `ms.At(k).Gauge().DataPoints()` directly to extract `partition_key` attributes from the output. Both now route through the `getCSOutputDPs` adapter from `delta_transmission_test.go` so they see the typed path transparently. Incidental gofmt drift on pre-existing struct literal indentation is included because `gofmt -w` fired on the whole file. None of it is functional. ## Validation Same pre-existing `go.opentelemetry.io/collector/processor/selfmonitor` resolution issue as PR #158 blocks local `go build` in this worktree layout — a go.mod replace directive gap that predates my changes. `gofmt -l` is clean on my three touched files. All API methods used (`SetEmptyCountSketch`, `SetSketch`, `SetEncoding`, `SetDimension`, `SetEpsilon`, `SetDelta`, `CountSketchEncodingProto` / `Delta`, `MetricTypeCountSketch`) are already exposed by pmetric — no new API needed. ## Follow-ups * `hllprocessor` — same refactor next, then `kllprocessor`. * MSGPACK encoding option per-processor once all four typed refactors land — one-line branch selecting sketchlib-go's `SerializeMsgpack` and `CountSketchEncodingMsgpack` (from PR #157). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Apr 15, 2026
Last of the four processor typed-emission refactors (PR #158: countmin, PR #159: countsketch, PR #160: hll dead-code cleanup, this PR: kll). Replaces Gauge-with-byte-attribute emission with typed `KLLSketchDataPoint` messages so ASAPQuery-backend's modified-OTLP sketch router sees them as `Metric.data = KLLSketch{...}` variants instead of anonymous Gauges. ## Why Before this PR the `TransmitSketch` path called `appendKLLSketchDataPoint` which emitted: dp := metric.Gauge().DataPoints().AppendEmpty() dp.Attributes().PutInt("kll.k", int64(k)) dp.Attributes().PutInt("kll.count", int64(sketch.Count())) dp.Attributes().PutEmptyBytes("kll.sketch_payload").FromRaw(payload) dp.SetDoubleValue(float64(sketch.Count())) ASAPQuery-backend's modified-OTLP decoder matches on `Metric.data = KLLSketch{data_points: [...]}` (oneof tag 14, typed `KLLSketchDataPoint`). The Rust-side KLL decoder (ASAPQuery PR #8 §KLL, with lossy statistical reconstruction via item replay) was already wired up, but no processor produced the typed data points it wanted. The processor and the decoder were both ready for each other but speaking past each other on the wire. ## What changed ### `processor.go` * **Batch path** (around line 226) — `TransmitSketch` branch now calls `findOrCreateKLLSketchMetric` (creates a typed `pmetric.MetricTypeKLLSketch` metric with `AggregationTemporalityDelta`) and `appendTypedKLLSketchDataPoint` (writes `SetSketch` / `SetEncoding` / `SetCount` on a typed `KLLSketchDataPoint`). * **Windowed path** (around line 486) — same two helpers. The inline `m.SetEmptyGauge()` is replaced with `m.SetEmptyKLLSketch().SetAggregationTemporality(...)`. * New helper `findOrCreateKLLSketchMetric` mirrors the existing `findOrCreateGaugeMetric` but creates typed KLL metrics and matches on `MetricType` so legacy Gauge metrics with the same name don't get accidentally reused. * New helper `appendTypedKLLSketchDataPoint` replaces the old `appendKLLSketchDataPoint`. Writes the sketch payload via `SetSketch` and tags the encoding with `KLLSketchEncodingProto`. Still carries `kll.k` on the attribute map for operator visibility (the typed DP has no dedicated setter for it; the Rust side derives k from the serialized payload). Sum/Min/Max fields on the typed DP are left at zero because sketchlib-go's `KLLSketch` type doesn't track them — KLL is quantile-only and the proto fields are observability-only. * Old `appendKLLSketchDataPoint` function deleted. Old `findOrCreateGaugeMetric` retained because the non-TransmitSketch quantile-emission path (lines 233-246 and 503-540) still exports gauge-shaped scalar quantile series. ### `processor_test.go` `TestBatchMode_TransmitSketch` (lines 68-115) previously asserted on `m.Gauge().DataPoints().At(0)` and read the payload from the `kll.sketch_payload` byte attribute. Updated to: * Assert `m.Type() == pmetric.MetricTypeKLLSketch` * Read the payload from `outDP.Sketch()` directly * Assert `outDP.Encoding() == pmetric.KLLSketchEncodingProto` Other tests in the file (TestBatchModeNoStatePersistence and below) all target the quantile-emission path (`TransmitSketch: false`), which still uses Gauge — they're unaffected. ## Validation Same pre-existing `go.opentelemetry.io/collector/processor/selfmonitor` resolution issue as PRs #158/#159/#160 blocks local `go build`. `gofmt -l` clean on both modified files. All API methods used (`pmetric.MetricTypeKLLSketch`, `SetEmptyKLLSketch`, `KLLSketch().DataPoints().AppendEmpty()`, `SetSketch`, `SetEncoding`, `SetCount`, `KLLSketchEncodingProto`) already exist on the pmetric patch. ## Stack * PR #158 (merged) — countminsketchprocessor typed emission * PR #159 — countsketchprocessor typed emission * PR #160 — hllprocessor dead-code cleanup + broken-test fix * **This PR** — kllprocessor typed emission * Next — per-processor MSGPACK config option (one-line branch each, calling sketchlib-go `SerializeMsgpack` from PR #51 and setting `*SketchEncodingMsgpack` from PR #157). Note: for KLL specifically, sketchlib-go #50/#51 out-of-scope'd the msgpack wire format because sketchlib-go's KLL and sketch-core's KLL don't share a byte-level backend — so KLL producers should keep using `_ENCODING_PROTO`. The MSGPACK enum value in PR #157 is reserved but not usable for KLL today. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Apr 15, 2026
Mirrors PR #158 for the CountSketch variant: replaces Gauge-with- byte-attribute emission with typed `CountSketchDataPoint` messages so ASAPQuery-backend's modified-OTLP sketch router sees them as `Metric.data = CountSketch{...}` variants instead of anonymous Gauges. ## Why Before this PR the processor emitted: metric.SetEmptyGauge() dp := gauge.DataPoints().AppendEmpty() dp.Attributes().PutEmptyBytes("sketch_payload").FromRaw(payload) dp.Attributes().PutStr("encoding", "proto_full") dp.Attributes().PutStr("partition_key", partitionKey) dp.Attributes().PutDouble("epsilon", p.config.Epsilon) dp.Attributes().PutDouble("delta", p.config.Delta) The ASAPQuery-backend's modified-OTLP decoder matches on `Metric.data = CountSketch{data_points: [...]}` (oneof tag 15, typed `CountSketchDataPoint` messages) and never looked at Gauge attribute maps. The decoder for CountSketch (ASAPQuery PR #15) was ready but nothing produced the typed data points it wanted. After this PR the same loop emits `CountSketchDataPoint` with `SetSketch` / `SetEncoding` / `SetDimension` / `SetEpsilon` / `SetDelta`, and the previously-orphaned backend decoder starts seeing real sketch bytes on the wire. ## What changed ### `processor.go` Split emission on `p.config.TransmitSketch`: * **TransmitSketch = true** (production): typed `CountSketchDataPoint`. The processor's `partition_key` string maps naturally onto `CountSketchDataPoint.Dimension` (both identify which sub-population the sketch covers). `Epsilon` and `Delta` get dedicated setters on the typed DP. `sample_count` and `window_duration_seconds` remain attribute-map entries — the typed DP has no setter for them, and they're observability-only (the backend doesn't use them for routing). `AggregationTemporality` set to `Delta` because each emission represents one window's delta. * **TransmitSketch = false**: keep the legacy Gauge emission so existing scalar-series dashboards continue to work. ### `delta_transmission_test.go` Same pattern as PR #158 — introduced a `csTestDataPoint` adapter and rewrote `getCSOutputDPs` to dispatch on `pmetric.MetricTypeCountSketch` (typed path) or `MetricTypeGauge` (legacy path) and synthesize the legacy attribute keys (`sketch_payload`, `encoding`, `partition_key`, `epsilon`, `delta`) from the typed fields, so every existing test assertion that reads via `dps[0].Attributes().Get(...)` keeps compiling without per-site rewrites. `csEncodingToLegacyString` maps `CountSketchEncodingProto` → "proto_full" / `CountSketchEncodingDelta` → "proto_delta" for the legacy string comparisons. ### `processor_test.go` Two tests (`TestGroupByPartitioning`, `TestWindowModeGroupBy`) previously walked `ms.At(k).Gauge().DataPoints()` directly to extract `partition_key` attributes from the output. Both now route through the `getCSOutputDPs` adapter from `delta_transmission_test.go` so they see the typed path transparently. Incidental gofmt drift on pre-existing struct literal indentation is included because `gofmt -w` fired on the whole file. None of it is functional. ## Validation Same pre-existing `go.opentelemetry.io/collector/processor/selfmonitor` resolution issue as PR #158 blocks local `go build` in this worktree layout — a go.mod replace directive gap that predates my changes. `gofmt -l` is clean on my three touched files. All API methods used (`SetEmptyCountSketch`, `SetSketch`, `SetEncoding`, `SetDimension`, `SetEpsilon`, `SetDelta`, `CountSketchEncodingProto` / `Delta`, `MetricTypeCountSketch`) are already exposed by pmetric — no new API needed. ## Follow-ups * `hllprocessor` — same refactor next, then `kllprocessor`. * MSGPACK encoding option per-processor once all four typed refactors land — one-line branch selecting sketchlib-go's `SerializeMsgpack` and `CountSketchEncodingMsgpack` (from PR #157). Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Apr 15, 2026
Last of the four processor typed-emission refactors (PR #158: countmin, PR #159: countsketch, PR #160: hll dead-code cleanup, this PR: kll). Replaces Gauge-with-byte-attribute emission with typed `KLLSketchDataPoint` messages so ASAPQuery-backend's modified-OTLP sketch router sees them as `Metric.data = KLLSketch{...}` variants instead of anonymous Gauges. ## Why Before this PR the `TransmitSketch` path called `appendKLLSketchDataPoint` which emitted: dp := metric.Gauge().DataPoints().AppendEmpty() dp.Attributes().PutInt("kll.k", int64(k)) dp.Attributes().PutInt("kll.count", int64(sketch.Count())) dp.Attributes().PutEmptyBytes("kll.sketch_payload").FromRaw(payload) dp.SetDoubleValue(float64(sketch.Count())) ASAPQuery-backend's modified-OTLP decoder matches on `Metric.data = KLLSketch{data_points: [...]}` (oneof tag 14, typed `KLLSketchDataPoint`). The Rust-side KLL decoder (ASAPQuery PR #8 §KLL, with lossy statistical reconstruction via item replay) was already wired up, but no processor produced the typed data points it wanted. The processor and the decoder were both ready for each other but speaking past each other on the wire. ## What changed ### `processor.go` * **Batch path** (around line 226) — `TransmitSketch` branch now calls `findOrCreateKLLSketchMetric` (creates a typed `pmetric.MetricTypeKLLSketch` metric with `AggregationTemporalityDelta`) and `appendTypedKLLSketchDataPoint` (writes `SetSketch` / `SetEncoding` / `SetCount` on a typed `KLLSketchDataPoint`). * **Windowed path** (around line 486) — same two helpers. The inline `m.SetEmptyGauge()` is replaced with `m.SetEmptyKLLSketch().SetAggregationTemporality(...)`. * New helper `findOrCreateKLLSketchMetric` mirrors the existing `findOrCreateGaugeMetric` but creates typed KLL metrics and matches on `MetricType` so legacy Gauge metrics with the same name don't get accidentally reused. * New helper `appendTypedKLLSketchDataPoint` replaces the old `appendKLLSketchDataPoint`. Writes the sketch payload via `SetSketch` and tags the encoding with `KLLSketchEncodingProto`. Still carries `kll.k` on the attribute map for operator visibility (the typed DP has no dedicated setter for it; the Rust side derives k from the serialized payload). Sum/Min/Max fields on the typed DP are left at zero because sketchlib-go's `KLLSketch` type doesn't track them — KLL is quantile-only and the proto fields are observability-only. * Old `appendKLLSketchDataPoint` function deleted. Old `findOrCreateGaugeMetric` retained because the non-TransmitSketch quantile-emission path (lines 233-246 and 503-540) still exports gauge-shaped scalar quantile series. ### `processor_test.go` `TestBatchMode_TransmitSketch` (lines 68-115) previously asserted on `m.Gauge().DataPoints().At(0)` and read the payload from the `kll.sketch_payload` byte attribute. Updated to: * Assert `m.Type() == pmetric.MetricTypeKLLSketch` * Read the payload from `outDP.Sketch()` directly * Assert `outDP.Encoding() == pmetric.KLLSketchEncodingProto` Other tests in the file (TestBatchModeNoStatePersistence and below) all target the quantile-emission path (`TransmitSketch: false`), which still uses Gauge — they're unaffected. ## Validation Same pre-existing `go.opentelemetry.io/collector/processor/selfmonitor` resolution issue as PRs #158/#159/#160 blocks local `go build`. `gofmt -l` clean on both modified files. All API methods used (`pmetric.MetricTypeKLLSketch`, `SetEmptyKLLSketch`, `KLLSketch().DataPoints().AppendEmpty()`, `SetSketch`, `SetEncoding`, `SetCount`, `KLLSketchEncodingProto`) already exist on the pmetric patch. ## Stack * PR #158 (merged) — countminsketchprocessor typed emission * PR #159 — countsketchprocessor typed emission * PR #160 — hllprocessor dead-code cleanup + broken-test fix * **This PR** — kllprocessor typed emission * Next — per-processor MSGPACK config option (one-line branch each, calling sketchlib-go `SerializeMsgpack` from PR #51 and setting `*SketchEncodingMsgpack` from PR #157). Note: for KLL specifically, sketchlib-go #50/#51 out-of-scope'd the msgpack wire format because sketchlib-go's KLL and sketch-core's KLL don't share a byte-level backend — so KLL producers should keep using `_ENCODING_PROTO`. The MSGPACK enum value in PR #157 is reserved but not usable for KLL today. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
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
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>
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.
Hand-adds the
*_ENCODING_MSGPACK = 3and*_ENCODING_MSGPACK_DELTA = 4variants to thepmetricgenerated Go enums and wrappers so any follow-up processor PR can referencepmetric.*SketchEncodingMsgpackdirectly. Matches the cross-language wire contract with ASAPQuery-backend PR #9 and sketchlib-go PR #51.Why
The patch file headers say
Code generated by pdatagen/main.go — DO NOT EDIT, but these files live inopentelemetry-collector-patch/pdata/— the DataCollector repo's manual override of the upstream collector'spdatapackage. #154 added the MSGPACK variants to the proto inopentelemetry-proto-patch/opentelemetry/proto/metrics/v1/metrics.proto, but the Go enums derived from that proto were never regenerated. This PR closes that gap.sketchlib-go PR #51 landed
SerializeMsgpack()methods on each sketch type. ASAPQuery-backend PR #9 decodes the msgpack wire format. The only missing piece for a DataCollector sketch processor to emitMetric.data = *Sketch{encoding: MSGPACK, sketch: ...}is the Go enum constant the processor code can assign todp.SetEncoding(...). Without this PR, a processor author has to runmake genpdatain the vendored collector-patch tree (which nobody documents) and commit regenerated files. 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 constants and extends the name/value maps +
String()switch.internal/generated_enum_countminsketchencoding.go+pmetric/countminsketch_encoding.gointernal/generated_enum_countsketchencoding.go+pmetric/countsketch_encoding.gointernal/generated_enum_ddsketchencoding.go+pmetric/ddsketch_encoding.gointernal/generated_enum_hllsketchencoding.go+pmetric/hllsketch_encoding.gointernal/generated_enum_kllsketchencoding.go+pmetric/kllsketch_encoding.goPer-sketch notes
CountMin / CountSketch / HLL: wired straight through; sketchlib-go has concrete
SerializeMsgpackmethods for these three.DDSketch: constant added, but the docstring warns that the DataDog library's internal state does not map directly to sketchlib-go's cross-language wire format. A processor using this encoding needs to convert
gamma → alphaand extract buckets explicitly.KLL: docstring warns this is not implementable end-to-end today — sketchlib-go's KLL and ASAPQuery-backend's sketch-core KLL don't share a byte-level backend. Enum reserved for parity and future use once a shared backend exists. See sketchlib-go Window aggregation per series #50's out-of-scope note and series aggregation at each timestamp #51's KLL omission.
What's NOT in this PR
The actual processor wire-up is a separate follow-up. Scope surfaced during implementation:
countminsketchprocessor/countsketchprocessor/hllprocessorcurrently emit sketches as Gauge data points withsketch_payloadbyte attributes, NOT as typed*SketchDataPointmessages. 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 tometric.SetEmptyCountMinSketch().DataPoints().AppendEmpty()+SetSketch+SetEncoding) before the msgpack encoding option becomes meaningful.ddsketchprocessoralready emits typedDDSketchDataPointmessages viaSetSketch+SetEncoding(see processor.go:282-283), so it's the closest to ready — but itsgithub.com/DataDog/sketches-godependency doesn't provide the cross-language msgpack format. It needs a conversion path or a switch to sketchlib-go's DDSketch.Both gaps 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.
Stack
*SketchDataPoint) + per-processor MSGPACK config option, one PR per processor🤖 Generated with Claude Code