diff --git a/internal/intelligence/detector.go b/internal/intelligence/detector.go index 10de6c5a..3bf09abf 100644 --- a/internal/intelligence/detector.go +++ b/internal/intelligence/detector.go @@ -148,37 +148,61 @@ func (d *Detector) detectAnomalies(ctx context.Context, start, end time.Time) [] return anomalies } -// detectErrorRateAnomalies detects error rate spikes -func (d *Detector) detectErrorRateAnomalies(ctx context.Context, start, end time.Time) []Anomaly { - startNano := start.UnixNano() - endNano := end.UnixNano() - namespace := d.duck.DefaultNamespace() - scope := detectorScopeClause(namespace) - - // Compare current error rate to baseline (previous period) - sql := fmt.Sprintf(` +// errorRateAnomalySQL compares the error rate in [startNano, endNano) against +// the window of equal length immediately before it. +// +// The z-score divides a difference of rates, so its denominator has to be the +// scatter of the RATE from bucket to bucket. It used to be +// STDDEV(CASE WHEN status = 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, so at the 13.8% baseline the live demo ran, clearing the 2.0 +// threshold needed the rate to jump 69 percentage points. A tripling to 41% +// scored 0.80. +// +// The denominator is floored because a healthy service's rate is flat at zero, +// giving it a bucket-to-bucket stddev of exactly zero -- so the service that +// just started failing was the one that could not alert (z=0.00 going from no +// errors to 42%). One percentage point is the least noise worth assuming: it +// keeps a single stray error in a few thousand spans below the threshold while +// letting a real break through. +// minErrorRateStddev floors the error-rate z-score denominator at one +// percentage point. See errorRateAnomalySQL. +const minErrorRateStddev = 0.01 + +func errorRateAnomalySQL(startNano, endNano int64, scope string) string { + return fmt.Sprintf(` WITH current_period AS ( SELECT - service as service_name, - COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR')) AS error_count, - COUNT(*) AS total_count, - (COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR'))::DOUBLE / COUNT(*)::DOUBLE) AS error_rate - FROM spans - WHERE start_unix_nano >= %d AND start_unix_nano < %d - %s - GROUP BY service + service_name, + AVG(bucket_rate) AS error_rate + FROM ( + SELECT + service as service_name, + (COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR'))::DOUBLE / COUNT(*)::DOUBLE) AS bucket_rate + FROM spans + WHERE start_unix_nano >= %d AND start_unix_nano < %d + %s + GROUP BY service, time_bucket(INTERVAL '5 minutes', start_time) + ) buckets + GROUP BY service_name ), baseline_period AS ( SELECT - service as service_name, - COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR')) AS error_count, - COUNT(*) AS total_count, - (COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR'))::DOUBLE / COUNT(*)::DOUBLE) AS error_rate, - STDDEV(CASE WHEN status IN ('STATUS_CODE_ERROR', 'ERROR') THEN 1.0 ELSE 0.0 END) AS error_stddev - FROM spans - WHERE start_unix_nano >= %d AND start_unix_nano < %d - %s - GROUP BY service + service_name, + AVG(bucket_rate) AS error_rate, + GREATEST(COALESCE(STDDEV(bucket_rate), 0.0), %f) AS error_stddev + FROM ( + SELECT + service as service_name, + (COUNT(*) FILTER (WHERE status IN ('STATUS_CODE_ERROR', 'ERROR'))::DOUBLE / COUNT(*)::DOUBLE) AS bucket_rate + FROM spans + WHERE start_unix_nano >= %d AND start_unix_nano < %d + %s + GROUP BY service, time_bucket(INTERVAL '5 minutes', start_time) + ) buckets + GROUP BY service_name ) SELECT c.service_name, @@ -191,7 +215,18 @@ func (d *Detector) detectErrorRateAnomalies(ctx context.Context, start, end time FROM current_period c LEFT JOIN baseline_period b ON c.service_name = b.service_name WHERE c.error_rate > 0 - `, startNano, endNano, scope, startNano-endNano+startNano, startNano, scope) + `, startNano, endNano, scope, minErrorRateStddev, startNano-endNano+startNano, startNano, scope) +} + +// detectErrorRateAnomalies detects error rate spikes +func (d *Detector) detectErrorRateAnomalies(ctx context.Context, start, end time.Time) []Anomaly { + startNano := start.UnixNano() + endNano := end.UnixNano() + namespace := d.duck.DefaultNamespace() + scope := detectorScopeClause(namespace) + + // Compare current error rate to baseline (previous period) + sql := errorRateAnomalySQL(startNano, endNano, scope) resp := d.duck.ExecuteSQL(ctx, query.SQLRequest{Query: sql}) if resp.Error != "" { diff --git a/internal/intelligence/error_rate_sql_test.go b/internal/intelligence/error_rate_sql_test.go new file mode 100644 index 00000000..b324343e --- /dev/null +++ b/internal/intelligence/error_rate_sql_test.go @@ -0,0 +1,130 @@ +package intelligence + +import ( + "database/sql" + "math" + "testing" + "time" +) + +// errorRateFixture writes six 5-minute buckets: the first three are the +// baseline window, the last three the current one. Each entry is a bucket's +// error percentage, so a service's whole history is one readable slice. +func errorRateFixture(t *testing.T, db *sql.DB, start time.Time, services map[string][]float64) { + t.Helper() + if _, err := db.Exec(`CREATE TABLE IF NOT EXISTS spans ( + service TEXT, namespace TEXT, status TEXT, start_time TIMESTAMP, start_unix_nano BIGINT + )`); err != nil { + t.Fatal(err) + } + const perBucket = 1000 + for service, rates := range services { + for bucket, rate := range rates { + at := start.Add(time.Duration(bucket) * 5 * time.Minute) + errors := int(math.Round(rate * perBucket)) + for i := range perBucket { + status := "STATUS_CODE_OK" + if i < errors { + status = "STATUS_CODE_ERROR" + } + ts := at.Add(time.Duration(i) * time.Millisecond) + if _, err := db.Exec(`INSERT INTO spans VALUES (?, 'default', ?, ?, ?)`, + service, status, ts, ts.UnixNano()); err != nil { + t.Fatal(err) + } + } + } + } +} + +func errorRateScores(t *testing.T, db *sql.DB, start, end time.Time) map[string]float64 { + t.Helper() + rows, err := db.Query(errorRateAnomalySQL(start.UnixNano(), end.UnixNano(), "")) + if err != nil { + t.Fatalf("error rate query: %v", err) + } + defer rows.Close() + scores := map[string]float64{} + for rows.Next() { + var service string + var current, baseline, zScore float64 + if err := rows.Scan(&service, ¤t, &baseline, &zScore); err != nil { + t.Fatal(err) + } + scores[service] = zScore + t.Logf("%-14s current=%.3f baseline=%.3f z=%.2f", service, current, baseline, zScore) + } + if err := rows.Err(); err != nil { + t.Fatal(err) + } + return scores +} + +// The z-score divided a difference of RATES by the STDDEV of a per-span 0/1 +// indicator. That indicator's spread is sqrt(p(1-p)) -- the scatter of +// individual span outcomes, about 0.34 at a 14% error rate -- and not the +// scatter of the rate itself from bucket to bucket, which is what a z-score on +// a rate needs. The units did not match, and the mismatch got worse the noisier +// the service: at the 13.8% baseline the live demo actually ran, clearing the +// 2.0 threshold required the rate to jump by 69 percentage points. +// +// So the services most worth watching were the ones least able to alert. +const errorRateThreshold = 2.0 + +func TestErrorRateFiresOnASpikeFromANoisyBaseline(t *testing.T) { + db, err := sql.Open("duckdb", "") + if err != nil { + t.Fatal(err) + } + defer db.Close() + + end := time.Date(2026, 9, 21, 18, 0, 0, 0, time.UTC) + start := end.Add(-15 * time.Minute) + errorRateFixture(t, db, start.Add(-15*time.Minute), map[string][]float64{ + // Sat at 13.8% and tripled. The old denominator scored this 0.47. + "spiking-noisy": {0.13, 0.14, 0.138, 0.40, 0.42, 0.41}, + // Same noisy baseline, no spike. Must stay quiet. + "steady-noisy": {0.13, 0.14, 0.138, 0.135, 0.142, 0.137}, + }) + + scores := errorRateScores(t, db, start, end) + if z := scores["spiking-noisy"]; math.Abs(z) < errorRateThreshold { + t.Errorf("a service that went from 13.8%% to 41%% errors scored z=%.2f, below the %.1f threshold", z, errorRateThreshold) + } + if z := scores["steady-noisy"]; math.Abs(z) >= errorRateThreshold { + t.Errorf("a service holding steady at 13.8%% scored z=%.2f and would alert", z) + } +} + +// A healthy service has a flat 0% error rate, so the bucket-to-bucket stddev of +// its rate is exactly zero. Dividing by it yields the CASE's 0.0 fallback, and +// the service that just started failing is the one that cannot alert. The +// denominator needs a floor. +func TestErrorRateFiresWhenAFlatlineBreaks(t *testing.T) { + db, err := sql.Open("duckdb", "") + if err != nil { + t.Fatal(err) + } + defer db.Close() + + end := time.Date(2026, 9, 21, 18, 0, 0, 0, time.UTC) + start := end.Add(-15 * time.Minute) + errorRateFixture(t, db, start.Add(-15*time.Minute), map[string][]float64{ + "clean-then-broken": {0, 0, 0, 0.40, 0.45, 0.42}, + "clean-throughout": {0, 0, 0, 0, 0, 0}, + // One stray error in a thousand spans is not an incident. + "clean-with-a-blip": {0, 0, 0, 0, 0.001, 0}, + }) + + scores := errorRateScores(t, db, start, end) + if z := scores["clean-then-broken"]; math.Abs(z) < errorRateThreshold { + t.Errorf("a service that went from no errors to 42%% scored z=%.2f, below the %.1f threshold", z, errorRateThreshold) + } + if z, ok := scores["clean-with-a-blip"]; ok && math.Abs(z) >= errorRateThreshold { + t.Errorf("one error in three thousand spans scored z=%.2f and would alert", z) + } + // A service with no errors at all is filtered out by `WHERE c.error_rate > 0`. + if z, ok := scores["clean-throughout"]; ok && math.Abs(z) >= errorRateThreshold { + t.Errorf("a service with no errors scored z=%.2f", z) + } +}