fix(deploy): unblock fake-exporter rebuild — proto bindings + sketchlib-go rename adaptation - #262
Merged
zzylol merged 1 commit intoMay 5, 2026
Conversation
…ib-go rename adaptation Two independent blockers prevented `asap/fake-exporter:dev` from rebuilding off main HEAD; both fixed. ## Blocker A — missing Go bindings for opentelemetry-proto-patch The patched `metrics.proto` adds five sketch message types (DDSketch / KLLSketch / CountSketch / CountMinSketch / HLLSketch) on top of upstream v1.9.0, but the regenerated `.pb.go` files were never committed. Every build hit `undefined: mpb.Metric_Ddsketch / mpb.DDSketchDataPoint / ...` in the patched OTLP transform layer. Fix: regenerated the Go bindings via the upstream `make gen-go` recipe (otel/build-protobuf:0.9.0 image, `--go_out=plugins=grpc`) over a clean v1.9.0 + the patched `.proto` files; committed all 10 `.pb.go` outputs (11,385 lines total) under `opentelemetry-proto-patch/gen/go/...`. Updated `.gitignore` to track that subtree. Added an explicit `go.opentelemetry.io/proto/otlp` replace in `deploy/fake-exporter/go.mod` pointing at the gen tree (the dependency module's replace isn't honored from a downstream main module). Updated `Dockerfile.fake-exporter` to copy `opentelemetry-proto/` into the build context so the relative replace path resolves. Wrote `opentelemetry-proto-patch/REGEN.md` with the full regeneration recipe. ## Blocker B — sketchlib-go API rename refactor (PR #53) absorption The patched OTLP transform files referenced `metricdata.KLLSketchEncodingGob`, `metricdata.CountSketchEncodingGob`, `metricdata.CountMinSketchEncodingGob` and the corresponding `mpb.*_ENCODING_GOB` enum values. Those names were renamed to `*EncodingProto` / `*EncodingDelta` and `*_ENCODING_PROTO` / `*_ENCODING_DELTA` by the gob→proto encoding rename (commit b031975) and by sketchlib-go PR #53. Fix: updated KLL/CountSketch/CountMinSketch encoding switches in both the otlpmetricgrpc and otlpmetrichttp transform packages to dispatch on the post-rename names. CountSketch/CountMinSketch get the additional Delta arm matching the sketch wire's sparse-delta path; KLL keeps a single Proto arm (no delta defined for KLL). The sketchlib-go API renames per PR #53 (`Add`/`Insert` → `Update` on per-value, `InsertValue` → `UpdateValue` on HLL, `Insert(input)` / `OctoInsert` → `Update`, `EstimateCardinality()` → `Estimate()` on HLL, `GetValueAtQuantile` → `Quantile` on DDSketch, `InsertWeight`/`InsertBatch`/`InsertHashes`/`BulkInsert` → `UpdateWeight`/`UpdateBatch`/`UpdateHashes`/`BulkUpdate`) were already absorbed into the patched `sdk/metric/internal/aggregate/{hllsketch, kllsketch,countminsketch,countsketch,ddsketch}.go` and `asap-precompute-go/sketches/hll.go` before this round; the rebuild verifies they're correct. ## Verification - `docker builder prune -af` then `DOCKER_BUILDKIT=1 docker build -f deploy/docker/Dockerfile.fake-exporter --build-context sketchlib-go=/path/to/sketchlib-go -t asap/fake-exporter:dev .` → clean cold-cache build. - `go build ./...` and `go test ./...` clean in `opentelemetry-go/sdk/metric/` and `asap-precompute-go/`. - Recreated against the live E0 stack (b3-delta + e2e-overlay, N=1): `docker compose ... up -d --force-recreate fake-exporter` → clean startup logs, container Up 30s with no errors.
zzylol
deleted the
fix/fake-exporter-rebuild-after-sketchlib-go-rename-and-proto-regen
branch
May 5, 2026 19:16
5 tasks
zzylol
added a commit
that referenced
this pull request
May 5, 2026
…pe absorption (#269) Sweep blocker #1: `EXPORTER_SDK_AGG=dd-delta` was rejected by the OTLP exporter with `unknown ddsketch encoding: ddsketch_proto_delta` because PR #262 introduced `metricdata.DDSketchEncodingProtoDelta` but did not extend `DDSketchEncodingValue`'s switch to cover it (sibling sketches HLL/CountSketch/CountMinSketch were absorbed in #262 but DDSketch was missed). Without this, none of the matrix's DDSketch cells run. Fix 1 (encoding-value mapping): adds the `ProtoDelta` arm to `DDSketchEncodingValue` in `otlpmetricgrpc` + `otlpmetrichttp` exporters and the shared template. New `encoding_value_test.go` pins the post-#262 surface for all five sketches' `*EncodingValue` helpers, including a round-trip through `DDSketchDataPoints`. Pinning tests for the agent-side decoder in `opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/ envelope_decode_test.go` document the wire-format invariants `decodeDDSketchEnvelope` relies on (full-state round trip, bare-state rejection, empty-envelope rejection) so any future encoder regression is caught at the unit-test layer. Fix 2 (alpha wire-type) + Fix 3 (oneof variant) are NOT addressed in this PR. Investigation confirmed they share a single structural root cause: the SDK's DDSketch aggregator (`opentelemetry-go-patch/sdk/metric/internal/aggregate/ddsketch.go`) still serializes via `proto.Marshal(sk.ToProto())` — DataDog's `sketchpb.DDSketch` proto — while every consumer (asap-precompute-go's `DDSketchWrapper.ApplyDelta`, asap-precompute-rs's `DDSketchWrapper::decode_envelope`, the agent's `decodeDDSketchEnvelope`) expects sketchlib-go's `SketchEnvelope{DDSketchState}`. The schemas collide on field 1 wire types (DataDog's `Mapping` is LengthDelimited; sketchlib-go's `alpha` is fixed64), surfacing as the reported `DDSketchState.alpha: invalid wire type` error. Aligning consumers with the SDK's DataDog format would lose the cross-language byte parity #243 just landed; aligning the SDK with sketchlib-go requires migrating the aggregator off DataDog/sketches-go onto sketchlib-go's DDSketch — same path KLL, HLL, CountSketch, CountMinSketch already took. That migration sits outside this PR's touch list (only the encoding-value map, OTLP encoding-switch, processor.go decoder, and Rust wrapper were in scope per the sweep agent's autopilot envelope). Verification: - `go test ./internal/transform/...` green for both grpc + http exporters (8 new tests under `encoding_value_test.go`). - `go test ./...` green for `opentelemetry-collector-contrib/ processor/ddsketchprocessor` including the 3 new envelope-decode tests. - `go test ./...` green for `opentelemetry-go/sdk/metric/...`. - `cargo test --release` green for `asap-precompute-rs` (no Rust changes; verified parity tests still pass). No sibling sketch (HLL/CS/CMS) gap was found — the original PR #262 absorption is complete for them; only DDSketch was missed. KLLSketchEncodingValue intentionally has no Delta arm (KLL has no delta wire path). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
7 tasks
zzylol
added a commit
that referenced
this pull request
May 5, 2026
…etchlib-go (post-#262) (#270) PR #262 migrated KLL / HLL / CountSketch / CountMinSketch off DataDog sketches-go onto sketchlib-go but left DDSketch on DataDog. PR #269 diagnosed the resulting structural mismatch — DataDog's `Mapping` is a `LengthDelimited` sub-message at field 1 while sketchlib-go's `alpha` is a `SixtyFourBit` `double` at field 1, so every consumer of the SDK's DDSketch envelope (asap-precompute-{go,rs}, the agent's ddsketchprocessor decoder, the ASAPQuery backend) failed to decode the SDK's emitted bytes. This blocked all `EXPORTER_SDK_AGG=dd-{full,delta}` cells in `run_e2e_sweep.sh`, which in turn blocks paper claims (1)(4)(5) for the DDSketch row of the §accuracy table. Migration shape (mirrors KLL/HLL/CS/CMS siblings): - Imports: drop `github.com/DataDog/sketches-go/{ddsketch,pb/sketchpb}`, pick up `github.com/ProjectASAP/sketchlib-go/sketches/DDSketch`. - Series struct: hold `*ddsketch.DDSketch` (sketchlib-go) keyed off `alpha` carried through `NewDDSketch(alpha)`. - `measure`: invoke sketchlib-go's `Update(float64)` (silently drops non-positive / NaN / Inf, matching the consumer-side invariant). - Full-state serialization: `SerializePortable() + proto.Marshal`, stripping `Producer` / `HashSpec` so bytes match the asap-precompute-{go,rs} wrapper outputs and the parity golden fixtures (`integration/parity/golden_test.go`). - Delta path: replace the hand-rolled `sketchpb.DDSketch` bucket-diff with sketchlib-go's `ddsketch.ComputeDelta(prev, curr, threshold)`, caching `*DDSketch` clones (not bytes) per-series so the next-tick delta has a typed snapshot to walk directly. - Encoding signal: emit `metricdata.DDSketchEncodingProto` for full, `DDSketchEncodingProtoDelta` for delta — both already wired through the OTLP exporter encoding-value switch by PR #269. Pool reuse note: sketchlib-go's DDSketch has no `Reset()`, so `newSeries` allocates a fresh sketch on each call. The `ddSketchSeries[N]` struct header is still pooled, which is the dominant allocation cost on the hot path (the buckets backing array itself starts at 128 entries and grows lazily). Test additions: - `TestDDSketchPayloadIsSketchlibPortableEnvelope`: confirms the SDK's DDSketchDataPoint.Sketch payload now round-trips through `proto.Unmarshal` into an `envpb.SketchEnvelope` carrying a `DDSketchState` — the exact decode path the agent processor takes via `decodeDDSketchEnvelope` and asap-precompute-rs's `DDSketchWrapper::decode_envelope`. - `TestDDSketchDeltaEncodingViaComputeDelta`: confirms the cumulative-mode `deltaTransmission=true` path emits a `DDSketchEncodingProtoDelta` payload on the second tick (after a snapshot exists), and that the delta bytes feed cleanly into sketchlib-go's `ApplyDelta`. Cross-language byte-parity preserved: the existing `asap-precompute-rs::ddsketch_byte_parity_with_go` test still passes unchanged — both sides now route through sketchlib-go's `SerializePortable`, so the SDK and the asap-precompute-{go,rs} wrappers emit byte-identical envelopes. DataDog dependency cleanup: `sdk/metric/go.mod` and `deploy/fake-exporter/go.mod` no longer pull `github.com/DataDog/sketches-go` (was a direct require / indirect respectively); the larger `sdk/go.mod` still carries it as indirect (transitive from upstream OTel — left alone, out of scope). LOC: ~150 net lines source (193 add / 122 remove in ddsketch.go); +111 lines new test functions; documentation refresh in metricdata/data.go for the `DDSketchEncodingProto{,Delta}` constants. Verification: - `go test ./opentelemetry-go-patch/sdk/metric/...` green. - `go test ./opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/...` green. - `go test ./asap-precompute-go/...` green. - `cargo test --release -p asap-precompute-rs ddsketch` green (cross-language parity intact). - `go build ./deploy/fake-exporter/...` clean. - `integration/parity` golden fixtures unchanged. 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
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
asap/fake-exporter:devrebuild was broken from two independentupstream drift sources. PR #257 (E2 label-axis CPU fix) needed a
post-fix live-cell capture but couldn't get it without this rebuild.
Both blockers fixed; cold-cache
docker buildnow lands a clean imagethat the E0 stack accepts.
Blocker A — missing Go bindings under
opentelemetry-proto-patch/The patched
metrics.protoaddsDDSketch / KLLSketch / CountSketch / CountMinSketch / HLLSketchmessage types +*DataPoint+*Encodingenums on top of upstream v1.9.0, but the regenerated
.pb.gofileswere never committed. Every build hit
undefined: mpb.Metric_Ddsketch / mpb.DDSketchDataPoint / ...deep in the patched OTLP gRPC transformlayer.
Fix:
(
otel/build-protobuf:0.9.0,--go_out=plugins=grpc) over a cleanv1.9.0 checkout overlaid with the patched
.protofiles.opentelemetry-proto-patch/gen/go/go.opentelemetry.io/proto/otlp/.Updated
.gitignoreto track that subtree (it was previouslyglobbed out).
go.opentelemetry.io/proto/otlp => ../../opentelemetry-proto/gen/go/go.opentelemetry.io/proto/otlptodeploy/fake-exporter/go.mod. The same replace exists in thepatched grpc/http exporters'
go.mod, but those aren't honored froma downstream main module.
Dockerfile.fake-exportertoCOPY opentelemetry-proto/into the build context so the relative replace path resolves.
opentelemetry-proto-patch/REGEN.mddocumenting the fullregeneration recipe so future contributors don't need to reverse-
engineer the toolchain.
Blocker B — sketchlib-go API rename refactor (PR #53) absorption
The patched OTLP transform files referenced
metricdata.*EncodingGoband
mpb.*_ENCODING_GOBenum names that the prior gob→proto renamehad already obsoleted. Once the proto bindings resolved, the build
caught these.
Fix: updated KLL / CountSketch / CountMinSketch encoding-switch arms
in both
otlpmetricgrpc/internal/transform/metricdata.goandotlpmetrichttp/internal/transform/metricdata.goto dispatch on thepost-rename names. CountSketch / CountMinSketch get the additional
Deltaarm matching the sparse-delta wire path; KLL keeps a singleProtoarm (no delta defined for KLL).The actual sketchlib-go PR #53 renames (
Add/Insert→Update,InsertValue→UpdateValue,EstimateCardinality()→Estimate(),GetValueAtQuantile→Quantile,InsertWeight/InsertBatch/InsertHashes/BulkInsert→UpdateWeight/UpdateBatch/UpdateHashes/BulkUpdate) had already been absorbed inopentelemetry-go-patch/sdk/metric/internal/aggregate/{hllsketch, kllsketch,countminsketch,countsketch}.goandasap-precompute-go/sketches/hll.gobefore this round; the rebuildverifies they're correct.
Files touched
opentelemetry-proto-patch/gen/go/...— 10 new.pb.gofiles +hand-maintained
go.mod(modeled on upstream).opentelemetry-proto-patch/.gitignore— track gen subtree.opentelemetry-proto-patch/REGEN.md— new.opentelemetry-go-patch/exporters/otlp/otlpmetric/{otlpmetricgrpc, otlpmetrichttp}/internal/transform/metricdata.go— Gob → Proto/Deltaenum switches.
deploy/fake-exporter/go.mod— replace for the local proto otlp tree.deploy/docker/Dockerfile.fake-exporter— copyopentelemetry-protointo build context + updated header comment to reference the proto
restore script.
PROGRESS.md— companion-changes follow-up entry marking therebuild unblocked.
Verification
Cold-cache build:
Image runs against the live E0 stack (b3-delta + e2e-overlay, N=1):
go build ./... && go test ./...clean foropentelemetry-go/sdk/metric/andasap-precompute-go/.Test plan
docker buildsucceeds end-to-end.for ≥30s with no errors.
go build ./...clean in patchedopentelemetry-go/sdk/metric/and
asap-precompute-go/.go test ./...clean inopentelemetry-go/sdk/metric/andasap-precompute-go/.run-sdk-cost-eval.shlabel-axis sub-sweep against the rebuilt image and confirm
keep zone,rackcell drops from ~0.749 c → ~0.22 c per the benchtable in
docs/eval-label-axis-cpu-rootcause.md.