Repository navigation
fix(metrics): keep historical export evidence out of current gauges - #301
Merged
Merged
Conversation
After ownership loss and regain, a failed or budget-cut export read keeps the previous owner's rows, marked historical in health. The export gauges still summed those rows, so Prometheus reported the old owner's committed peers as current and PrefixdBlackholeNotExported could keep firing on a historical warning. Metrics now use a current-only summary: a historical route counts as a read error, while health and detail are unchanged. The export rotation cursor also moved one place per pass regardless of how many reads fit in the budget. It now resumes at the first read the budget cut off, so N blackholes at K reads per budget are all read within ceil(N/K) passes. The cursor advances only when the pass is stored.
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.
Problem
Historical evidence reported as current in metrics. After the controller loses and regains ownership, a failed or budget-cut export read keeps the previous owner's evidence for the route (#298). That includes its peer rows, counts and warning, and health and detail mark it
historical: true.ExportEvidenceStore::summary()still summed those counts and warnings, and reconciliation published the result asprefixd_blackhole_export_peers{disposition}andprefixd_blackhole_unexported{reason}. Prometheus therefore reported the old owner's committed peers as current, andPrefixdBlackholeNotExportedcould keep firing on a historical warning.Rotation advanced one place per pass. The export rotation cursor (#294) moved one position per pass however many reads fit in the budget. With K > 1 reads per budget, some blackholes went unread for up to N - K + 1 passes, not ceil(N/K).
Fix
ExportEvidenceStore::current_summary()is the same aggregation without historical routes' counts and warnings. A historical route still counts inprefixd_blackhole_export_read_errors, because its read this pass failed. Reconciliation uses it for the gauges.summary()and the health and detail JSON are unchanged. No new metric or label.Fixedsection are updated. The alert rule and its promtool test are unchanged: the rule still keys on the reason gauge, which now counts only current evidence.Tests
historical_export_evidence_is_not_published_as_current_metrics: a committed route and anexport_policy-withheld route are read at epoch 0; then ownership is lost and regained and both reads fail. Health still shows the historical rows (historical, committed 1, one unexported). The disposition gauges are all 0, the unexported gauges sum to 0, and read errors are 2. A fresh read of the withheld route makes it current again.blackhole_export_rotation_advances_by_the_reads_in_each_pass: 5 candidates, 4 s reads in a 10 s budget (K = 2). After ceil(5/2) = 3 passes all five have been read, and the cursor is 6.Negative controls: I applied only the new tests to
main'sreconcile.rsandexport_evidence.rs. Both fail:Gate
Every
just gatestep passed on this content, but not as one uninterrupted run. Twojust gateruns stopped at the GoBGP-only test lane on the PostgreSQL row fixture withCreateContainer(RequestTimeoutError): local Docker was failing to create any container at the time, which a plaindocker createprobe confirmed. Those runs had passed fmt, ruff, proto and both clippy lanes. After Docker recovered, the remaining steps ran in order:cargo test --no-fail-fastfor both feature sets: 1763 passed, 0 failed, 50 ignored, including the PostgreSQL container tests.msrv,build,frontend,auditanddeny: all passed.speakers(GoBGP fixture plus rustbgpd 0.79.0): 33 passed.git diff --checkis clean.