Skip to content

Commit efbcfe1

Browse files
os-zhuangclaude
andauthored
feat(observability): admin-only per-request timing detail via X-OS-Debug-Timing: json (#2408) (#3169)
Completes the optional "richer JSON" diagnostic from #2408. On top of the basic Server-Timing header, an admin/service caller can request a per-query breakdown — the slowest SQL statements + a query count — with `X-OS-Debug-Timing: json`, returned in a separate `X-OS-Debug-Timing-Detail` header (compact JSON). It is admin-only **even under global mode** — an ordinary caller never sees SQL shapes. - observability: `PerfTiming` gains opt-in per-event detail capture (`enableDetail` / `recordDetail` / `details`) + ambient `recordServerTimingDetail`. The disclosure gate gains a `privileged` level (set by `allowPerfDisclosure`, read via `isPerfDisclosurePrivileged`) so the richer detail is gated independently of the basic header. - driver-sql: with detail on, the query listener also records each query's PARAMETRIZED statement (knex `q.sql`, `?` placeholders) — never the bindings, so no literal row value enters the collector. Free when detail is off. - plugin-hono-server: `X-OS-Debug-Timing: json` enables capture; the middleware emits `X-OS-Debug-Timing-Detail` (slowest queries, capped + sanitized to header-safe ASCII) only for a proven admin. Tests: unit coverage for the detail collector, the privileged gate level, the driver's parametrized-SQL capture, the middleware's json/basic/global paths, and a real-HTTP end-to-end test (admin gets the detail header over a genuine socket running real SQL; a non-admin never does, even under global mode). Docs + changeset updated. Basic/global behavior unchanged; `json` is purely additive. Claude-Session: https://claude.ai/code/session_011upHdyr5AnNc6dWAu63qxD Co-authored-by: Claude <noreply@anthropic.com>
1 parent ff41188 commit efbcfe1

10 files changed

Lines changed: 558 additions & 21 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/observability": minor
3+
"@objectstack/plugin-hono-server": minor
4+
"@objectstack/driver-sql": minor
5+
---
6+
7+
feat(observability): admin-only richer per-request timing detail via `X-OS-Debug-Timing: json` (#2408)
8+
9+
Completes the optional "richer JSON" diagnostic from #2408. In addition to the
10+
basic `Server-Timing` header, an admin/service caller can now request a
11+
per-query breakdown — the slowest SQL statements and a query count — by sending
12+
`X-OS-Debug-Timing: json`. The detail is returned in a separate
13+
`X-OS-Debug-Timing-Detail` response header (compact JSON) and is **admin-only,
14+
even under global mode**: an ordinary caller never sees SQL shapes.
15+
16+
- **observability**: `PerfTiming` gains opt-in per-event detail capture
17+
(`enableDetail` / `recordDetail` / `details`) plus the ambient
18+
`recordServerTimingDetail`. The disclosure gate gains a `privileged` level
19+
(set by `allowPerfDisclosure`, read via `isPerfDisclosurePrivileged`) so the
20+
richer detail can be gated independently of the basic header.
21+
- **driver-sql**: when detail capture is on, the query listener additionally
22+
records each query's **parametrized** statement (knex's `q.sql`, `?`
23+
placeholders) — never the bindings, so no literal row value ever enters the
24+
collector. Zero overhead when detail is off.
25+
- **plugin-hono-server**: `X-OS-Debug-Timing: json` enables detail capture; the
26+
middleware emits `X-OS-Debug-Timing-Detail` (slowest queries, capped and
27+
sanitized to header-safe ASCII) only when the principal is a proven admin.
28+
29+
Basic and global behavior are unchanged; `json` is purely additive.

docs/OBSERVABILITY.md

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,28 @@ they can never pull timings, so this is safe to leave available on a live
271271
environment. This is the path to reach for when diagnosing "why is *this* request
272272
slow?" against a running environment.
273273

274+
For a per-query breakdown, an admin sends `json` instead:
275+
276+
```
277+
X-OS-Debug-Timing: json
278+
```
279+
280+
which adds an **admin-only** `X-OS-Debug-Timing-Detail` response header — compact
281+
JSON listing the slowest SQL statements (by shape), the single slowest, and the
282+
query count:
283+
284+
```json
285+
{"db":{"count":6,"totalMs":210.3,"slowest":{"sql":"select * from widgets where id = ?","dur":88.1},"queries":[{"sql":"select * from widgets where id = ?","dur":88.1}, ]}}
286+
```
287+
288+
The statements are **parametrized** — knex's `?` placeholders, never the
289+
bindings — so the query *shape* (the useful part for spotting N round-trips) is
290+
exposed while literal row values never leave the server. The detail is
291+
**admin-only even under global mode**: an ordinary caller who sends `json` still
292+
gets the basic `Server-Timing` header (if global mode is on) but never the
293+
`X-OS-Debug-Timing-Detail` payload. The list is capped and the labels sanitized
294+
to header-safe ASCII.
295+
274296
To hard-disable both paths (no middleware registered at all), set
275297
`serverTiming: false` explicitly.
276298

@@ -285,7 +307,7 @@ out of the box:
285307
| `handler` | HTTP adapter | Route-handler execution. |
286308
| `serialize` | HTTP adapter | Response JSON encoding. |
287309
| `auth` | Dispatcher | Identity / session resolution — the prime suspect for unexplained data-API overhead. |
288-
| `db` | SQL driver | Total SQL time across the request; `desc` is the **query count** (folded from knex's per-query events, attributed to the originating request via `AsyncLocalStorage` so it is correct under concurrency). SQL text is never emitted. |
310+
| `db` | SQL driver | Total SQL time across the request; `desc` is the **query count** (folded from knex's per-query events, attributed to the originating request via `AsyncLocalStorage` so it is correct under concurrency). The basic header carries no SQL text; individual **parametrized** statements appear only in the admin-only `X-OS-Debug-Timing: json` detail payload. |
289311
| `hooks` | ObjectQL engine | Total business-hook execution time; `desc` is the hook count. |
290312

291313
Each phase is recorded through a request-scoped collector that is a no-op when
@@ -336,5 +358,7 @@ countServerTiming('db', queryMs, 'queries'); // → db;dur=<sum>;desc="<n> queri
336358
- [ ] Alerts wired: error rate, p95 latency per route.
337359
- [ ] (Optional) `Server-Timing` verified in DevTools with global mode
338360
(`serverTiming: true` / `OS_PERF_TIMING=1`) on, confirmed **absent** for a
339-
normal request, and confirmed the per-request `X-OS-Debug-Timing: 1` header
340-
returns timing **only** to an admin/service caller.
361+
normal request, confirmed the per-request `X-OS-Debug-Timing: 1` header
362+
returns timing **only** to an admin/service caller, and confirmed
363+
`X-OS-Debug-Timing: json` returns the `X-OS-Debug-Timing-Detail` payload
364+
**only** to an admin (never to an ordinary caller, even under global mode).

packages/observability/src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,12 @@ export {
5252
startServerTiming,
5353
measureServerTiming,
5454
countServerTiming,
55+
recordServerTimingDetail,
5556
runWithPerfDisclosure,
5657
allowPerfDisclosure,
5758
isPerfDisclosureAllowed,
59+
isPerfDisclosurePrivileged,
5860
type ServerTimingMark,
61+
type ServerTimingDetail,
5962
type PerfDisclosureGate,
6063
} from './perf-timing.js';

packages/observability/src/perf-timing.test.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import {
1313
runWithPerfDisclosure,
1414
allowPerfDisclosure,
1515
isPerfDisclosureAllowed,
16+
isPerfDisclosurePrivileged,
17+
recordServerTimingDetail,
1618
type PerfDisclosureGate,
1719
} from './perf-timing.js';
1820

@@ -213,11 +215,88 @@ describe('ambient collector', () => {
213215
});
214216
});
215217

218+
describe('detail capture', () => {
219+
it('is off by default — recordDetail is a no-op, details() empty', () => {
220+
const t = new PerfTiming();
221+
expect(t.detailEnabled).toBe(false);
222+
t.recordDetail('db', 'select * from x where id = ?', 5);
223+
expect(t.details('db')).toEqual([]);
224+
});
225+
226+
it('captures per-event samples once enabled', () => {
227+
const t = new PerfTiming();
228+
t.enableDetail();
229+
t.recordDetail('db', 'select a from t', 3);
230+
t.recordDetail('db', 'select b from t where id = ?', 9);
231+
const db = t.details('db');
232+
expect(db).toHaveLength(2);
233+
expect(db[1]).toEqual({ label: 'select b from t where id = ?', dur: 9 });
234+
});
235+
236+
it('keeps categories separate and coerces bad durations to 0', () => {
237+
const t = new PerfTiming();
238+
t.enableDetail();
239+
t.recordDetail('db', 'q', Number.NaN);
240+
t.recordDetail('hooks', 'h', -4);
241+
expect(t.details('db')).toEqual([{ label: 'q', dur: 0 }]);
242+
expect(t.details('hooks')).toEqual([{ label: 'h', dur: 0 }]);
243+
});
244+
245+
it('bounds retained samples at the cap', () => {
246+
const t = new PerfTiming();
247+
t.enableDetail();
248+
for (let i = 0; i < 1200; i++) t.recordDetail('db', `q${i}`, 1);
249+
expect(t.details('db').length).toBe(1000);
250+
});
251+
252+
it('recordServerTimingDetail folds onto the ambient collector only when enabled', async () => {
253+
const off = new PerfTiming();
254+
await runWithPerfTiming(off, async () => {
255+
recordServerTimingDetail('db', 'select 1', 2); // detail off → dropped
256+
});
257+
expect(off.details('db')).toEqual([]);
258+
259+
const on = new PerfTiming();
260+
on.enableDetail();
261+
await runWithPerfTiming(on, async () => {
262+
recordServerTimingDetail('db', 'select 1 where id = ?', 2);
263+
});
264+
expect(on.details('db')).toEqual([{ label: 'select 1 where id = ?', dur: 2 }]);
265+
});
266+
267+
it('recordServerTimingDetail is a no-op with no active collector', () => {
268+
recordServerTimingDetail('db', 'select 1', 1); // must not throw
269+
});
270+
});
271+
216272
describe('disclosure gate', () => {
217273
it('isPerfDisclosureAllowed() is false with no active gate', () => {
218274
expect(isPerfDisclosureAllowed()).toBe(false);
219275
});
220276

277+
it('privileged is false with no active gate, and allowPerfDisclosure sets both', () => {
278+
expect(isPerfDisclosurePrivileged()).toBe(false);
279+
const gate: PerfDisclosureGate = { allowed: false };
280+
runWithPerfDisclosure(gate, () => {
281+
expect(isPerfDisclosurePrivileged()).toBe(false);
282+
allowPerfDisclosure();
283+
expect(isPerfDisclosureAllowed()).toBe(true);
284+
expect(isPerfDisclosurePrivileged()).toBe(true);
285+
});
286+
expect(gate.allowed).toBe(true);
287+
expect(gate.privileged).toBe(true);
288+
});
289+
290+
it('global-mode disclosure (allowed but not privileged) does not grant privilege', () => {
291+
// The middleware seeds `{ allowed: true }` for global mode WITHOUT calling
292+
// allowPerfDisclosure — so basic timing discloses but detail stays gated.
293+
const gate: PerfDisclosureGate = { allowed: true };
294+
runWithPerfDisclosure(gate, () => {
295+
expect(isPerfDisclosureAllowed()).toBe(true);
296+
expect(isPerfDisclosurePrivileged()).toBe(false);
297+
});
298+
});
299+
221300
it('allowPerfDisclosure() is a no-op with no active gate (does not throw)', () => {
222301
allowPerfDisclosure();
223302
expect(isPerfDisclosureAllowed()).toBe(false);

packages/observability/src/perf-timing.ts

Lines changed: 109 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,21 @@ export interface ServerTimingMark {
4545
desc?: string;
4646
}
4747

48+
/**
49+
* One recorded sub-event when DETAIL capture is on (see
50+
* {@link PerfTiming.enableDetail}). Unlike the aggregate `db` mark — which folds
51+
* every query into a single count+duration — a detail sample keeps the
52+
* individual event so an admin can see *which* queries ran and which was
53+
* slowest. `label` is a description of the event (for SQL: the PARAMETRIZED
54+
* statement, bindings stripped — the query shape, never literal row values).
55+
*/
56+
export interface ServerTimingDetail {
57+
/** Event label — e.g. a parametrized SQL statement (no bindings). */
58+
label: string;
59+
/** Duration in milliseconds. */
60+
dur: number;
61+
}
62+
4863
/**
4964
* Monotonic millisecond clock. Prefers `performance.now()` (monotonic, not
5065
* affected by wall-clock adjustments); falls back to `Date.now()` on the rare
@@ -138,12 +153,64 @@ export class PerfTiming {
138153
* already inserted into {@link _marks}, mutated in place as events arrive.
139154
*/
140155
private _aggregates?: Map<string, { mark: ServerTimingMark; count: number; unit?: string }>;
156+
/**
157+
* Per-event detail samples by category, populated only while detail capture
158+
* is on (see {@link enableDetail}). Lazily created so a request that never
159+
* enables detail pays nothing.
160+
*/
161+
private _detail?: Map<string, ServerTimingDetail[]>;
162+
private _detailOn = false;
163+
/**
164+
* Hard cap on stored detail samples per category — detail is only ever on
165+
* for a deliberate debug request, but a pathological request must not pin
166+
* unbounded memory. The aggregate {@link count} still reflects the true
167+
* total; only the retained per-event list is bounded.
168+
*/
169+
private static readonly DETAIL_CAP = 1000;
141170

142171
/** Record an already-measured phase. */
143172
record(name: string, dur: number, desc?: string): void {
144173
this._marks.push({ name, dur, desc });
145174
}
146175

176+
/**
177+
* Turn on per-event DETAIL capture for this request. Off by default so the
178+
* hot path never allocates a per-event list; the HTTP middleware enables it
179+
* only for an admin-gated `X-OS-Debug-Timing: json` request. Idempotent.
180+
*/
181+
enableDetail(): void {
182+
this._detailOn = true;
183+
}
184+
185+
/** Whether per-event detail capture is on. */
186+
get detailEnabled(): boolean {
187+
return this._detailOn;
188+
}
189+
190+
/**
191+
* Record one per-event detail sample under `category` (e.g. `'db'`). A no-op
192+
* unless {@link enableDetail} was called, so the hot-path call site (the SQL
193+
* driver's query listener) pays only a boolean check when detail is off.
194+
* Bounded by {@link DETAIL_CAP}; excess events still count toward the
195+
* aggregate via {@link count} but are not retained individually.
196+
*/
197+
recordDetail(category: string, label: string, dur: number): void {
198+
if (!this._detailOn) return;
199+
const detail = (this._detail ??= new Map());
200+
let list = detail.get(category);
201+
if (!list) {
202+
list = [];
203+
detail.set(category, list);
204+
}
205+
if (list.length >= PerfTiming.DETAIL_CAP) return;
206+
list.push({ label: String(label), dur: Number.isFinite(dur) && dur > 0 ? dur : 0 });
207+
}
208+
209+
/** Retained detail samples for `category`, in record order (empty when none). */
210+
details(category: string): readonly ServerTimingDetail[] {
211+
return this._detail?.get(category) ?? [];
212+
}
213+
147214
/**
148215
* Begin timing a phase. Returns an idempotent `end()` - the first call
149216
* records the elapsed duration; later calls are ignored, so it is safe to
@@ -284,6 +351,16 @@ export function countServerTiming(name: string, dur: number, unit?: string): voi
284351
store.getStore()?.count(name, dur, unit);
285352
}
286353

354+
/**
355+
* Record a per-event DETAIL sample (e.g. one parametrized SQL statement) onto
356+
* the ambient collector — see {@link PerfTiming.recordDetail}. A no-op when no
357+
* collector is active OR detail capture is off, so the hot-path call site pays
358+
* only an `AsyncLocalStorage` lookup + a boolean check when not debugging.
359+
*/
360+
export function recordServerTimingDetail(category: string, label: string, dur: number): void {
361+
store.getStore()?.recordDetail(category, label, dur);
362+
}
363+
287364
// --- Disclosure gate (WHO may see the timing) -------------------------
288365

289366
/**
@@ -302,10 +379,23 @@ export function countServerTiming(name: string, dur: number, unit?: string): voi
302379
*
303380
* Keeping this out of {@link PerfTiming} preserves the collector's invariant
304381
* ("it only measures, it never decides whether to emit").
382+
*
383+
* Two levels, because global mode discloses the basic header to everyone but the
384+
* richer per-query detail must stay admin-only:
385+
* - `allowed` — the basic `Server-Timing` header may be disclosed (opened by
386+
* global mode for everyone, or by a proven admin per-request).
387+
* - `privileged` — the principal is a proven admin/service. Gates the richer,
388+
* SQL-shape-bearing detail payload, which must NEVER reach an
389+
* ordinary caller even when global mode is on.
305390
*/
306391
export interface PerfDisclosureGate {
307-
/** Whether the collected timing may be disclosed to the client. */
392+
/** Whether the basic collected timing may be disclosed to the client. */
308393
allowed: boolean;
394+
/**
395+
* Whether the principal is a proven admin/service — gates the richer detail
396+
* payload independently of `allowed`. Absent = not privileged.
397+
*/
398+
privileged?: boolean;
309399
}
310400

311401
/**
@@ -330,17 +420,30 @@ export function runWithPerfDisclosure<T>(gate: PerfDisclosureGate, fn: () => T):
330420
}
331421

332422
/**
333-
* Open the ambient disclosure gate — the request has proven it may see its own
334-
* `Server-Timing` header (admin/service identity). A no-op when no gate is
335-
* active (perf-tuning off, or already-global mode with no gate to flip), so the
336-
* call site stays branch-free.
423+
* Open the ambient disclosure gate — the request has proven an admin/service
424+
* identity, so it may see its own `Server-Timing` header AND the richer detail
425+
* payload. Sets both {@link PerfDisclosureGate.allowed} and `privileged`. A
426+
* no-op when no gate is active (perf-tuning off), so the call site stays
427+
* branch-free.
337428
*/
338429
export function allowPerfDisclosure(): void {
339430
const g = gateStore.getStore();
340-
if (g) g.allowed = true;
431+
if (g) {
432+
g.allowed = true;
433+
g.privileged = true;
434+
}
341435
}
342436

343437
/** Whether the ambient disclosure gate is open. `false` when none is active. */
344438
export function isPerfDisclosureAllowed(): boolean {
345439
return gateStore.getStore()?.allowed ?? false;
346440
}
441+
442+
/**
443+
* Whether the ambient principal is a proven admin/service — gates the richer
444+
* detail payload. `false` when no gate is active or only global-mode disclosure
445+
* (not a proven admin) opened it.
446+
*/
447+
export function isPerfDisclosurePrivileged(): boolean {
448+
return gateStore.getStore()?.privileged ?? false;
449+
}

packages/plugins/driver-sql/src/sql-driver-server-timing.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,31 @@ describe('SqlDriver Server-Timing db span', () => {
100100
});
101101
expect(dbMark(t)?.desc).toBe('1 queries');
102102
});
103+
104+
describe('detail mode (admin-gated per-query SQL shapes)', () => {
105+
it('records NO per-query detail when detail capture is off', async () => {
106+
const t = new PerfTiming(); // detail not enabled
107+
await runWithPerfTiming(t, async () => {
108+
await knexInstance('orders').where({ status: 'open' }).select('*');
109+
});
110+
expect(dbMark(t)?.desc).toMatch(/^\d+ queries$/); // aggregate still recorded
111+
expect(t.details('db')).toEqual([]); // …but no SQL shapes retained
112+
});
113+
114+
it('records parametrized SQL (no bindings) when detail capture is on', async () => {
115+
const t = new PerfTiming();
116+
t.enableDetail();
117+
await runWithPerfTiming(t, async () => {
118+
await knexInstance('orders').where({ status: 'open' }).select('*');
119+
});
120+
const detail = t.details('db');
121+
expect(detail.length).toBeGreaterThanOrEqual(1);
122+
const sql = detail[0].label;
123+
// Parametrized: carries a `?` placeholder, never the literal 'open'.
124+
expect(sql).toContain('?');
125+
expect(sql.toLowerCase()).toContain('select');
126+
expect(sql).not.toContain('open');
127+
expect(detail[0].dur).toBeGreaterThanOrEqual(0);
128+
});
129+
});
103130
});

0 commit comments

Comments
 (0)