Skip to content

fix: expire boundary double-count and label sort order in counter chains#42

Merged
davidbirdsong merged 1 commit into
developfrom
davidbirdsong/net-146-switch-cloudflare-exporter-from-pull-based-scraping-to-push-14
Jul 3, 2026
Merged

fix: expire boundary double-count and label sort order in counter chains#42
davidbirdsong merged 1 commit into
developfrom
davidbirdsong/net-146-switch-cloudflare-exporter-from-pull-based-scraping-to-push-14

Conversation

@davidbirdsong

Copy link
Copy Markdown

Summary

Fix two bugs that cause counter value spikes in pushed metrics, add Prometheus observability counters for convergence tuning, and add a fixture capture tool for future spike investigations.

Bug fixes

  • Expire boundary double-count. When WindowTTL == Lookback (or after collapsing them into a single setting), buckets at exactly the boundary were expired and then re-fetched on the next tick. The re-inserted gauge was double-counted in the chain base, shifting all subsequent counters upward. Fixed by changing the expire check from >= to >.

  • Label sort order in Key.String(). Keys with the same labels in different insertion order produced different string representations, causing the engine to create separate counter chains for what VM treats as the same series. The seeding path (LastValues) iterates a Go map (unordered), so seeded chains could silently diverge from live chains. Fixed by sorting labels by name before serialization.

Collapse WindowTTL into Lookback

WindowTTL and Lookback served the same purpose in practice (a window only receives data from the live fetch, so keeping it open longer than the fetch range is pointless). Removed WindowTTL as a separate setting. Lookback now controls both the live query range and window expiry.

Prometheus metrics

Added per-tick counters and gauges on /metrics labeled by component to make convergence behavior observable and tuning data-driven:

Metric Type Tuning signal
converge_post_stabilize_updates_total counter High = threshold too low
converge_expire_flushes_total counter High = threshold too high
converge_ingest_samples_total counter Tracker stabilization volume
converge_expire_samples_total counter TTL expiry corrections
converge_live_fetch_observations_total counter CF API observation volume
converge_backfill_fetch_observations_total counter Backfill volume
converge_snapshot_samples_total counter Post-backfill snapshot size
converge_open_windows gauge Active window count
converge_tracker_count gauge Active tracker count
converge_push_errors_total counter Sink failures

Fixture capture tool

cmd/capture-fixtures fetches observations from the CF API for a specific zone and time range, trims to a single metric, and writes JSON fixtures for use in spike_test.go. Includes a second captured incident (zone 43c0ec06, 2026-06-26 14:00-14:22 UTC).

Spike test improvements

Refactored spike tests to be table-driven across multiple fixture sets. Both the original supabase.co capture and the new 0626 incident are tested for pre-fix spike detection and post-fix clean output.

Test plan

  • go test ./... passes
  • TestExpireBoundaryDoubleCount demonstrates and verifies the boundary fix
  • TestKeyStringLabelOrder demonstrates and verifies the sort fix
  • TestSpikeDetection_PreFix confirms spikes exist under old behavior (both fixture sets)
  • TestSpikeDetection_PostFix confirms no spikes after fix (both fixture sets)
  • Deploy to staging, verify converge_* metrics appear on /metrics
  • Verify no rate spikes in Grafana after deploy with rate(cloudflare_zone_requests_total[5m]) * 60

@davidbirdsong

Copy link
Copy Markdown
Author

This change is part of the following stack:

Change managed by git-spice.

@davidbirdsong davidbirdsong force-pushed the davidbirdsong/net-146-switch-cloudflare-exporter-from-pull-based-scraping-to-push-14 branch 5 times, most recently from a702fbd to 316fd99 Compare June 30, 2026 20:13
@davidbirdsong davidbirdsong requested a review from a team as a code owner June 30, 2026 20:13
@davidbirdsong

Copy link
Copy Markdown
Author

Waiting on reads granted: https://github.com/supabase/k8s-observability/pull/455

@davidbirdsong davidbirdsong force-pushed the davidbirdsong/net-146-switch-cloudflare-exporter-from-pull-based-scraping-to-push-14 branch from 316fd99 to 3d428e6 Compare July 1, 2026 21:04
Two bugs that cause counter value spikes in pushed metrics:

1. When Lookback == WindowTTL, buckets at the exact boundary were
expired then re-fetched, double-counting the gauge in the chain
base. Changed expire check from >= to >.

2. Key.String() preserved label insertion order, so the same series
could map to different counter chains when labels arrived in
different order (e.g. seeding from VM). Labels are now sorted by
name before serialization.

Also: collapse WindowTTL into Lookback (they served the same purpose),
add Prometheus counters for convergence tuning, add fixture capture
tool and second spike incident test data.
@davidbirdsong davidbirdsong force-pushed the davidbirdsong/net-146-switch-cloudflare-exporter-from-pull-based-scraping-to-push-14 branch from 3d428e6 to 5f6a766 Compare July 1, 2026 23:16
@davidbirdsong davidbirdsong merged commit 2f01b18 into develop Jul 3, 2026
10 checks passed
@davidbirdsong davidbirdsong deleted the davidbirdsong/net-146-switch-cloudflare-exporter-from-pull-based-scraping-to-push-14 branch July 3, 2026 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants