refactor(ddsketchprocessor): thin shim delegating to asap-precompute-go (Phase 2 step 2.5) - #227
Merged
Merged
Conversation
DDSketch processor reduces from ~942 LoC to a ~178 LoC shim. The windowing, snapshot caching, and delta encoding state machine moves to asap-precompute-go (already extracted). DDSketch sketch_wrapper.go implements QuantileSketch over sketchlib-go. Public test API: Shim.ProcessBatch/ProcessMetrics/FlushWindow per ADR-0002 §"Test API contract". Parity harness: TestParity_DDSketch byte-identical pre/post refactor. Phase 2 step 2.5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
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
DDSketch processor reduces from ~942 LoC to a ~178 LoC shim
(
processor.go) that delegates the windowing, snapshot caching, anddelta encoding state machine to the
asap-precompute-goruntimeextracted in PR #219 (Phase 2 steps 2.1–2.4). The shim's responsibility
shrinks to what it actually owns:
Start/Shutdown/ConsumeMetrics) andthe window-mode tick goroutine.
Config→precompute.PrecomputeConfig.TransmitSketch=false(decode theruntime's envelope back to a DDSketch and emit Gauge per-quantile
data points). The Phase-2.5 spec keeps this on the shim because
otel.Encodedoes not reconstruct sketches from payload bytes.Auxiliary helpers (sketch wrapper, mergeAppend, metadata propagation,
config translation, self-monitor wiring) live in sibling files in the
processor package so
processor.gostays focused on lifecycle +pipeline.
LoC delta
processor.gosketch_wrapper.goshim_helpers.goconfig_translate.gomonitor.goprocessor.goitself is the lifecycle + Test API surface; theremaining files are reusable adapter glue. Pre-refactor
processor.gowas 942 LoC and held all four concerns (binding, decode, runtime,
encode) in one file.
Parity test result
TestParity_DDSketchis byte-identical pre/post refactor:All five parity subtests still pass at HEAD:
go test -race ./...passes for the processor package(
TestShutdownDuringConsume,TestWindowModeFlushDuringConsume,TestWindowModeConcurrentConsumeall clean).Public API surface confirmation
ADR-0002 §"Test API contract" lists three test-friendly public methods
that today's tests directly exercise. The shim ships exactly these:
Shim.ProcessBatch(ctx, md) (pmetric.Metrics, error)— synchronousdecode → observe → tick → encode → append-into-md → return md (the
legacy
processBatchshape).Shim.ProcessMetrics(ctx, md) error— observe-only; window-modeConsumeMetrics calls this; tests use it directly.
Shim.FlushWindow(ctx context.Context) error— force-tick + encode +forward via
nextConsumer. Concurrency-safe: swaps the per-metricPrecompute map under
p.muso the flush loop owns the snapshotexclusively.
processor_test.gomechanical rename:processBatch→ProcessBatch,flushWindow→FlushWindow. No test logic changed.Divergence from the execution plan
Two adapter-side accommodations were necessary because the
otel.Decode/
otel.Encodehelpers inasap-precompute-go(frozen for this PR)don't propagate certain typed pmetric fields:
Count/AggregationTemporalitypropagation.SketchEnvelope.Countand.AggregationTemporalityare typed Gofields, not on the proto wire, so
otel.Decodedoesn't read themfrom input DDSketch DPs and
otel.Encodedoesn't write them ontooutput DPs. Two helpers in
shim_helpers.go(
patchEnvelopeMetadata,stampDPMetadata) thread these typedfields through the runtime. A future PR can fold this into
otel.Decode/Encodeonce all five shims need the same plumbing.Per-input-metric Precompute. The legacy DDSketch processor
emits one synthesized output metric per input metric name; the
runtime models one Precompute per
(sketch type, agg_id). Theshim runs one Precompute per distinct input metric name, keyed on
getOrCreate(batch, observation.Metric, cfg). This keeps existingmulti-metric tests (
TestBatchModeDualInput,TestWindowModeDualInput) working without touching the runtime.Both accommodations are isolated to the shim and don't leak into the
runtime contract.
Test plan
go test ./opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/...clean (no -race issues)go test -v -run TestParity_DDSketch ./integration/parity/...byte-identical 6 envelopesgo test -v ./integration/parity/...all 5 sketch subtests still byte-identicalgo vet ./opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/...cleanasap-precompute-gosource (runtime frozen for this PR)🤖 Generated with Claude Code