feat(kllprocessor): emit typed KLLSketchDataPoint - #161
Merged
Merged
Conversation
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>
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.
Last of the four processor typed-emission refactors (#158: countmin, #159: countsketch, #160: hll dead-code cleanup, this PR: kll). Replaces Gauge-with-byte-attribute emission with typed
KLLSketchDataPointmessages so ASAPQuery-backend's modified-OTLP sketch router sees them asMetric.data = KLLSketch{...}variants instead of anonymous Gauges.Why
Before this PR the
TransmitSketchpath calledappendKLLSketchDataPointwhich emitted:ASAPQuery-backend's modified-OTLP decoder matches on
Metric.data = KLLSketch{...}(oneof tag 14, typedKLLSketchDataPoint). The Rust-side KLL decoder (ASAPQuery #8 §KLL, with lossy statistical reconstruction via item replay) was already wired up, but no processor produced the typed data points it wanted. Processor and decoder were both ready — just speaking past each other on the wire.What changed
processor.goTransmitSketchbranch now callsfindOrCreateKLLSketchMetric(creates a typedpmetric.MetricTypeKLLSketchmetric withAggregationTemporalityDelta) andappendTypedKLLSketchDataPoint(writesSetSketch/SetEncoding/SetCounton a typedKLLSketchDataPoint).m.SetEmptyGauge()replaced withm.SetEmptyKLLSketch().SetAggregationTemporality(...).findOrCreateKLLSketchMetricmirrorsfindOrCreateGaugeMetricbut creates typed KLL metrics and matches onMetricTypeso legacy Gauge metrics with the same name don't get accidentally reused.appendTypedKLLSketchDataPointreplaces the oldappendKLLSketchDataPoint. Writes the sketch payload viaSetSketchand tags the encoding withKLLSketchEncodingProto. Keepskll.kon the attribute map for operator visibility.Sum/Min/Maxfields are left at zero because sketchlib-go'sKLLSketchdoesn't track them — KLL is quantile-only.appendKLLSketchDataPointdeleted. OldfindOrCreateGaugeMetricretained because the non-TransmitSketchquantile-emission path still exports gauge-shaped scalar quantile series.processor_test.goTestBatchMode_TransmitSketchpreviously asserted onm.Gauge().DataPoints().At(0)and read the payload from thekll.sketch_payloadbyte attribute. Updated to assertm.Type() == pmetric.MetricTypeKLLSketch, readoutDP.Sketch()directly, and verifyoutDP.Encoding() == pmetric.KLLSketchEncodingProto.Other tests in the file target the quantile-emission path (
TransmitSketch: false), which still uses Gauge — unaffected.Validation
Same pre-existing
go.opentelemetry.io/collector/processor/selfmonitorresolution issue as #158/#159/#160 blocks localgo build.gofmt -l: clean on both modified filespmetric.MetricTypeKLLSketch,SetEmptyKLLSketch,KLLSketch().DataPoints().AppendEmpty(),SetSketch,SetEncoding,SetCount,KLLSketchEncodingProto) already exist on the pmetric patchStack
_ENCODING_PROTO; the MSGPACK enum value in #157 is reserved but not usable for KLL today.🤖 Generated with Claude Code