Conversation
…iance
The z-score divides a difference of rates, so its denominator has to be
the scatter of the rate from bucket to bucket. It was
STDDEV(CASE WHEN status IN ('STATUS_CODE_ERROR','ERROR') THEN 1.0 ELSE 0.0 END)
over raw spans, which is the scatter of individual span outcomes --
sqrt(p(1-p)), about 0.34 at a 14% error rate. Those are different
quantities, and the mismatch punished exactly the services worth
watching: the noisier the service, the larger the denominator. At the
13.8% baseline the live demo actually ran, clearing the 2.0 threshold
required the rate to jump 69 percentage points. A tripling from 13.8%
to 41% scored 0.80.
The denominator is now the standard deviation of the per-bucket error
rate across the baseline window, and both windows average over the same
5-minute buckets -- the same asymmetry the volume detector had.
It is also floored, because a healthy service's error rate is flat at
zero and its bucket-to-bucket stddev is therefore exactly zero. Dividing
by that hit the CASE's 0.0 fallback, so the service that had just
started failing was the one that could not alert: going from no errors
to 42% scored 0.00. One percentage point is the least noise worth
assuming. It keeps a single stray error in three thousand spans at
z=0.03 while a real break reaches z=42.
Measured on the fixtures in error_rate_sql_test.go, before -> after:
13.8% -> 41% (noisy baseline) 0.80 -> 27.40 now fires
13.8% -> 13.8% (noisy baseline) 0.01 -> 0.20 stays quiet
0% -> 42% (flat baseline) 0.00 -> 42.33 now fires
0% -> 0.03% (one stray error) 0.00 -> 0.03 stays quiet
Verified on the demo across five detector cycles: 1-2 anomalies each,
health 85-95, no errors. The concern that a more sensitive denominator
would produce an alert flood does not survive contact with the data.
…on grid service_performance answers a question about one service. It also returned the latency heatmap -- the twelve busiest services by every bucket in the window -- because the browser's performance page draws that grid and both callers shared one method. On the live demo that was about 85% of a 113 KB response describing twelve services the caller had not asked about, plus the second DuckDB query that built it. The grid is now opt-in through PerformanceOptions. The browser asks for it; the MCP tool does not, and skips the query rather than blanking the field afterwards, so the cost actually goes away. Also reattaches three doc comments that had been separated from what they document, and adds the guard that found the third. A declaration inserted between a doc comment and its subject is invisible to gofmt, go vet and the compiler: the new declaration inherits a comment describing something else and the original is left undocumented. errorRateAnomalySQL's doc had landed on minErrorRateStddev, ParquetStore.Trace's on TraceTotals, and attrsJSON's on attrBufPool -- where it had also gone stale, still promising a nil return from a function that returns "". Requiring every doc comment to begin with its own declaration's name would flag hundreds that simply do not follow that convention, and a guard that is mostly false positives gets muted; the first cut of this one flagged 17, nearly all of them legitimate. It checks the exact signature instead -- a doc comment whose first word names the declaration immediately below it -- which is what such an insertion produces and what nobody writes on purpose. One flag across 2,378 declarations, and it was real. The first version of that guard did not catch the bug it was written for: it walked only documented declarations, so it compared against the next documented one and stepped straight over the undocumented victim. It now walks every declaration in source order, verified by reintroducing the defect.
Member
Author
|
Superseded: rebuilt on main as a clean branch after #265 squash-merged. |
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.
service_performanceanswers a question about one service. It alsoreturned the latency heatmap -- the twelve busiest services by every
bucket in the window -- because the browser's performance page draws that
grid and both callers shared one method. On the live demo that was ~85%
of a 113 KB response describing twelve services the caller had not asked
about, plus the second DuckDB query that built it.
The grid is now opt-in via
PerformanceOptions. The browser asks for it;the MCP tool does not, and skips the query rather than blanking the
field afterwards.
Also reattaches three doc comments separated from what they document, and
adds the guard that found the third. A declaration inserted between a doc
comment and its subject is invisible to gofmt, vet and the compiler.
errorRateAnomalySQL's doc had landed onminErrorRateStddev,ParquetStore.Trace's onTraceTotals, andattrsJSON's onattrBufPool-- where it had also gone stale, promising a nil returnfrom a function that returns
"".The guard checks the exact signature (a doc whose first word names the
declaration immediately below it), not the general convention: the first
cut flagged 17, nearly all legitimate. This one flags 1 across 2,378
declarations, and it was real. Its first version also failed to catch the
bug it was written for -- it walked only documented declarations and
stepped over the undocumented victim. Verified by reintroducing the
defect.