From be54a7173da9e4fda40279e59af674c7bf45276b Mon Sep 17 00:00:00 2001 From: htjulia Date: Mon, 12 Oct 2026 16:12:22 +0900 Subject: [PATCH 1/2] fix(kpi): show a dash, not zero, when the platform keeps no totals On a platform with per-module metrics off, the request and error-rate tiles read "0" and "0.00%" over live traffic. They sum the per-module rows in `metrics`, and that frame is empty when nothing is aggregated, so an unknown became a confident zero. Seen live on quickstart under a 3,000-request burst. - The tiles read the `ready` ack the way the module table already does: recording off first, then metrics off, then not reported. In each case they show a dash and name the reason - Metrics on with no rows still shows 0, because that is true. Rows that did arrive are always shown, since the setting can change after the ack - Not replaced with `summary.count`: it is capped by the ring capacity and reads lower the busier the platform is. The summary trend lines stay - 7 scenarios replay the protean, Spring and Go connect sequences. With the old tiles, 4 of them fail; the other 3 guard what must not change Closes #81 Tags: #kpi #honesty Co-Authored-By: htjulia --- src/App.tsx | 7 +- src/components/kpi-row.tsx | 63 +++++++++++++-- src/components/kpi-totals.test.tsx | 123 +++++++++++++++++++++++++++++ 3 files changed, 186 insertions(+), 7 deletions(-) create mode 100644 src/components/kpi-totals.test.tsx diff --git a/src/App.tsx b/src/App.tsx index d92fe38..4629f4d 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -84,7 +84,12 @@ export default function App() { aria-busy={disconnected} > - +
diff --git a/src/components/kpi-row.tsx b/src/components/kpi-row.tsx index 9f03520..1bd97d3 100644 --- a/src/components/kpi-row.tsx +++ b/src/components/kpi-row.tsx @@ -80,12 +80,42 @@ function Tile({ ) } +/** + * Why the cumulative totals are unknown, or `null` when the numbers can be shown. + * + * Requests and error rate are sums over `metrics`, and an empty array sums to zero + * whether or not anything was recorded. A platform with per-module metrics off sends + * exactly that, so these tiles read "0 requests, 0.00%" over live traffic — the + * module table already knew better from the same ack, and the tiles did not ask. + * + * The order is the module table's (`emptyMetrics`), for the same reason: + * `metricsEnabled` is the effective value, so recording off reports metrics off too, + * and recording is the cause worth naming. Rows that did arrive are always shown — + * the setting is live and may have changed since the ack, but the rows are data. + * + * Not replaced with `summary.count`: that is capped by the ring `capacity`, so it + * would read lower the busier the platform got. A dash is the honest answer. + */ +function totalsUnknown( + metrics: ModuleMetricsSnapshot[], + metricsEnabled: boolean | null, + tracesEnabled: boolean | null, +): string | null { + if (metrics.length > 0) return null + if (tracesEnabled === false) return 'recording off' + if (metricsEnabled === false) return 'metrics off' + if (metricsEnabled === true) return null // on and idle: zero is the true count + return 'not reported' +} + /** * Header KPIs. Requests and error rate are the platform's all-time cumulative * aggregates (from `metrics`); the sub-line under each is the recent-window trend * from the `summary` SSE event (windowed count / error rate / p95 vs the previous * window). When `summary` is absent or a trend field is null, the sub-line renders - * a muted placeholder rather than a made-up delta. + * a muted placeholder rather than a made-up delta. When the platform is not + * aggregating per-module metrics the totals do not exist, and the tiles say why + * instead of showing zero (`totalsUnknown`). * * Latency p95 is the exception: it is `summary`'s windowed p95, the same number the * latency chart plots, so the tile reads as the chart's newest point. It used to be @@ -93,7 +123,18 @@ function Tile({ * third source and agreed with neither. Until a `summary` arrives it shows a dash — * the per-module figure is a different measurement, not a stand-in for this one. */ -export function KpiRow({ metrics, summary }: { metrics: ModuleMetricsSnapshot[]; summary: TraceSummary | null }) { +export function KpiRow({ + metrics, + summary, + metricsEnabled = null, + tracesEnabled = null, +}: { + metrics: ModuleMetricsSnapshot[] + summary: TraceSummary | null + metricsEnabled?: boolean | null + tracesEnabled?: boolean | null +}) { + const unknown = totalsUnknown(metrics, metricsEnabled, tracesEnabled) const total = metrics.reduce((a, m) => a + m.count, 0) const errors = metrics.reduce((a, m) => a + m.errorCount, 0) const errorRate = total ? errors / total : 0 @@ -103,7 +144,7 @@ export function KpiRow({ metrics, summary }: { metrics: ModuleMetricsSnapshot[]; return (
- + {summary?.requestsDeltaPct != null ? ( {Math.abs(summary.requestsDeltaPct * 100).toFixed(0)}% vs prev @@ -111,16 +152,26 @@ export function KpiRow({ metrics, summary }: { metrics: ModuleMetricsSnapshot[]; ) : ( {win} · no baseline yet )} + {unknown && · {unknown}} - + {summary?.errorRateDeltaPp != null ? ( {Math.abs(summary.errorRateDeltaPp).toFixed(2)}pp · {num(summary.errorCount)} err - ) : ( - {win} · {num(summary?.errorCount ?? errors)} err + ) : summary ? ( + {win} · {num(summary.errorCount)} err + ) : unknown ? null : ( + // The error count is the same sum as the rate above: no rows, no count. + {win} · {num(errors)} err )} + {unknown && {summary ? ' · ' : ''}{unknown}} ({ + moduleId: 'greeter', + count, + errorCount, + errorRate: count ? errorCount / count : 0, + p50LatencyMs: 1, + p95LatencyMs: 2, + p99LatencyMs: 3, + maxLatencyMs: 4, + lastSeenEpochMillis: 1_790_000_000_000, +}) + +describe('on a live connect sequence', () => { + let restoreEventSource: () => void + beforeEach(() => { + restoreEventSource = installFakeEventSource() + }) + afterEach(() => restoreEventSource()) + + function connect(body: string) { + const { result } = renderHook(() => useConsoleData()) + act(() => { + const es = FakeEventSource.current() + es.emit('open') + replay(es, body) + }) + const { data, platform } = result.current + render( + , + ) + } + + it('says metrics are off, not that nothing happened, on a platform that turned them off', () => { + // 🔴 The protean capture: `metricsEnabled: false`, `metrics: []`, and a summary + // counting traffic. The tiles used to print a confident zero over it. + connect(PROTEAN_CONNECT) + expect(value('Requests · total')).toBe('—') + expect(value('Error rate')).toBe('—') + expect(subline('Requests · total')).toContain('metrics off') + expect(subline('Error rate')).toContain('metrics off') + // The windowed trend is a different measurement, and it is still there. + expect(subline('Requests · total')).toContain('vs prev') + }) + + it('says it was not told, on a platform that sends no ack and no rows', () => { + connect(SPRING_CONNECT) + expect(value('Requests · total')).toBe('—') + expect(subline('Requests · total')).toContain('not reported') + }) + + it('shows the totals a platform did send', () => { + // Go sends module rows (and an older `hello`, not `ready`): the rows are data. + connect(GO_CONNECT) + const rows = JSON.parse(parseStream(GO_CONNECT).find((f) => f.event === 'metrics')!.data) as ModuleMetricsSnapshot[] + expect(value('Requests · total')).toBe(String(rows.reduce((a, r) => a + r.count, 0))) + expect(subline('Requests · total')).not.toMatch(/off|not reported/) + }) +}) + +describe('the reading, case by case', () => { + const summary = JSON.parse(parseStream(PROTEAN_CONNECT).find((f) => f.event === 'summary')!.data) as TraceSummary + + it('names recording before metrics, as the module table does', () => { + // `metricsEnabled` is the effective value, so recording off reports metrics off + // too; "metrics off" would point at the wrong switch. + render() + expect(value('Requests · total')).toBe('—') + expect(subline('Requests · total')).toContain('recording off') + expect(subline('Requests · total')).not.toContain('metrics off') + }) + + it('keeps a true zero: metrics on and no traffic yet', () => { + render() + expect(value('Requests · total')).toBe('0') + expect(value('Error rate')).toBe('0.00%') + }) + + it('shows rows that arrived even if the ack said off', () => { + // The setting is live: it can be switched on after the ack. Rows are data. + render() + expect(value('Requests · total')).toBe('40') + expect(value('Error rate')).toBe('10.00%') + }) + + it('names the reason alone when there is no summary to trend', () => { + render() + expect(subline('Error rate')).toBe('metrics off') + }) +}) From 90c4bade814d14552d6d37f0c0fc0ef8bbbe48c5 Mon Sep 17 00:00:00 2001 From: htjulia Date: Mon, 12 Oct 2026 16:15:53 +0900 Subject: [PATCH 2/2] fix(kpi): put the reason beside the dash, not under it Checked live on quickstart with metrics off. Appending the reason to the sub-line wrapped the requests tile onto a second line, so it stood taller than the rest of the row. The reason now sits where the unit would, after a non-breaking space, and the sub-lines say what they always said. Tags: #kpi Co-Authored-By: htjulia --- src/components/kpi-row.tsx | 16 +++++++++++----- src/components/kpi-totals.test.tsx | 20 ++++++++------------ 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/components/kpi-row.tsx b/src/components/kpi-row.tsx index 1bd97d3..f9c9f9d 100644 --- a/src/components/kpi-row.tsx +++ b/src/components/kpi-row.tsx @@ -144,7 +144,13 @@ export function KpiRow({ return (
- + {/* The reason sits where the unit would, beside the dash: the sub-line keeps + saying what it always says, and the tile stays one line tall. */} + {summary?.requestsDeltaPct != null ? ( {Math.abs(summary.requestsDeltaPct * 100).toFixed(0)}% vs prev @@ -152,13 +158,12 @@ export function KpiRow({ ) : ( {win} · no baseline yet )} - {unknown && · {unknown}} {summary?.errorRateDeltaPp != null ? ( @@ -167,11 +172,12 @@ export function KpiRow({ ) : summary ? ( {win} · {num(summary.errorCount)} err - ) : unknown ? null : ( + ) : unknown ? ( // The error count is the same sum as the rate above: no rows, no count. + {win} · no summary yet + ) : ( {win} · {num(errors)} err )} - {unknown && {summary ? ' · ' : ''}{unknown}} { // 🔴 The protean capture: `metricsEnabled: false`, `metrics: []`, and a summary // counting traffic. The tiles used to print a confident zero over it. connect(PROTEAN_CONNECT) - expect(value('Requests · total')).toBe('—') - expect(value('Error rate')).toBe('—') - expect(subline('Requests · total')).toContain('metrics off') - expect(subline('Error rate')).toContain('metrics off') + expect(value('Requests · total')).toMatch(/^—\smetrics\soff$/) + expect(value('Error rate')).toMatch(/^—\smetrics\soff$/) // The windowed trend is a different measurement, and it is still there. expect(subline('Requests · total')).toContain('vs prev') }) it('says it was not told, on a platform that sends no ack and no rows', () => { connect(SPRING_CONNECT) - expect(value('Requests · total')).toBe('—') - expect(subline('Requests · total')).toContain('not reported') + expect(value('Requests · total')).toMatch(/^—\snot\sreported$/) }) it('shows the totals a platform did send', () => { @@ -87,7 +84,7 @@ describe('on a live connect sequence', () => { connect(GO_CONNECT) const rows = JSON.parse(parseStream(GO_CONNECT).find((f) => f.event === 'metrics')!.data) as ModuleMetricsSnapshot[] expect(value('Requests · total')).toBe(String(rows.reduce((a, r) => a + r.count, 0))) - expect(subline('Requests · total')).not.toMatch(/off|not reported/) + expect(value('Requests · total')).not.toMatch(/off|not reported/) }) }) @@ -98,9 +95,7 @@ describe('the reading, case by case', () => { // `metricsEnabled` is the effective value, so recording off reports metrics off // too; "metrics off" would point at the wrong switch. render() - expect(value('Requests · total')).toBe('—') - expect(subline('Requests · total')).toContain('recording off') - expect(subline('Requests · total')).not.toContain('metrics off') + expect(value('Requests · total')).toMatch(/^—\srecording\soff$/) }) it('keeps a true zero: metrics on and no traffic yet', () => { @@ -116,8 +111,9 @@ describe('the reading, case by case', () => { expect(value('Error rate')).toBe('10.00%') }) - it('names the reason alone when there is no summary to trend', () => { + it('does not invent an error count when there is no summary either', () => { render() - expect(subline('Error rate')).toBe('metrics off') + expect(value('Error rate')).toMatch(/^—\smetrics\soff$/) + expect(subline('Error rate')).not.toMatch(/\d+ err/) }) })