Add ddsketch metric type in otel-collector-contrib telemetrygen - #15
Merged
Merged
Conversation
zzylol
changed the title
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
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>
SieDeta
pushed a commit
that referenced
this pull request
Apr 17, 2026
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.
No description provided.