fix(emit): inject cumulativetodelta upstream of agent routing for Counter metrics (closes #298) - #299
Merged
Conversation
…nter metrics (closes #298) The fake-exporter's `Float64Counter` instruments default to OTel-SDK **cumulative** temporality — each export carries the running lifetime value, not the per-export delta. The asap-otel agent's 5-sketch routing pipeline previously had no `cumulativetodelta` processor, so the backend's `SumAccumulator::update` (sum-of-deltas) was fed cumulatives and computed `Σ-of-cumulatives-in-window` — a quadratic-in-time blowup. The replay path's outer sum across the lookback then made it cubic, so `sum by (zone) (http_requests_total)` on the asap tier returned ~300× the b0 baseline. Fix: - New `EdgeStageConfig::cumulative_counter_metrics: Vec<String>` field, populated from a new `collect_cumulative_counter_metrics()` helper that walks the WorkloadStore picking up every metric whose workload query classifies as `AggRole::Sum` (bare-selector / sum / rate / increase / sum_over_time / irate). - `emit_edge_yaml_5sketch_routing` declares a `cumulativetodelta` processor with `include.metrics = [...]` (`match_type: strict`, sort-stable for byte-stable YAML) when the list is non-empty, and inserts it as the FIRST processor in the entry `metrics:` pipeline so every routed copy of each listed metric reaches the routing connector with delta temporality. - Bootstrap (`main::emit_bootstrap_typed`) and OpAMP replan (`replan::Replanner::try_emit_typed_edge_yaml_for_workload`) both wire the new field via the new helper, mirroring the existing `metric_to_family` / `metric_to_grouping_labels` stitches. - Strict matching keeps the processor a no-op for any unrelated metric (quantile gauges like `http_requests_total_latency_ms` pass through unchanged). 6 new tests pin: * presence/absence of the processor by list non-emptiness * processor runs FIRST on the entry pipeline (before routing connector fan-out) * include-metrics list is sorted (byte-stable YAML across runs) * helper classifies Sum-role entries correctly, dedupes, and skips quantile/cardinality entries Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 19, 2026
zzylol
added a commit
that referenced
this pull request
May 20, 2026
…oses #301, #300) (#303) Post-#299 the agent streams per-window DELTAS for counters, but the asap engine collapsed `sum` / `sum_over_time` / `increase` / `rate` — four semantically-distinct PromQL counter idioms — onto the same two reducer paths, so all instant counter sums returned the same wrong number and `rate` was systematically under-reported. Four layered bugs: Layer 1 (engine dispatch): only `rate` vs everything-else branched; sum / sum_over_time / increase / instant-sum all hit one path. Layer 2 (lookback): instant sum used a 5-min default == the [5m] range, so on a <5min producer every idiom captured the same horizon. Layer 3 (instant projection): the instant branch took `samples.last()` — the MOST RECENT window's delta — instead of the cumulative-since-storage value PromQL `sum(counter)` requires. Layer 4 (rate divisor): `evaluate_exact_agg_rate` divided by the NOMINAL range (300 for [5m]) regardless of how much data actually covered the window, halving the rate when the producer ran < range. Fix: - control_plane: extend `OuterFn` from {Plain, Rate} to the full counter-fn taxonomy {Plain, Rate, Increase, SumOverTime}, populated by the analyzer's `trace_from_promql` walker (inner-counter-idiom wins for composed shapes like `sum by (..) (rate(..))`; precedence enforced by a new `set_counter_fn` helper). Carried on `ASAPTierCandidate.outer_fn`. - engine dispatch (instant + range surfaces): branch on the typed counter-fn — `Rate` → rate reducer; `Increase` → accumulate windows over the [t-r,t] clip into one cumulative number; `Plain` instant sum → accumulate ALL windows over the full storage horizon (t0=0) → cumulative-since-start; `SumOverTime` over a counter → capability-miss → archive (asap stores deltas and cannot reconstruct the Σ-of-cumulative-samples sum_over_time wants — issue #301 decision (a), subsumes #300). - reducer: `evaluate_exact_agg` gains an `accumulate_windows` flag that collapses each group's per-window deltas into ONE cumulative sample (Layer 3); the matrix/range surface keeps `accumulate_windows=false`. `evaluate_exact_agg_rate` now divides by `min(range_seconds, actual_coverage_span_seconds)` via a new `SketchStore::exact_agg_coverage_bounds(sid, t0, t1)` that reports the true `(min_window_start, max_window_end)` span (Layer 4). Tests: control_plane 783, data_plane 755 green. New unit + integration coverage pins each counter-fn semantic (instant-sum accumulates all windows not last; increase accumulates without divisor; sum_over_time capability-misses; rate divisor uses actual coverage). Multinode validation (4 zones, 10000 series @ 100 Hz; asap node2:9091 vs VictoriaMetrics baseline node2:8428), steady state: rate : asap 997,413 b0 999,987 rel-err 0.26% (was 64%) topk(rate): asap 997,413 (tracks rate; unchanged semantic) sum_over_time: asap capability-miss → archive (no longer fabricates) The four idioms now return distinct values (sum != increase != rate), confirming the dispatch no longer collapses them. Working queries (`quantile_over_time(0.99, ...)`, `max by (zone) (quantile_over_time)`) unaffected. Instant `sum` is cumulative-since-storage (runtime-dependent; residual gap is producer-runtime / flush-lag, not a dispatch bug). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 20, 2026
…oses #301, #300) Post-#299 the agent streams per-window DELTAS for counters, but the asap engine collapsed `sum` / `sum_over_time` / `increase` / `rate` — four semantically-distinct PromQL counter idioms — onto the same two reducer paths, so all instant counter sums returned the same wrong number and `rate` was systematically under-reported. Four layered bugs: Layer 1 (engine dispatch): only `rate` vs everything-else branched; sum / sum_over_time / increase / instant-sum all hit one path. Layer 2 (lookback): instant sum used a 5-min default == the [5m] range, so on a <5min producer every idiom captured the same horizon. Layer 3 (instant projection): the instant branch took `samples.last()` — the MOST RECENT window's delta — instead of the cumulative-since-storage value PromQL `sum(counter)` requires. Layer 4 (rate divisor): `evaluate_exact_agg_rate` divided by the NOMINAL range (300 for [5m]) regardless of how much data actually covered the window, halving the rate when the producer ran < range. Fix: - control_plane: extend `OuterFn` from {Plain, Rate} to the full counter-fn taxonomy {Plain, Rate, Increase, SumOverTime}, populated by the analyzer's `trace_from_promql` walker (inner-counter-idiom wins for composed shapes like `sum by (..) (rate(..))`; precedence enforced by a new `set_counter_fn` helper). Carried on `ASAPTierCandidate.outer_fn`. - engine dispatch (instant + range surfaces): branch on the typed counter-fn — `Rate` → rate reducer; `Increase` → accumulate windows over the [t-r,t] clip into one cumulative number; `Plain` instant sum → accumulate ALL windows over the full storage horizon (t0=0) → cumulative-since-start; `SumOverTime` over a counter → capability-miss → archive (asap stores deltas and cannot reconstruct the Σ-of-cumulative-samples sum_over_time wants — issue #301 decision (a), subsumes #300). - reducer: `evaluate_exact_agg` gains an `accumulate_windows` flag that collapses each group's per-window deltas into ONE cumulative sample (Layer 3); the matrix/range surface keeps `accumulate_windows=false`. `evaluate_exact_agg_rate` now divides by `min(range_seconds, actual_coverage_span_seconds)` via a new `SketchStore::exact_agg_coverage_bounds(sid, t0, t1)` that reports the true `(min_window_start, max_window_end)` span (Layer 4). Tests: control_plane 783, data_plane 755 green. New unit + integration coverage pins each counter-fn semantic (instant-sum accumulates all windows not last; increase accumulates without divisor; sum_over_time capability-misses; rate divisor uses actual coverage). Multinode validation (4 zones, 10000 series @ 100 Hz; asap node2:9091 vs VictoriaMetrics baseline node2:8428), steady state: rate : asap 997,413 b0 999,987 rel-err 0.26% (was 64%) topk(rate): asap 997,413 (tracks rate; unchanged semantic) sum_over_time: asap capability-miss → archive (no longer fabricates) The four idioms now return distinct values (sum != increase != rate), confirming the dispatch no longer collapses them. Working queries (`quantile_over_time(0.99, ...)`, `max by (zone) (quantile_over_time)`) unaffected. Instant `sum` is cumulative-since-storage (runtime-dependent; residual gap is producer-runtime / flush-lag, not a dispatch bug). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 20, 2026
…oses #301, #300) (#304) * fix(query): honor counter-function semantics in ExactAgg dispatch (closes #301, #300) Post-#299 the agent streams per-window DELTAS for counters, but the asap engine collapsed `sum` / `sum_over_time` / `increase` / `rate` — four semantically-distinct PromQL counter idioms — onto the same two reducer paths, so all instant counter sums returned the same wrong number and `rate` was systematically under-reported. Four layered bugs: Layer 1 (engine dispatch): only `rate` vs everything-else branched; sum / sum_over_time / increase / instant-sum all hit one path. Layer 2 (lookback): instant sum used a 5-min default == the [5m] range, so on a <5min producer every idiom captured the same horizon. Layer 3 (instant projection): the instant branch took `samples.last()` — the MOST RECENT window's delta — instead of the cumulative-since-storage value PromQL `sum(counter)` requires. Layer 4 (rate divisor): `evaluate_exact_agg_rate` divided by the NOMINAL range (300 for [5m]) regardless of how much data actually covered the window, halving the rate when the producer ran < range. Fix: - control_plane: extend `OuterFn` from {Plain, Rate} to the full counter-fn taxonomy {Plain, Rate, Increase, SumOverTime}, populated by the analyzer's `trace_from_promql` walker (inner-counter-idiom wins for composed shapes like `sum by (..) (rate(..))`; precedence enforced by a new `set_counter_fn` helper). Carried on `ASAPTierCandidate.outer_fn`. - engine dispatch (instant + range surfaces): branch on the typed counter-fn — `Rate` → rate reducer; `Increase` → accumulate windows over the [t-r,t] clip into one cumulative number; `Plain` instant sum → accumulate ALL windows over the full storage horizon (t0=0) → cumulative-since-start; `SumOverTime` over a counter → capability-miss → archive (asap stores deltas and cannot reconstruct the Σ-of-cumulative-samples sum_over_time wants — issue #301 decision (a), subsumes #300). - reducer: `evaluate_exact_agg` gains an `accumulate_windows` flag that collapses each group's per-window deltas into ONE cumulative sample (Layer 3); the matrix/range surface keeps `accumulate_windows=false`. `evaluate_exact_agg_rate` now divides by `min(range_seconds, actual_coverage_span_seconds)` via a new `SketchStore::exact_agg_coverage_bounds(sid, t0, t1)` that reports the true `(min_window_start, max_window_end)` span (Layer 4). Tests: control_plane 783, data_plane 755 green. New unit + integration coverage pins each counter-fn semantic (instant-sum accumulates all windows not last; increase accumulates without divisor; sum_over_time capability-misses; rate divisor uses actual coverage). Multinode validation (4 zones, 10000 series @ 100 Hz; asap node2:9091 vs VictoriaMetrics baseline node2:8428), steady state: rate : asap 997,413 b0 999,987 rel-err 0.26% (was 64%) topk(rate): asap 997,413 (tracks rate; unchanged semantic) sum_over_time: asap capability-miss → archive (no longer fabricates) The four idioms now return distinct values (sum != increase != rate), confirming the dispatch no longer collapses them. Working queries (`quantile_over_time(0.99, ...)`, `max by (zone) (quantile_over_time)`) unaffected. Instant `sum` is cumulative-since-storage (runtime-dependent; residual gap is producer-runtime / flush-lag, not a dispatch bug). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: remove obsolete SQL query-string test + doc refs (post-#302 PromQL-only) #302 removed the SQL/ElasticDSL front-ends but left `query_string_sql_populates_workload` (asserts `SELECT ... GROUP BY` parses into a workload) plus two "PromQL or SQL" doc comments in pipeline.rs. With SQL parsing gone the test fails. Removed the stale test and de-SQL'd the QuerySpec docs. Surfaced when rebasing the #301 counter-fn fix onto post-#302 main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Aug 27, 2026
…ntity ASAPPlanner's own scope statement (README "Scope"; asap-aware-mapping/README.md "Non-Goals") is explicit that it does not choose collector/backend placement, transport mode, or physical resources - confirmed against its current crates/types/src/post_asap module (SummaryAgg/SummaryFamilyType/GroupingStrategy/Reduction/SummaryEstimate), not older docs. Add design-compiled-plan-collector-backend-split.md: the ASAPQuery control plane compiles a selected post-ASAP DAG into a CompiledPlan carrying a CollectorSubplan (asap_edge YAML via OpAMP) and a BackendSubplan (BackendPlan), sharing one plan_id/plan_version/ activation/expiry/backend_compat identity - closing the exact gap ASAPCollector PR #558 documents (no plan_id/version/activation/expiry/ backend-compat on the OpAMP wire today) - rather than serializing the selected DAG directly into collector YAML. Update the migration doc (PR #444) to reference the compile step wherever it previously said "collector/backend stage allocation" or "collector configuration generation", fix the target-architecture diagram to show both subplans instead of only BackendPlan, add the legacy physical::plan::PlanNode/PipelineStage allocator to the post-cutover removal list, broaden PR 5 from a BackendPlan-only conversion to the full two-subplan compile, and note open ASAPPlanner PRs #300 (explicit update/readout phase boundary - the same boundary this split already uses structurally) and #299 (accuracy propagation) as tracked, non-blocking upstream changes for PR 6's coverage list. Correct design-backend-plan-wire-format.md's BackendPlan::plan_id comment ("observability only, not identity") to reflect its new role as the cross-subplan join key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Aug 28, 2026
* docs: plan ASAPPlanner workload migration * docs: design recurring rule workload planning * docs: reuse existing ASAPPlanner workload types * docs: isolate Prometheus query and rule adapters * docs: clarify planner backend and adapter ownership * docs: keep IDs and plan commitment downstream * docs: compile selected DAG into two physical subplans with shared identity ASAPPlanner's own scope statement (README "Scope"; asap-aware-mapping/README.md "Non-Goals") is explicit that it does not choose collector/backend placement, transport mode, or physical resources - confirmed against its current crates/types/src/post_asap module (SummaryAgg/SummaryFamilyType/GroupingStrategy/Reduction/SummaryEstimate), not older docs. Add design-compiled-plan-collector-backend-split.md: the ASAPQuery control plane compiles a selected post-ASAP DAG into a CompiledPlan carrying a CollectorSubplan (asap_edge YAML via OpAMP) and a BackendSubplan (BackendPlan), sharing one plan_id/plan_version/ activation/expiry/backend_compat identity - closing the exact gap ASAPCollector PR #558 documents (no plan_id/version/activation/expiry/ backend-compat on the OpAMP wire today) - rather than serializing the selected DAG directly into collector YAML. Update the migration doc (PR #444) to reference the compile step wherever it previously said "collector/backend stage allocation" or "collector configuration generation", fix the target-architecture diagram to show both subplans instead of only BackendPlan, add the legacy physical::plan::PlanNode/PipelineStage allocator to the post-cutover removal list, broaden PR 5 from a BackendPlan-only conversion to the full two-subplan compile, and note open ASAPPlanner PRs #300 (explicit update/readout phase boundary - the same boundary this split already uses structurally) and #299 (accuracy propagation) as tracked, non-blocking upstream changes for PR 6's coverage list. Correct design-backend-plan-wire-format.md's BackendPlan::plan_id comment ("observability only, not identity") to reflect its new role as the cross-subplan join key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: redesign asap_edge.metrics[] to name post_asap directly Checked against the real processor (opentelemetry-collector-contrib-patch/processor/asapedgeprocessor/ config.go), not the doc summary of it: today's MetricFamily.Mode is a bare two-value string (per_series/whole_stream, ParseAggMode-validated) and AggregateBy is a plain []string with no way to express GroupKeys.without - neither can distinguish Reduction::PerEntity from a genuine zero-key Reduce, the exact ambiguity Reduction was introduced to remove. Family is a flat string with no exact_kind discriminator and no GroupingStrategy field at all. Redesign MetricFamily to name every field and enum value directly from post_asap - Source/Family/ExactKind/ReduceBy/ReduceWithout/PerEntity/ Grouping/HydraKind/SharedRows/SharedColumns - while keeping Go's flat mapstructure-struct idiom (matching this same file's own FamilyKind/ Tier/ColdFormat pattern) rather than grafting a serde-style nested tagged union onto a decoder that was never built for one. The exact_kind and grouping gaps §6 previously listed as open follow-ups close as a direct consequence of the realignment, not as separate work. Explicit about scope: this is a schema proposal against real code, not a claim ASAPCollector has implemented it, and lists which MetricFamily fields have no DAG counterpart and correctly stay untouched (tier, spatial_filter, gos_*, emit_heap/weight_mode, threshold/CDM, cold archive, control_channel). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: align Planner integration and compiled plan contracts * docs: rewrite ASAPPlanner integration migration plan * docs: rewrite compiled and backend plan designs * docs: reorganize control-plane design by ownership --------- 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
Diagnosis chain (closes #298)
Pre-fix: `sum by (zone) (http_requests_total)` on asap returned ~37× the b0 baseline (Issue #298 body table); post-fix the asap tier returns the per-window event count, matching `b0_rate × window_secs` within ~0% (see measurements below).
Constraints honored
Test plan
Regression baselines
Companion (separate PR)
Static bootstrap YAMLs (`deploy/mvp-multinode/configs/asap/asap-otel-agent-b6-asap-single-sketch.yaml` and the singlenode variant) need the matching `cumulativetodelta` processor declaration so agents pre-OpAMP-apply also have the fix. That edit lives in `ASAPCollector` and is staged in the working tree there alongside other in-flight changes; will land in a separate PR.
🤖 Generated with Claude Code