Give the p95 alert a bucket edge to fire on, and test both percentiles - #389
Merged
Merged
Conversation
PortalApiSlow alerts on p95 > 2s, but the histogram had no edge at 2 — the edges ran 1.5 then 2.5. A quantile is interpolated inside whichever bucket it lands in, so every p95 that alert ever fired on was a straight line drawn across the range the threshold sits in. That is the same flaw 0.75 and 1.5 were added to fix, missed one level up. 5 was already an edge, so the p99 alert was fine. The tests that found it cover the machinery end to end: the buckets climb and never repeat, every latency threshold the rules alert on has an exact edge under it, and nothing above 0.5s more than doubles. A local implementation of histogram_quantile runs against the real edges — a healthy service sits under both thresholds, 3% of calls at 4s moves p99 past 2.5 while p95 stays under 2, a uniformly slow service trips both, a reported quantile never escapes the bucket its true value lands in, and nothing past 10s can read as more than 10 because +Inf has no upper edge to interpolate towards. The rules themselves are checked too: every histogram_quantile reads _bucket grouped by le, which is the difference between an alert and one that silently never fires, and every recorded name an alert references exists. Coverage last: all 156 procedures the router exposes carry the same timing middleware, by identity. One without it contributes to no percentile at all, however slow it gets.
aamoghS
changed the title
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.
The bug
PortalApiSlowalerts on p95 > 2s, but the histogram had no bucket edge at 2 — the edges ran… 1, 1.5, 2.5, 5, 10. A quantile is interpolated inside whichever bucket it lands in, so every p95 that alert has ever fired on was a straight line drawn across the range the threshold sits in.That is the same flaw
0.75and1.5were added to fix, missed one level up.5was already an edge, soPortalApiTailSlow(p99 > 5) was fine.Fix is one bucket edge at
2. Additive — it adds anle="2"series and changes nothing already recorded.The tests that found it
Buckets — climb, never repeat; every latency threshold the rules alert on has an exact edge under it (this is the one that failed); nothing above 0.5s more than doubles, since that is where the alerts live.
Percentile math — a local implementation of Prometheus
histogram_quantilerun against the real edges:+Infhas no upper edge to interpolate towardsThe rules — every
histogram_quantilereads_bucketgrouped byle, which is the difference between a working alert and one that silently never fires; p99 is recorded both overall and per procedure; every recorded name an alert references exists.Coverage — all 156 procedures the router exposes carry the same timing middleware, asserted by identity against the one on
publicProcedure. A procedure without it contributes to no percentile at all, however slow it gets. Paths are also checked to match[a-zA-Z0-9.]+, so the label stays bounded.What this does not do
Measure real latency. That needs a running instance and a load generator against live Neon. This tests the machinery that reports p95/p99 — that it can express the numbers, that the alerts read it correctly, and that nothing is unmeasured.
14 tests, all passing.