CountSketch: OTLP export and sketch serialization - #77
Merged
Merged
Conversation
This was referenced Mar 24, 2026
5 tasks
zzylol
requested changes
Mar 24, 2026
zzylol
approved these changes
Mar 25, 2026
SieDeta
pushed a commit
that referenced
this pull request
Apr 17, 2026
* Update CountSketch processor sketch serialization * Restore CountSketch non-serialization logic * rename pathway references
3 tasks
zzylol
added a commit
that referenced
this pull request
Jul 17, 2026
Wires the GOS-aware sampled primitives (sketchlib-go #77: UpdateStringSampledPerRowGOS/UpdateStringAtRowsGOS for CountSketch, InsertWithHashSampledPerRowGOS/InsertWithHashAtRowsGOS for CMS) into the ASAPCollector wrapper layer, closing the gap #518's sampling stack would otherwise reopen: before this, a sampled insert returned before ever reaching the GOS threshold check, so a CountSketch/CMS with both sample_p<1 (or SDK row-admission) AND gos_delta_epsilon>0 silently lost insert-time detection for every sampled sample — no crash, estimates stayed unbiased, but the GOS mechanism itself never fired for that sketch. Scope: only the collector-side wrapper composition (this PR). The SDK-side machinery that DECIDES admittedRows/sampleP and gets them onto the wire (aggregationrouter.go, policyfingerprint.go, monitor engine changes, OTel Go SDK, asap-precompute-rs) is #518's own, larger, separate concern — out of scope here, reattaches independently once it needs these primitives. - observation.go: RowSampled/AdmittedRows/SampleP fields on ObservationValue (lifted from #518, unmodified) — the wire the SDK's row-admission decision travels on into the collector. - countsketch.go: UpdateString's sampler branch now calls UpdateStringSampledPerRowGOS when GOS is active (upgrading from the old whole-item Admit()+divide model to per-row NitroSketch admission, per #518's intent, while preserving GOS composition); heap-msgpack mode is still excluded (its DELTA-HEAP wire form isn't GOS-converted) via the same gosActive condition ComputeDeltaAgainst already checks. New ApplyAdmittedOccurrence method + CountSketchObserver wiring for the SDK-pre-decided row-admission path. - cms.go: CMS's whole-item WithSampleP/admit() sampling already composed correctly with GOS (InsertWithHashGOS calls admit() internally) — no change needed there. New ApplyAdmittedOccurrence method (CMS had none before) + CMSObserver wiring for the SDK-pre-decided path, mirroring CountSketch's. Test plan: - TestCountSketch_RowSampledObserve_ComposesWithGOS / TestCMS_RowSampledObserve_ComposesWithGOS: drive Observe with RowSampled=true through both wrappers, assert the wake signal fires and the drained delta is non-empty — proving the SDK-row-admission path participates in GOS instead of silently bypassing it. - TestCountSketch_PerRowSampler_ComposesWithGOS: a live WithSampleP geometric sampler still triggers GOS detection through UpdateString directly (the other reattached path). - go build/go vet/go test clean in asap-precompute-go (full suite, no regressions) and the processor package (post restore_all.sh). Co-Authored-By: Claude Sonnet 5 <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.
Changes
Code changes
Implementation pattern used in the processor
Go module file changes
OTLP exporter compatibility settings
Compression notes
OTLP message shape
Common wire format (both flows):
gRPC transport notes
Shared top-level structure:
Raw Metrics Flow message shape
In this flow, data points carry numeric values directly (gauge or sum).
How ASAPQuery treats it:
Sketch Payload Flow message shape (CountSketch)
In this flow, processor attaches serialized sketch bytes into OTLP attributes.
Sketch bytes are produced as:
Expected sketch payload attribute key in this PR:
How ASAPQuery treats it:
How to test these changes
1) Start ASAPQuery OTLP receiver
From ASAPQuery/:
cd ASAPQuery RUST_LOG=query_engine_rust=debug ./target/release/query_engine_rust \ --enable-otel-ingest \ --otel-grpc-port 4317 \ --otel-http-port 4318 \ --kafka-topic dummy \ --input-format json \ --config asap-query-engine/examples/promql/inference_config.yaml \ --streaming-config asap-query-engine/examples/promql/streaming_config.yaml \ --streaming-engine arroyo \ --prometheus-scrape-interval 15 \ --output-dir /tmp/asapquery \ --query-language promql \ --lock-strategy globalExpected startup logs include OTLP listeners on :4317 (gRPC) and :4318 (HTTP).
2) Run CountSketch processor config
From DataCollector/opentelemetry-collector-contrib-patch/:
Raw Metrics Flow:
Sketch Payload Flow (batch):
3) Run load generator
From DataCollector/otel_collector_benchmark/:
cd ProjectASAP/DataCollector/otel_collector_benchmark go run main.go --endpoint localhost:53217 --workers 2 --hosts 5 --metrics 10 --duration 15sNotes: