Skip to content

fix(processors): unbreak baselines - DataDog->sketchlib-go fixtures + post-#211 sink assertions - #220

Merged
zzylol merged 1 commit into
mainfrom
fix/baseline-test-debt
May 2, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/baseline-test-debt

Conversation

@zzylol

@zzylol zzylol commented May 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Restores green baselines across all 5 OTel sketch processors (ddsketch, kll, hll, countsketch, countminsketch) so refactor work can proceed without inheriting pre-existing debt.

Why

Three classes of pre-existing test failure, one per cluster of processors:

  1. DataDog/sketches-go fixture leftover (ddsketch). Production switched to sketchlib-go in fix(ddsketch): make delta_transmission actually work end-to-end #210; tests still imported DataDog/sketches-go to build sketch fixtures. Done in a prior commit (already on this branch): import swap to github.com/ProjectASAP/sketchlib-go/sketches/DDSketch and proto/sketch_envelope; setSketchPayload uses ddsketch.NewDDSketch + Update + SerializeStateProtoBytes; decodeSketch tries envelope first then bare-state fallback.

  2. Post-fix(processors): forward inputs through windowed sketch processors #211 window-mode pass-through. After PR fix(processors): forward inputs through windowed sketch processors #211, window-mode ConsumeMetrics forwards raw inputs through nextConsumer so chained windowed sketch processors (processors: [kll, hll, batch]) can see the original payload. Tests written before fix(processors): forward inputs through windowed sketch processors #211 expected sink.AllMetrics() to be empty after ConsumeMetrics; they now observe both the input pass-through AND the synthesized output emitted by flushWindow. Updated assertions to count N inputs + 1 flush = N+1 entries and scan-by-name where ordering is non-deterministic (concurrent test).

  3. selfmonitor import + go.mod replace missing (kll, countsketch, countminsketch). These processors import go.opentelemetry.io/collector/processor/selfmonitor, a package that lives in the local fork at opentelemetry-collector/processor/selfmonitor but is not in any upstream tag. The corresponding go.mod was missing the replace go.opentelemetry.io/collector/processor => ../../../opentelemetry-collector/processor directive that ddsketchprocessor already had. Added the replace; go mod tidy resolves cleanly afterward.

A bonus issue surfaced for kll/countminsketch: production emits sketch payloads via proto envelopes (SerializePortable + proto.Marshal for KLL, SerializeProtoBytesFO for CMS), but two tests called Deserialize<sketch>FromBytes (gob-encoded) — which mismatches the wire format and panics with gob: duplicate type received. Switched the tests to Deserialize<sketch>FromProtoBytes to match the emit path.

Scope

Per the user-supplied plan, this PR does NOT touch:

Submodule pointer drift on opentelemetry-collector and opentelemetry-go was left unstaged (pre-existing noise).

Per-processor results

Processor go test go build gofmt (modified files)
ddsketchprocessor pass (no fail) pass clean
kllprocessor pass (no fail) pass clean
hllprocessor pass (no fail) pass clean
countsketchprocessor pass (no fail) pass clean
countminsketchprocessor pass (no fail) pass clean

(gofmt issues remain in files I did not touch — pre-existing debt, out of scope.)

Full collector build via ./build_sketchcollector.sh succeeds end-to-end: builder produces cmd/sketchcollector/sketchcollector binary.

Files modified per processor

  • ddsketchprocessor: processor_test.go (5 window-mode tests + concurrent test → expect input pass-through + flush; index synthesized output by name where ordering is non-deterministic), go.mod / go.sum (DataDog dep removed in prior commit on this branch).
  • kllprocessor: processor_test.go (5 window-mode tests fixed; gob→proto deserializer in TestBatchModeTransmitSketch), go.mod (added replace go.opentelemetry.io/collector/processor).
  • hllprocessor: processor_test.go (2 window-mode tests fixed), go.mod / go.sum (no replace needed; go mod tidy rebalanced direct vs. indirect).
  • countsketchprocessor: go.mod / go.sum only — added replace go.opentelemetry.io/collector/processor. Tests already accounted for pass-through.
  • countminsketchprocessor: processor_test.go (gob→proto deserializer in TestProcessor_TumblingWindow_Correctness), go.mod (added the replace).

Test plan

  • cd <each-processor> then go mod tidy && go build ./... && go test ./... -count=1 — all 5 pass.
  • ./build_sketchcollector.sh — succeeds.
  • Per code-review: assertion changes truthfully reflect the post-fix(processors): forward inputs through windowed sketch processors #211 pass-through contract; no test was weakened (all original semantic checks preserved, just relocated to scan the synthesized entry instead of out[0]).

…post-#211 sink assertions

Why:
- DataDog/sketches-go is no longer a runtime dep (production
  switched to sketchlib-go in #210). Tests that still imported
  DataDog/sketches-go to build sketch fixtures were leftover.
- After #211, window-mode ConsumeMetrics forwards raw inputs
  through nextConsumer so chained windowed sketch processors can
  see the original payload. Tests written before #211 expected
  sink.AllMetrics() to be empty after ConsumeMetrics; they now
  observe both the input pass-through AND the synthesized output
  emitted by flushWindow. Update assertions to count both and
  scan-by-name where ordering is non-deterministic.
- KLL/CountSketch/CountMinSketch processors imported
  go.opentelemetry.io/collector/processor/selfmonitor (the local
  fork's selfmonitor package) but the corresponding go.mod was
  missing the `replace` directive that points
  go.opentelemetry.io/collector/processor at the local fork. Add
  the replace; drop the now-unused gob deserializer in two test
  sites where the test was reading proto-encoded emit bytes.

Per-processor:
- ddsketchprocessor: fix 5 window-mode tests + concurrent test;
  treat sink as [forwarded inputs..., synthesized flush output].
- kllprocessor: add `replace go.opentelemetry.io/collector/processor`
  to go.mod, fix 5 window-mode test assertions, swap
  DeserializeKLLSketchFromBytes (gob) → DeserializeKLLSketchFromProtoBytes
  (proto) in TestBatchModeTransmitSketch to match emit format.
- hllprocessor: fix 2 window-mode test assertions (selfmonitor
  was already resolvable).
- countsketchprocessor: add the `replace` directive only — tests
  already accounted for pass-through.
- countminsketchprocessor: add the `replace` directive, swap
  DeserializeCountMinSketchFromBytes (gob) →
  DeserializeCountMinSketchFromProtoBytes (proto) to match
  SerializeProtoBytesFO emit path.

All 5 packages pass `go test ./... -count=1` and `go build ./...`.
Full sketchcollector build via build_sketchcollector.sh succeeds.
gofmt is clean for all files modified by this change; pre-existing
gofmt debt in unrelated files left untouched.

Submodule pointer drift on opentelemetry-collector and
opentelemetry-go was left unstaged (pre-existing noise).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 3b05c93 into main May 2, 2026
@zzylol
zzylol deleted the fix/baseline-test-debt branch May 2, 2026 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant