fix(metrics): crawl duration was a millisecond series named for a base unit - #62
Merged
Conversation
…e unit `crawl.duration_ms` recorded a Date.now() delta through timing(), which declares unit `ms`. Seconds is the OTel and Prometheus base unit, so every dashboard quantile over this series was reading against the convention the rest of the org assumes. The instrument becomes `crawl_duration_seconds` and the call site divides in the same change. A `_seconds` histogram still fed milliseconds reads 1000x high in a series whose name and unit both promise seconds, which is worse than the wrong name — the wrong name is at least honest. Explicit bucket edges land with it, and they matter more here than the rename. A full crawl sweep is minutes; OTel's default edges top out at 10000, which as seconds is under three hours but as a range for this series puts every real observation in the overflow bucket. `histogram_quantile` cannot return a value above the highest finite edge, so the three dashboard panels would have rendered a constant rather than a latency. Dashboard series and the runbook's metric name move with it. Runtime re-vendored at 48c8ae8 for metrics.duration().
The coverage floors here sit within a tenth of a point of actual, so adding an untested export dropped both lines and functions below them. Two things were wrong rather than one. `duration()` and the crawl buckets had no test — added, and the useful assertion is not that the unit string reads `s` but that a 400-second sweep lands in a declared bucket rather than the overflow one above OTel's 10000 default, since no quantile can return a value from there. And `timing()` was left with no callers at all: `recordCrawlDuration` was its only one, and it now records seconds. Keeping an unused wrapper to satisfy a coverage floor would be the wrong repair — it is dead code either way, so it is deleted. Lines 93.10 and functions 94.07 against floors of 93 and 94, both now above where main sat.
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.
crawl.duration_msrecorded aDate.now()delta throughtiming(), which declares unitms. Seconds is the OTel and Prometheus base unit, so every dashboard quantile over this series read against the convention the rest of the org assumes.The instrument becomes
crawl_duration_secondsand the call site divides in the same change. A_secondshistogram still fed milliseconds reads 1000× high in a series whose name and unit both promise seconds — worse than the wrong name, because the wrong name is at least honest.The buckets matter more than the rename here
A full crawl sweep is minutes. OTel's default bucket edges top out at 10000, and
histogram_quantilecannot return a value above the highest finite edge — so once the series is in seconds, every real observation lands in the overflow bucket and the three dashboard panels render a constant rather than a latency.CRAWL_DURATION_BUCKETS = [10, 30, 60, 120, 300, 600, 1200]seconds, sized for the actual shape of a sweep.Also moved
label_valuestemplate variabledocs/RUNBOOK.md's metric namemetrics.duration()(nanohype#209)Verification
tsc --noEmitclean, lint and format clean, 26 test files / 229 tests green.