Say in the HELP text what oracle_last_metrics_update actually covers - #820
Open
shaurya703 wants to merge 1 commit into
Open
Say in the HELP text what oracle_last_metrics_update actually covers#820shaurya703 wants to merge 1 commit into
shaurya703 wants to merge 1 commit into
Conversation
Fixes SO4-Markets#800. "Timestamp of last metrics update" reads as any metrics activity, but only the price and keeper cycle recorders call stamp() — the four HTTP recorders never have. An operator reading it as whole-service liveness would see it advancing from cycle activity while the HTTP layer was stalled, and conclude the service was healthy. Took the documentation half of the issue's either/or. The other half — calling stamp() from the HTTP recorders — changes what the gauge means, and renaming the series breaks any dashboard or alert already selecting it; both are the maintainers' call rather than a side effect of clarifying a HELP string. Nothing else in the repo references the name. stamp()'s doc comment now says the same thing from the other side, so the next person adding a recorder sees why it is not called there.
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
1 participant
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.
Fixes #800.
Why the documentation half, and not the other one
The issue offers either/or. I took the HELP text deliberately:
oracle_last_cycle_metrics_update) breaks every dashboard panel and alert rule already selecting the old name. That is a migration, not a clarification.stamp()from the HTTP recorders changes what the gauge means: it would then advance whenever anything hit the service, which is close to the opposite of the failure it is watched for — the cycles hanging while HTTP keeps answering.Both are yours to decide. This one makes the exported text true today and breaks nothing, and either of the others is still open afterwards.
stamp()'s doc comment now says the same thing from the other side, so the next person adding a recorder sees why it is not called there rather than assuming an oversight — which is how the two sides drifted in the first place.Nothing else in the repo references the metric name (checked README,
docs/,config/, tests), so the HELP string was the only place the scope was stated.Checks
cargo test16 suites green,cargo fmt --checkclean,cargo clippy --all-targets -- -D warningsclean.test_promtool_validationstill passes, so the exposition format is still valid.