feat(asapedgeprocessor): accept per-metric sample_p in fused asap_edge + sample HLL/CMS - #444
Merged
Merged
Conversation
…e + sample HLL/CMS The fused asap_edge processor's per-metric config struct did not define sample_p, so the collector's strict mapstructure decode rejected the key the control plane emits when a metric's workload sample_p < 1, crash-looping the agent. Add SampleP to MetricFamily (validated to (0,1], 0/unset => 1.0 = no sampling) and wire it into the warm sketch-build: HLL and CountMinSketch factories call sketchlib-go's WithSampleP(p). WithSampleP(1.0) is an exact no-op, so the unset/1.0 path stays byte-identical to today. DDSketch/KLL/ CountSketch (no sampling support) ignore it. Mirrors the standalone hll/countminsketch processors' sample_p wiring (#441) for the fused path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Aug 27, 2026
…redesign ASAPPlanner's post-ASAP IR (crates/types/src/post_asap) selects a candidate logical DAG (SummaryAgg/summary family+algorithm+params/ Reduction/SummaryEstimate) and, per its own README "Scope" and asap-aware-mapping/README.md "Non-Goals", explicitly does not choose collector/backend placement, transport mode, or physical resources. Note in Purpose that the ASAPQuery-backend control plane compiles that selection - it does not serialize it directly - into this document's collector configuration and a companion backend configuration sharing one plan identity, and link ASAPQuery-backend PR #444's new design-compiled-plan-collector-backend-split.md for that compile step. Point the existing "Current contract gap" section (no plan_id/version/ activation/expiry/backend-compat on this wire) at that same design as the proposed closing mechanism, without claiming any of it is implemented here yet - the OpAMP envelope encoding for those fields is still an open question in that design, to be resolved and documented in this file once it lands. 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.
Summary
sample_pinto the FUSEDasap_edgeprocessor's config (set when a metric's workloadsample_p < 1), but the fused per-metric config struct didn't define the key, so the collector's strict mapstructure decode rejected it (metrics[N] has invalid keys: sample_p) and crashed. A prior PR (feat(sketches): per-metric sample_p knob on the CMS + HLL warm processors #441) addedSamplePonly to the NON-fusedhll/countminsketchprocessors; the deploy runsASAP_EDGE_FUSED=1.SampleP float64 \mapstructure:"sample_p"`toMetricFamily(the fused per-metric struct) inconfig.go, validated to(0,1]with0/unset normalised to1.0` (sampling disabled).sketch.gonewSketchAggregator): the HLL and CountMinSketch factories call sketchlib-go'sWithSampleP(p).WithSampleP(1.0)is an exact no-op, so the unset/1.0path stays byte-identical to today. DDSketch/KLL/CountSketch (no sampling support) ignore it. Mirrors the standalone processors' wiring from feat(sketches): per-metric sample_p knob on the CMS + HLL warm processors #441.sample_poff the SketchEnvelope and rescales by1/pat query, so the agent just needs to accept the key and build the sketch with the probability.Files / functions touched (for merge-coordination with the cold-density agent)
Scoped to the warm path only — the cold/gorilla feed path is untouched:
config.go—MetricFamilystruct (+SamplePfield);Config.Validate()(+(0,1]normalise/validate loop).sketch.go—newSketchAggregator()(applyWithSamplePto HLL/CMS factories; retainfactoryonsketchAggregatorfor test observability).sample_p_path_test.go— new test file.go.mod/go.sum— addconfmap(test-only, for the strict-decode regression).Test plan
go build ./...,go vet ./...,gofmt -lcleansample_p_path_test.go:TestFusedConfigSamplePDecodes— a fused config withsample_pon HLL/CMS metrics decodes through the real strictconfmappath WITHOUT the crash; values populate; unset normalises to1.0.TestFusedSketchBuildAppliesSampleP— the built HLL/CMS sketch hasSampleP()==0.5withsample_p: 0.5, and1.0(no sampling) when unset; DDSketch ignores the knob.TestFusedConfigSamplePRejectsOutOfRange— out-of-rangesample_prejected at Validate.processor_test.gopdata-skew in this checkout)🤖 Generated with Claude Code