Add batch/window modes, support sketch/raw SDK collection and benchmarking - #32
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds two output-timing modes to ddsketchprocessor (per-batch and tumbling-window) so it can aggregate either Gauge samples or DDSketch inputs and then emit either merged DDSketch payloads or quantile gauges. It also wires these new modes into the benchmark harness and updates documentation with configuration guidance and representative benchmark results.
Changes:
- Implement
mode: batch(flush on each incoming batch) andmode: window(periodic tumbling-window flush) inddsketchprocessor. - Extend tests to cover Gauge input in both modes and DDSketch input merging across multiple batches in window mode.
- Update benchmark script/configs/docs to run
ddsketchcol-batch/ddsketchcol-windowscenarios and perform a basic Prometheus-scrape quantile correctness check.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| otel_collector_benchmark/README.md | Documents DDSketch benchmark modes, correctness check, and sample window-mode results. |
| opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/processor.go | Adds window-mode state, periodic flushing, and mode-aware ConsumeMetrics behavior. |
| opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/factory.go | Switches from processorhelper.NewMetrics to returning the processor implementation directly. |
| opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/config.go | Introduces mode/window_duration configuration with validation and defaults. |
| opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/config_test.go | Adds mode-specific validation test cases. |
| opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/processor_test.go | Adds tests for Gauge input in batch/window modes and DDSketch merging across batches in window mode. |
| opentelemetry-collector-contrib-patch/processor/ddsketchprocessor/README.md | Updates processor documentation for new modes/config, but currently contains conflicting legacy docs. |
| opentelemetry-collector-contrib-patch/cmd/otelcontribcol/go.mod | Adds generated builder module with a local replace for the patched ddsketchprocessor. |
| opentelemetry-collector-contrib-patch/cmd/ddsketchcol/config.yaml | Sets ddsketch processor mode explicitly to batch. |
| opentelemetry-collector-contrib-patch/cmd/ddsketchcol/config-window.yaml | Adds a new collector config for window mode, exporting Prometheus on 8889. |
| opentelemetry-collector-contrib-patch/cmd/bench.sh | Adds ddsketch batch/window targets, Prometheus correctness check, and output/input ratio reporting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 34 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add batch/window modes, support sketch/raw SDK collection and benchmarking
feat(multinode): wire gorilla-merger into the ASAP arms (#32)
DDSketch processor
mode: batch(per-batch flush) andmode: window(tumbling window) toddsketchprocessor.AggregationTemporalityfor DDSketch inputs, matching batch-mode semantics.Start/Shutdownso window-mode shutdown does not block when the window goroutine was never started.ddsketchprocessor/README.mdto treat input type (sketch vs. raw) and output mode (batch vs. window) as orthogonal dimensions.KLL processor
mode: batchandmode: windowover Gauge inputs, with batch mode appending quantile metrics and window mode emitting per-window quantiles only.bench.shthat:p50 <= p90 <= p99), and bounds within the expected Zipf range.otel_collector_benchmark/README.mdalready includes full KLL batch and window tables (10k–50k MPS) plus comparative analysis rows; this PR keeps that coverage in sync with the code.CountSketch processor
mode: batchfor per-batch CountSketch snapshots andmode: windowfor ticker-driven, per-window snapshots.drop_original; window mode is tuned for high storage reduction with summaries only.cmd/bench.shthat:8889.countsketch_rowandcountsketch_colmetadata metrics are present; missing metrics cause the scenario to be flagged as a failure.DropOriginalbehavior.otel_collector_benchmark/README.mdto:Benchmark harness (
cmd/bench.shandotel_collector_benchmark/README.md)ddsketchcol-batch,ddsketchcol-windowkll-batch,kll-window, legacykllcountsketchcol-batch,countsketchcol-windowotel_collector_benchmark/benchmark_results/{processor}/.8889._p50,_p90,_p99existence, monotonicity, and bounds check via the same exporter.countsketch_rowandcountsketch_colmetadata metrics after each run.8888and Prometheus port8889before each scenario to avoid stale metrics influencing correctness checks.otel_collector_benchmark/README.mdto: