Skip to content

Commit ce468c8

Browse files
os-zhuangclaude
andauthored
feat(observability): decompose Server-Timing into auth/db/hooks/serialize spans (perf-tuning mode) (#3107)
Completes #2408. The opt-in Server-Timing header now breaks a request's server time into the phases that actually explain it, so an operator can read the breakdown in DevTools -> Network -> Timing without standing up an external tracing backend. - collector: PerfTiming.count() / countServerTiming() fold high-frequency events (per query, per hook) into one aggregate member (name;dur=<sum>;desc="<count> <unit>"). - db: SqlDriver wires knex query / query-response events (keyed by __knexQueryUid), attributing per-query time to the originating request via AsyncLocalStorage (correct under concurrency; SQL text is never emitted). - auth: the dispatcher times identity/session resolution. - hooks: fed through the engine's existing HookMetricsRecorder seam, wired from the runtime so objectql's lean core tier stays observability-free. - serialize: the HTTP adapter times response JSON encoding. Fix a latent dual-instance bug found by a real end-to-end HTTP test: the ambient AsyncLocalStorage store was a plain module-level const, so the ESM and CJS builds (or an inlined bundle) each got their own store. A request scope opened by the HTTP server was then invisible to the SQL driver / engine, silently dropping the cross-layer spans. The store is now pinned to a global-registry symbol so every copy shares one. Every span is a no-op when perf-tuning is off (serverTiming / OS_SERVER_TIMING), so there is zero measurable overhead on the normal path. Gating is unchanged (env flag / option); only durations and counts are exposed, never SQL text. Claude-Session: https://claude.ai/code/session_01VoJNgMJRhgqm78xF7MxFhX Co-authored-by: Claude <noreply@anthropic.com>
1 parent 26e82b3 commit ce468c8

14 files changed

Lines changed: 512 additions & 12 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/observability": patch
3+
"@objectstack/driver-sql": patch
4+
"@objectstack/runtime": patch
5+
"@objectstack/plugin-hono-server": patch
6+
---
7+
8+
feat(observability): decompose `Server-Timing` into auth / db / hooks / serialize spans (perf-tuning mode)
9+
10+
The opt-in `Server-Timing` header now breaks a request's server time into the phases that actually explain it, so an operator can open DevTools → Network → Timing and see where the time went without standing up an external tracing backend:
11+
12+
- **`db`** — total SQL time with a **query count**. The SQL driver wires knex's `query` / `query-response` events (keyed by `__knexQueryUid`) and folds each query into one aggregate member (`db;dur=210;desc="6 queries"`) — the query count is the number most useful for spotting N sequential round-trips. Timing is attributed to the originating request via `AsyncLocalStorage`, so it is correct under concurrency and never cross-attributes. SQL text is never emitted, only durations and a count.
13+
- **`auth`** — identity / session resolution in the dispatcher, the prime suspect for unexplained data-API overhead.
14+
- **`hooks`** — total business-hook execution time with a hook count, fed through the engine's existing `HookMetricsRecorder` seam (wired from the runtime, so `@objectstack/objectql`'s lean `core` tier stays observability-free).
15+
- **`serialize`** — response JSON encoding in the HTTP adapter.
16+
17+
Adds `countServerTiming(name, dur, unit)` (and `PerfTiming.count`) to fold high-frequency phases into a single aggregate member instead of flooding the header. Every phase is a no-op when perf-tuning is off (`serverTiming: true` / `OS_SERVER_TIMING=true`), so there is zero measurable overhead on the normal path.
18+
19+
Closes #2408.

docs/OBSERVABILITY.md

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,15 @@ phases inline, which makes it trivial to see where wall-clock time went on a
229229
slow request without attaching a profiler.
230230

231231
```
232-
Server-Timing: total;dur=18.7;desc="Total server time", parse;dur=0.4;desc="Body parse", handler;dur=17.9;desc="Route handler"
232+
Server-Timing: parse;dur=0.4;desc="Body parse", auth;dur=42;desc="Identity/session", db;dur=210;desc="6 queries", hooks;dur=18;desc="3 hooks", serialize;dur=7;desc="Response serialize", handler;dur=280;desc="Route handler", total;dur=355;desc="Total server time"
233233
```
234234

235+
Reading it: the request spent 42ms resolving identity, 210ms across **6** SQL
236+
queries (the count is the number to watch — six sequential round-trips is the
237+
usual culprit behind an inexplicably slow list), 18ms in **3** business hooks,
238+
and 7ms serializing the response. `db` and `hooks` are aggregates — one member
239+
carrying the summed duration and the event count, not one member per query.
240+
235241
This is **off by default**: the header discloses internal phase durations,
236242
which is helpful for profiling but also lets a caller fingerprint the backend.
237243
Treat it as a perf-tuning toggle you flip in staging (or briefly in production
@@ -251,9 +257,23 @@ OS_SERVER_TIMING=true os serve
251257
```
252258

253259
When enabled, every response carries `total` (the whole request, measured by
254-
an outer middleware) plus any sub-phases the request recorded. The HTTP adapter
255-
contributes `parse` (request-body parsing) and `handler` (route-handler
256-
execution) out of the box.
260+
an outer middleware) plus the sub-phases the request path records out of the
261+
box:
262+
263+
| Member | Recorded by | Meaning |
264+
|:---|:---|:---|
265+
| `total` | Hono server middleware | Whole request, wall-clock. |
266+
| `parse` | HTTP adapter | Request-body parsing. |
267+
| `handler` | HTTP adapter | Route-handler execution. |
268+
| `serialize` | HTTP adapter | Response JSON encoding. |
269+
| `auth` | Dispatcher | Identity / session resolution — the prime suspect for unexplained data-API overhead. |
270+
| `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. |
271+
| `hooks` | ObjectQL engine | Total business-hook execution time; `desc` is the hook count. |
272+
273+
Each phase is recorded through a request-scoped collector that is a no-op when
274+
the mode is off, so every one of them costs nothing on the normal path. The
275+
`db` / `hooks` aggregates fold high-frequency events into a single member via
276+
`countServerTiming` (below) rather than emitting one member per event.
257277

258278
### Recording your own phases
259279

@@ -271,6 +291,16 @@ const rows = await measureServerTiming('db', () => engine.find(query), 'Primary
271291

272292
`startServerTiming(name)` (returns an `end()` callback) and
273293
`recordServerTiming(name, dur)` are also available for manual instrumentation.
294+
For a phase that fires many times per request (per query, per hook), use
295+
`countServerTiming(name, dur, unit)` — it folds every call into one aggregate
296+
member `name;dur=<sum>;desc="<count> <unit>"` instead of flooding the header:
297+
298+
```ts
299+
import { countServerTiming } from '@objectstack/observability';
300+
301+
// each call adds to the running total + count for `db`
302+
countServerTiming('db', queryMs, 'queries'); // → db;dur=<sum>;desc="<n> queries"
303+
```
274304

275305
## Go-live checklist
276306

packages/observability/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,6 @@ export {
5151
recordServerTiming,
5252
startServerTiming,
5353
measureServerTiming,
54+
countServerTiming,
5455
type ServerTimingMark,
5556
} from './perf-timing.js';

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
recordServerTiming,
1010
startServerTiming,
1111
measureServerTiming,
12+
countServerTiming,
1213
} from './perf-timing.js';
1314

1415
describe('formatServerTiming', () => {
@@ -102,6 +103,49 @@ describe('PerfTiming', () => {
102103
expect(t.marks()).toHaveLength(1);
103104
expect(t.marks()[0].name).toBe('boom');
104105
});
106+
107+
describe('count() aggregate', () => {
108+
it('folds repeated events into one mark carrying total + count', () => {
109+
const t = new PerfTiming();
110+
t.count('db', 10, 'queries');
111+
t.count('db', 5, 'queries');
112+
t.count('db', 3, 'queries');
113+
expect(t.marks()).toHaveLength(1);
114+
expect(t.toHeader()).toBe('db;dur=18;desc="3 queries"');
115+
});
116+
117+
it('keeps the aggregate at its first-seen position (before a later total)', () => {
118+
const t = new PerfTiming();
119+
t.count('db', 4, 'queries');
120+
t.record('total', 20, 'Total server time');
121+
t.count('db', 6, 'queries'); // still folds into the first db mark
122+
expect(t.marks().map((m) => m.name)).toEqual(['db', 'total']);
123+
expect(t.toHeader()).toBe('db;dur=10;desc="2 queries", total;dur=20;desc="Total server time"');
124+
});
125+
126+
it('tracks independent names separately', () => {
127+
const t = new PerfTiming();
128+
t.count('db', 10, 'queries');
129+
t.count('hooks', 2, 'hooks');
130+
t.count('hooks', 3, 'hooks');
131+
expect(t.toHeader()).toBe('db;dur=10;desc="1 queries", hooks;dur=5;desc="2 hooks"');
132+
});
133+
134+
it('emits a bare count when no unit is given', () => {
135+
const t = new PerfTiming();
136+
t.count('x', 1);
137+
t.count('x', 1);
138+
expect(t.toHeader()).toBe('x;dur=2;desc="2"');
139+
});
140+
141+
it('ignores non-finite / negative durations but still counts the event', () => {
142+
const t = new PerfTiming();
143+
t.count('db', Number.NaN, 'queries');
144+
t.count('db', -5, 'queries');
145+
t.count('db', 7, 'queries');
146+
expect(t.toHeader()).toBe('db;dur=7;desc="3 queries"');
147+
});
148+
});
105149
});
106150

107151
describe('ambient collector', () => {
@@ -113,10 +157,34 @@ describe('ambient collector', () => {
113157
recordServerTiming('x', 1); // must not throw
114158
const end = startServerTiming('y');
115159
end(); // must not throw
160+
countServerTiming('db', 1, 'queries'); // must not throw
116161
const v = await measureServerTiming('z', async () => 7);
117162
expect(v).toBe(7);
118163
});
119164

165+
it('countServerTiming folds onto the ambient collector', async () => {
166+
const t = new PerfTiming();
167+
await runWithPerfTiming(t, async () => {
168+
countServerTiming('db', 4, 'queries');
169+
await new Promise((r) => setTimeout(r, 1));
170+
countServerTiming('db', 6, 'queries'); // after an await — same ALS scope
171+
});
172+
expect(t.toHeader()).toBe('db;dur=10;desc="2 queries"');
173+
});
174+
175+
it('pins the ambient store to a global-registry symbol (shared across module copies)', () => {
176+
// The store MUST live on globalThis under Symbol.for so that a second
177+
// copy of this module (ESM vs CJS build, or an inlined bundle) shares it
178+
// — otherwise cross-layer spans (db/auth/hooks recorded from the SQL
179+
// driver / engine) never reach the collector the HTTP server opened.
180+
const key = Symbol.for('@objectstack/observability:perf-timing-store');
181+
expect((globalThis as Record<symbol, unknown>)[key]).toBeDefined();
182+
// And the ambient free functions must read THAT store.
183+
const t = new PerfTiming();
184+
runWithPerfTiming(t, () => recordServerTiming('shared', 1));
185+
expect(t.marks().map((m) => m.name)).toContain('shared');
186+
});
187+
120188
it('records onto the ambient collector inside runWithPerfTiming', async () => {
121189
const t = new PerfTiming();
122190
await runWithPerfTiming(t, async () => {

packages/observability/src/perf-timing.ts

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@
1717
* 2. **Ambient collector.** Run a request inside {@link runWithPerfTiming}
1818
* and any framework code on that async call chain records phases via the
1919
* free functions ({@link measureServerTiming}, {@link startServerTiming},
20-
* {@link recordServerTiming}) without threading the request object
21-
* through every layer. When no collector is active the free functions are
22-
* cheap no-ops, so call sites pay nothing when the feature is off.
20+
* {@link recordServerTiming}, {@link countServerTiming}) without threading
21+
* the request object through every layer. When no collector is active the
22+
* free functions are cheap no-ops, so call sites pay nothing when the
23+
* feature is off. High-frequency phases (per SQL query, per hook) use
24+
* {@link countServerTiming} to fold into one aggregate mark carrying a
25+
* total duration and an event count.
2326
*
2427
* `Server-Timing` exposes internal phase durations to any client, which is a
2528
* (mild) information-disclosure surface - it helps an attacker profile the
@@ -129,6 +132,12 @@ export function formatServerTiming(marks: readonly ServerTimingMark[]): string {
129132
*/
130133
export class PerfTiming {
131134
private readonly _marks: ServerTimingMark[] = [];
135+
/**
136+
* Live aggregate marks by name (see {@link count}). Lazily created so a
137+
* request that never aggregates pays nothing. Each entry points at a mark
138+
* already inserted into {@link _marks}, mutated in place as events arrive.
139+
*/
140+
private _aggregates?: Map<string, { mark: ServerTimingMark; count: number; unit?: string }>;
132141

133142
/** Record an already-measured phase. */
134143
record(name: string, dur: number, desc?: string): void {
@@ -160,6 +169,38 @@ export class PerfTiming {
160169
}
161170
}
162171

172+
/**
173+
* Accumulate a repeated sub-phase into a SINGLE aggregate mark. Each call
174+
* adds `dur` to the running total for `name` and increments a counter; the
175+
* mark serializes as `name;dur=<sum>;desc="<count> <unit>"` (or just the
176+
* bare count when no `unit` is given).
177+
*
178+
* Use this for high-frequency phases — one SQL query, one hook execution —
179+
* where recording a distinct mark per event would blow the header out to
180+
* hundreds of entries. The single `db;dur=210;desc="6 queries"` member is
181+
* both the total DB time and the query count, which is the number most
182+
* useful for spotting N sequential round-trips.
183+
*
184+
* The aggregate mark is inserted into the record stream the first time its
185+
* name is seen, so it keeps its natural position relative to explicit marks
186+
* (e.g. before the outer `total`, which is recorded last).
187+
*/
188+
count(name: string, dur: number, unit?: string): void {
189+
const add = Number.isFinite(dur) && dur > 0 ? dur : 0;
190+
const aggregates = (this._aggregates ??= new Map());
191+
let entry = aggregates.get(name);
192+
if (!entry) {
193+
const mark: ServerTimingMark = { name, dur: 0 };
194+
entry = { mark, count: 0, unit };
195+
aggregates.set(name, entry);
196+
this._marks.push(mark);
197+
}
198+
entry.count += 1;
199+
entry.mark.dur += add;
200+
if (unit) entry.unit = unit;
201+
entry.mark.desc = entry.unit ? `${entry.count} ${entry.unit}` : String(entry.count);
202+
}
203+
163204
/** Snapshot of recorded marks, in record order. */
164205
marks(): readonly ServerTimingMark[] {
165206
return this._marks;
@@ -173,7 +214,25 @@ export class PerfTiming {
173214

174215
// --- Ambient (request-scoped) collector -------------------------------
175216

176-
const store = new AsyncLocalStorage<PerfTiming>();
217+
/**
218+
* The ambient collector lives in ONE process-wide `AsyncLocalStorage`, pinned
219+
* to a global-registry symbol rather than a plain module-level `const`.
220+
*
221+
* Why: this module is consumed from many packages and can legitimately be
222+
* loaded more than once in a single process — the ESM build (`dist/index.js`)
223+
* and the CJS build (`dist/index.cjs`) are distinct module instances, and a
224+
* bundler may inline yet another copy. A plain `const store` would give each
225+
* copy its OWN store, so a request scope opened through one copy (the HTTP
226+
* server's `runWithPerfTiming`) would be invisible to code reading the ambient
227+
* collector through another copy (the SQL driver, the ObjectQL engine) — the
228+
* cross-layer `db` / `auth` / `hooks` spans would silently never record.
229+
* `Symbol.for` resolves to the same symbol across every copy, so they all share
230+
* the one store.
231+
*/
232+
const STORE_KEY = Symbol.for('@objectstack/observability:perf-timing-store');
233+
const globalStore = globalThis as unknown as Record<symbol, AsyncLocalStorage<PerfTiming> | undefined>;
234+
const store: AsyncLocalStorage<PerfTiming> =
235+
globalStore[STORE_KEY] ?? (globalStore[STORE_KEY] = new AsyncLocalStorage<PerfTiming>());
177236

178237
/** Run `fn` with `timing` as the ambient collector for the async call chain. */
179238
export function runWithPerfTiming<T>(timing: PerfTiming, fn: () => T): T {
@@ -214,3 +273,13 @@ export async function measureServerTiming<T>(
214273
if (!t) return fn();
215274
return t.measure(name, fn, desc);
216275
}
276+
277+
/**
278+
* Accumulate a repeated sub-phase (one SQL query, one hook execution) onto the
279+
* ambient collector — see {@link PerfTiming.count}. A no-op when no collector is
280+
* active, so the hot-path call sites (the SQL driver's query listener, the hook
281+
* runner) pay only a single `AsyncLocalStorage` lookup when perf-tuning is off.
282+
*/
283+
export function countServerTiming(name: string, dur: number, unit?: string): void {
284+
store.getStore()?.count(name, dur, unit);
285+
}

packages/plugins/driver-sql/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
},
2020
"dependencies": {
2121
"@objectstack/core": "workspace:*",
22+
"@objectstack/observability": "workspace:*",
2223
"@objectstack/spec": "workspace:*",
2324
"@objectstack/types": "workspace:*",
2425
"knex": "^3.3.0",

0 commit comments

Comments
 (0)