feat(gateway): patched-build gateway with raw-or-sketch aggregate modes - #206
Merged
Merged
Conversation
Real fix for the warm-tier sketch-drop the e2e harness has been working around. #205's yaml-only swap (PRW → OTLP exporter) didn't solve it because the drop wasn't at the exporter — it was at the gateway's pdata *unmarshal* step. Stock `otel/opentelemetry-collector-contrib:0.108.0`'s pdata only knows the standard `Metric.data` OneOf variants (Gauge / Sum / Histogram / ExpHistogram / Summary). Tags 13–17 (DDSketch / KLLSketch / HLLSketch / CountSketch / CountMinSketch) hit the `default:` arm at `opentelemetry-collector/pdata/internal/generated_proto_metric.go:1201` which calls `proto.ConsumeUnknown(...)`. That advances past the bytes without storing them — there's no `XXX_unrecognized` field on the Metric struct to catch them — so by the time any exporter sees the metric, the typed sketch payload is gone. Fix: run the gateway from the ASAP-patched OTel collector — the same `asap/sketchcol:dev` image agents already use, whose pdata is generated from the patched proto and round-trips tags 13–17 intact. base.yml: - gateway service: image stock 0.108 → asap/sketchcol:dev. - Mounted config selectable via $GATEWAY_CONFIG env var (mirrors $AGENT_CONFIG). Default = gateway.yaml. - Self-telemetry host port 18889:8889 → 18890:8890 (the patched build doesn't include a prometheus exporter on 8889; self- telemetry has been on 8890 since #205, intra-docker scrape was already pointed there). deploy/configs/: - gateway.yaml — pure forwarder (default). Updated comments to document the patched-build dependency + cross-reference the two aggregate variants. - gateway-aggregate-from-raw.yaml — agents send raw OTLP; gateway runs DDSketch processor at gateway tier (KLL / HLL / CS / CMS blocks present but commented out — uncomment to add). Use when sketch placement is at the gateway tier per the controller's plan. - gateway-aggregate-from-sketches.yaml — agents already sketched; gateway runs countminsketchmerge + countsketchmerge to reconstruct full state from delta payloads. DD / KLL / HLL pass through unchanged (merge processors for those don't exist yet; backend handles per-agent merging via the accumulator's merge_into path). Builder manifests (both `builder-config.yaml` and the e2e-harness-trimmed `builder-config-sketches.yaml`): - Add countminsketchmergeprocessor + countsketchmergeprocessor. Their factories expose `countminsketchmerge` / `countsketchmerge` as the OTel component types — the names referenced in the new gateway-aggregate-from-sketches.yaml. PROGRESS.md follow-up #1 rewritten to document the real root cause and the real fix; the yaml-only changes from #205 are still in main and remain correct (just not sufficient alone). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
zzylol
added a commit
that referenced
this pull request
May 1, 2026
Caught when running the e2e for the first time after #206 merged. Three gateway configs landed in #206 referenced components that the patched `asap/sketchcol:dev` build doesn't include: 1. **`prometheusremotewriteexporter` is not in the patched build.** The OCB manifest at `opentelemetry-collector-contrib-patch/cmd/sketchcollector/builder-config.yaml` only ships `otlpexporter`, `otlphttpexporter`, `debugexporter`, `prometheusexporter` — no PRW. `gateway.yaml`'s `prometheusremotewrite/backend` block (kept as a "non-active fallback") fails the config decode on startup: 'exporters' unknown type: "prometheusremotewrite" (valid values: [otlphttp prometheus debug otlp]) Removed the block. Comment in its place documents that the PRW exporter would need to be added to the OCB manifest + rebuild; sketch-native pipelines don't need it. 2. **`service.telemetry.metrics.address` is the deprecated v0.140 shorthand.** v0.141 only accepts the `readers:` form (matches what the agent configs already use). Same pattern in all three gateway configs: readers: - pull: exporter: prometheus: host: 0.0.0.0 port: 8890 3. **`gateway-aggregate-from-sketches.yaml` was using the wrong processors.** I claimed in #206 that DD/KLL/HLL needed new merge processors as a follow-up. That was wrong. The existing five sketch processors already handle merge for the typed wire format — each one's input switch dispatches both raw inputs AND typed sketch inputs into the same windowed accumulator (see `processor/countminsketchprocessor/processor.go:269` `case pmetric.MetricTypeCountMinSketch:` and `processor/ddsketchprocessor/processor.go:206` `consumeDDSketchDataPoints`). Configuring the regular `ddsketch` processor at the gateway with the same window the agent used produces a windowed cross-agent merge. The legacy `countminsketchmergeprocessor` / `countsketchmergeprocessor` are for the OLD Gauge-with-payload wire format only (where sketch bytes were stuffed into a `sketch_payload` byte attribute on a Gauge data point). The typed wire format (`Metric.data` variants 13–17) supersedes that. The follow-up PR I said I'd open isn't needed. This config now uses `ddsketch` (same as the agent) with KLL / HLL / CountSketch / CMS commented in for opt-in, and a prominent comment block explaining the dual-mode dispatch. PROGRESS.md item #1 updated to reflect both the boot fix and the merge-processors clarification, plus a record of the e2e probe that proved the gateway preservation works (Gauge probe via HTTP traversed agent-tier → gateway → backend OTLP receiver intact). Co-authored-by: Claude Opus 4.7 (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.
Summary
The actual fix for the warm-tier sketch drop. #205's yaml-only swap (PRW → OTLP exporter) wasn't sufficient — sketches were already gone before any exporter saw them.
Root cause
Stock
otel/opentelemetry-collector-contrib:0.108.0's pdata only knows the standardMetric.dataOneOf variants (Gauge / Sum / Histogram / ExpHistogram / Summary). Tags 13–17 (DDSketch / KLLSketch / HLLSketch / CountSketch / CountMinSketch) hit thedefault:arm atopentelemetry-collector/pdata/internal/generated_proto_metric.go:1201:default: pos, err = proto.ConsumeUnknown(buf, pos, wireType)ConsumeUnknown(pdata/internal/proto/unmarshal.go:32) advances past the bytes without storing them. There's noXXX_unrecognizedfield on the Metric struct to catch them either. So by the time any exporter sees the metric, the typed sketch payload is gone — regardless of whether the exporter is PRW or OTLP.Fix
Run the gateway from the ASAP-patched OTel collector — the same
asap/sketchcol:devimage the agents already use. Its pdata is generated from the patched proto and round-trips tags 13–17 intact.Changes
base.ymlgateway service:otel/opentelemetry-collector-contrib:0.108.0→asap/sketchcol:dev.$GATEWAY_CONFIG(mirrors$AGENT_CONFIG). Default =gateway.yaml.18889:8889→18890:8890(patched build has no prometheus exporter on 8889; self-telemetry has been on 8890 since feat(deploy): warm-tier OTLP end-to-end — gateway → backend OTLP, alt query_engine_rust image #205).Three gateway configs in
deploy/configs/:gateway.yamlattributes/drop-internal → batch → otlp/backendgateway-aggregate-from-raw.yamlattributes/drop-internal → ddsketch → batch → otlp/backend(KLL/HLL/CS/CMS blocks commented in for opt-in)gateway-aggregate-from-sketches.yamlattributes/drop-internal → countminsketchmerge → countsketchmerge → batch → otlp/backendBuilder manifests (
builder-config.yaml+builder-config-sketches.yaml): addcountminsketchmergeprocessor+countsketchmergeprocessor. Their factories exposecountminsketchmerge/countsketchmergeas the OTel component types.Coverage notes
gateway-aggregate-from-sketches.yamlunchanged. Merge processors for those don't exist yet — backend handles per-agent merging via the accumulator'smerge_intopath. (sketchlib-go has the merge math for all three; an OTel processor wrapping it is a follow-up.)--enable-otel-ingeston backend) are still in main and remain correct — they just weren't sufficient alone.Why this is the right shape for the controller's placement decision
Same image, same processors compiled in, three ready configs — the controller's plan picks where each sketch is computed (per-agent vs gateway-tier vs none) by selecting the matching config at deploy time. Adding a new placement strategy is a config swap, not a new image.
Test plan
docker compose ... config --quietparses cleanly with all three$GATEWAY_CONFIGvalues.service.pipelines.metrics.processors.NewFactory()and the typeStr (countminsketchmerge/countsketchmerge) matches the config references.asap/sketchcol:devviabuild_sketchcollector.sh(now picks up the merge processors frombuilder-config.yaml) and verify the binary starts under each gateway config.GATEWAY_CONFIG=gateway.yamland run ahistogram_quantile(0.5, ...)PromQL through it — verify the answer comes back from the warm-tier (backend's OTLP precompute store), not cold-tier fallback. Then repeat withGATEWAY_CONFIG=gateway-aggregate-from-raw.yaml(agents emit raw, gateway sketches) andGATEWAY_CONFIG=gateway-aggregate-from-sketches.yaml(CMS / CountSketch metrics, verify the merged sketch reaches the backend).🤖 Generated with Claude Code