Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions .changeset/http-request-errors-total-retired.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
---
"@objectstack/observability": minor
"@objectstack/runtime": minor
"@objectstack/spec": minor
---

fix(observability): **BREAKING** — `http_request_errors_total` is retired (ADR-0049 enforce-or-remove, #9834)

**⛔ If you have a Grafana panel, an alert rule or a recording rule keyed on
`http_request_errors_total`, it will read a FLAT ZERO after this upgrade.** That
zero is the removal, not a healthy server, and it is the one way this change can
hurt you — nothing throws, nothing warns, the series simply stops receiving
samples. Rewrite the query before you deploy.

Maintainer ruling 2026-08-20: **RETIRE**. The name was declared in `SEMCONV` as
part of a stable namespace *"so hosts can wire alerts/dashboards against it"*,
but the only emitter was `@objectstack/runtime`'s `instrumentRouteHandler`,
applied only by the dispatcher's own route Proxy — so the series never saw
auth's `getRawApp()` mount, the REST data API via `RouteManager`, or any other
inbound surface. Its two siblings in the same HTTP family moved to the
`IHttpServer.afterResponse` transport seam (`http_requests_total`, #9650/#9835;
`http_request_duration_ms`, #9834/#10004) and this one could not follow:
`HttpResponseObservation` carries `{method, routePattern, status, elapsedMs}`
and **no throw signal of any kind**, so every transport-side shape would have
counted a *different* population rather than the same one more widely.

Migration (FROM → TO):

| Wrote | Write instead |
|---|---|
| `rate(http_request_errors_total[5m])` in a panel or alert | `rate(http_requests_total{status=~"5.."}[5m])` — emitted by the transport, so it covers every inbound surface instead of the dispatcher's routes only |
| `sum by (route) (http_request_errors_total)` | `sum by (route) (http_requests_total{status=~"5.."})` |
| `SEMCONV.httpRequestErrorsTotal` / `RUNTIME_METRICS.httpRequestErrorsTotal` in host code | Delete the read. Both members are gone; `tsc` reports the missing property at the read site. |

One-line fix: replace the metric name with `http_requests_total{status=~"5.."}`.

<!-- adr-0087: registered http-request-errors-total-retired -->

**The replacement is wider, not merely different.** The retired counter was
divergent from a 5xx rate in *both* directions, measured: the dispatcher answers
its own errors through `errorResponseBase`, which sets a status and does **not**
re-throw — so the counter **missed** those — while its `catch` incremented
unconditionally, so a **thrown 4xx WAS counted** as an error. And
`http_requests_total` already carries a `status` label, so a status-class error
counter was fully derivable from data the transport already publishes. Prove the
new query wider rather than merely non-empty: make an auth route or a REST
data-API route answer 5xx and confirm it moves, where the retired counter would
not have moved at all.

**If what you were actually alerting on was "a handler threw rather than
returning an error envelope"** — the one signal this counter uniquely carried —
that is the `errorReporter`, not a metric. Wire an `ErrorReporter` adapter
(Sentry / Datadog / your own); it still fires on every 5xx throw and is
untouched by this change.

What is NOT removed: `http_requests_total`, `http_request_duration_ms`,
request-id propagation, the 5xx error reporter, and the
`res.__obsRecordedError` side channel that carries a swallowed error to it. The
dispatcher still instruments every route it mounts; it just no longer publishes
a fourth series whose name promised more coverage than it had.
12 changes: 10 additions & 2 deletions content/docs/deployment/production-readiness.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -55,8 +55,7 @@ That's it — every route the dispatcher mounts now:

1. Sends conservative security headers.
2. Echoes `X-Request-Id` (honored from caller, or freshly minted).
3. Emits the `http_request_errors_total` counter on a thrown handler.
4. Reports 5xx exceptions to Sentry (or your reporter) with the
3. Reports 5xx exceptions to Sentry (or your reporter) with the
request id attached.

The `http_requests_total{method,route,status}` counter and the
Expand All@@ -73,6 +72,15 @@ is the registered pattern (`/api/v1/data/:id`), never the concrete path.
(middleware chain and body parse included), not the handler alone — so p95 is
the latency a caller experiences minus the network.

**`http_request_errors_total` was retired in 17.2.0** (#9834). Nothing declares
or emits it any more, so a dashboard keyed on that name reads a flat zero —
that is the removal, not a healthy server. **Alert on
`http_requests_total{status=~"5.."}` instead**: the transport emits it for every
inbound surface, whereas the retired counter only ever saw the dispatcher's own
routes and only when a handler *threw*, missing every error answered politely
with a status. Unhandled exceptions specifically still reach the
`errorReporter`, which is the signal that counter was mistaken for.

**A transport that does not implement the seam reports no HTTP metrics**: zero
on `http_requests_total` there means "not instrumented", never "no traffic".
The shipped Hono adapter implements it; verify a custom adapter with
Expand Down
20 changes: 17 additions & 3 deletions docs/OBSERVABILITY.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,7 +9,6 @@
`createDispatcherPlugin` automatically instruments every route it mounts with:

- **Request id** propagation: honors incoming `X-Request-Id` (or mints `req_<uuid>`); echoes on the response.
- **`http_request_errors_total{method,route}`** counter (incremented on thrown errors).
- **Error reporting** for 5xx (handler-thrown or via `errorResponseBase` side channel).

**`http_requests_total{method,route,status}`** (1 per request) and
Expand DownExpand Up@@ -75,9 +74,23 @@ import { RUNTIME_METRICS } from '@objectstack/runtime';

RUNTIME_METRICS.httpRequestsTotal // 'http_requests_total'
RUNTIME_METRICS.httpRequestDurationMs // 'http_request_duration_ms'
RUNTIME_METRICS.httpRequestErrorsTotal // 'http_request_errors_total'
```

> **⛔ `http_request_errors_total` was RETIRED in 17.2.0 (#9834).** It is no
> longer declared in `SEMCONV`/`RUNTIME_METRICS` and nothing emits it — a panel
> or alert keyed on that name reads flat zero, which is the removal, not an
> outage.
>
> **Read the 5xx rate from `http_requests_total{status=~"5.."}` instead**, which
> the transport emits for *every* inbound surface. The retired counter was
> declared as a server-wide error signal but incremented only from the
> dispatcher's per-route wrapper, and only when a handler **threw** — so it
> missed auth, the REST data API and every error a handler answered politely
> through `errorResponseBase`, while counting thrown 4xx as errors. The
> replacement query is both wider and better defined. If what you actually want
> is the *unhandled-exception* rate rather than the 5xx rate, that signal is the
> `errorReporter` (below), which still fires on every 5xx throw.

### `cache_*` — a flat zero means "no configured consumer"

The `@objectstack/service-cache` adapters emit `cache_lookups_total`,
Expand DownExpand Up@@ -411,7 +424,8 @@ countServerTiming('db', queryMs, 'queries'); // → db;dur=<sum>;desc="<n> queri
- [ ] Verified 4xx does **not** flood the APM.
- [ ] Log records include `requestId` field; cross-checked one against the
response `X-Request-Id` header.
- [ ] Alerts wired: error rate, p95 latency per route.
- [ ] Alerts wired: error rate (`http_requests_total{status=~"5.."}` — **not**
the retired `http_request_errors_total`, #9834), p95 latency per route.
- [ ] (Optional) `Server-Timing` verified in DevTools with global mode
(`serverTiming: true` / `OS_PERF_TIMING=1`) on, confirmed **absent** for a
normal request, confirmed the per-request `X-OS-Debug-Timing: 1` header
Expand Down
3 changes: 3 additions & 0 deletions docs/protocol-upgrade-guide.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -469,6 +469,9 @@ No mechanical rewrite exists, in either direction. The refused values carry no r

This is a RUNTIME registration API, not stored metadata, so — like `hook-context-session-roles-retired` at this step — there is no `sys_metadata` row for the D2 chain to rewrite and the ledger entry is the notification channel. One metadata surface reaches it INDIRECTLY and is the reason this is not purely a code-side note: a `record-change` flow's start node forwards `config.objectName` verbatim into `registerHook` (`RecordChangeTrigger.start`), so a flow authored with a blank `objectName` used to bind a trigger to EVERY object in the tenant. It now fails to bind instead, loudly — the automation engine's per-flow bind guard warns and the `kernel:bootstrapped` binding audit re-reports it — which is the correct end state, but it is an observable change for that flow. #6573, #4281, #4001, #5928, ADR-0078.
- Done when: No `registerHook` call site passes an empty `object` target, and none passes an `excludeObjects` list covering every name in its `object` list. Every `record-change` flow start node declares a non-blank `config.objectName`, or omits the key if the flow is genuinely meant to fire on every object. Boot completes with no "[ObjectQL] Hook ... declares an empty `object` target" throw and no "[record-change] ... not bound" warning naming a flow you expect to fire.
- **`http-request-errors-total-retired`** — `observability.SEMCONV.httpRequestErrorsTotal (the published metric name http_request_errors_total{method,route}, and its emission from the runtime dispatcher's per-route wrapper)` → the 5xx rate is `http_requests_total{status=~"5.."}` — the TRANSPORT emits that family through the `IHttpServer.afterResponse` seam, so it covers every inbound surface; unhandled-exception rate specifically, which is the one thing the retired counter uniquely reported, is the `errorReporter` (Sentry / Datadog / your adapter), which still fires on every 5xx throw
- Why not automatic: ADR-0049 enforce-or-remove, on a DECLARED-not-enforced metric name. `SEMCONV` published `http_request_errors_total` as part of a stable namespace declared "so hosts can wire alerts/dashboards against it", but the only emitter was `@objectstack/runtime`'s `instrumentRouteHandler`, applied only by the dispatcher's own route Proxy — so the series never saw auth's `getRawApp()` mount, the REST data API via `RouteManager`, or any other inbound surface. Its two siblings in the same family were moved to the transport seam (#9650/#9835 for the counter, #9834/#10004 for the histogram) and this one could not follow: `HttpResponseObservation` carries `{method, routePattern, status, elapsedMs}` and NO throw signal of any kind, so every transport-side shape would have counted a DIFFERENT population rather than the same one more widely. The divergence was measured in both directions — the dispatcher answers its own errors through `errorResponseBase`, which sets a status and does not re-throw, so the old counter MISSED those, while its `catch` incremented unconditionally, so a thrown 4xx WAS counted as an error. And `http_requests_total` already carries a `status` label, so a status-class error counter would be fully derivable from data the transport already publishes. Maintainer ruling 2026-08-20 (option C of four presented, over B "move it to the transport as a status class" and D "keep it dispatcher-scoped and rename it"): RETIRE. A metric NAME is a RESPONSE surface, not authorable metadata — no stack, example or template carries it, so there is no source for a D2 conversion to rewrite and no schema to tombstone; a host names the series in its own dashboard or alert file, outside this repo. That is exactly why this entry exists: for an operator whose Grafana keys on the string, the ledger is the only notification channel there is. Same disposition, and the same reason, as `runtime-httpserver-wrapper-retired` (#5122) and `enhanced-api-error-field-errors-renamed` (#3977). ADR-0049 / ADR-0087, #9834.
- Done when: No dashboard, alert rule or exporter config names `http_request_errors_total`: the series stops receiving samples the moment 17.2.0 is deployed, so a panel keyed on it draws a flat zero that reads as a healthy server rather than as a removed metric — the one failure mode this retirement can produce, and the reason the changeset announces it loudly. A 5xx-rate panel or alert is rewritten to `http_requests_total{status=~"5.."}` and then PROVEN wider, not merely non-empty: make an auth route or a REST data-API route answer 5xx and confirm the new query moves, where the retired counter would not have moved at all. If the signal you were actually alerting on was "a handler threw rather than returning an error envelope", that is the `errorReporter`, not a counter — wire an APM adapter and assert one synthetic 5xx throw arrives. In code, `SEMCONV.httpRequestErrorsTotal` and `RUNTIME_METRICS.httpRequestErrorsTotal` no longer resolve (tsc reports TS2339 at any surviving read) and no `metrics.counter` call names the string.
- **`http-server-runtime-vocabulary-retired`** — `system.serverEvent / system.serverEventType / system.serverCapabilities / system.serverStatus (the lifecycle-event, capability-report and status vocabulary of system/http-server.zod.ts — 4 defs, 8 exported names)` → (removed — there is no replacement key, because there was never a key. Server lifecycle is the transport plugin's own start/stop seam; per-request and per-server observability is `system/metrics.zod.ts` and `system/logging.zod.ts` (plus `OS_SERVER_TIMING` for timings), and liveness is the `/health` endpoint. What a transport plugin can DO it states by implementing the kernel plugin contract — the seams it registers are the capability statement, and a self-described capability record can only disagree with them. Server-level configuration that IS authorable lives on `defineStack({ server })` / `StackServerConfigSchema`, which is unaffected)
- Why not automatic: The second and final ADR-0049 pass over `system/http-server.zod.ts`. #4938 removed the CONFIG half (`HttpServerConfigSchema`, nine keys, zero readers, zero authoring entry); this removes the RUNTIME half — a 7-member lifecycle event union with a timestamped envelope, an eight-boolean capability report, and a five-state status record with connection and request counters. Nothing ever emitted, consumed or parsed any of them. This card was HELD for four days rather than queued, on a specific and legitimate doubt: a response/capability vocabulary can be a REFERENCE surface for host implementers, so "zero consumers in this repo" is weaker evidence for one of those than for an authorable key (the CSS-variable rebuttal). The hold was lifted by measuring the reference reader itself rather than by re-running the same grep: `plugin-hono-server`, the one in-tree host implementation, neither implements nor reports any of the three — it names no capability record, no status shape and no event union, and what it registers is routes and middleware through the kernel plugin contract. A declaration-site grep put every declaration in this one file, a quoted-name sweep across objectstack and objectui found no reader outside it, and the control passed in the SAME run: `MiddlewareConfig`, declared twelve lines away, resolves to `packages/runtime/src/middleware.ts`. So the sweep could see a reader in this file when there was one. With no carrier key there is nothing to tombstone, and with no author there is no source or `sys_metadata` row for a D2 conversion to rewrite: RETIRED_DEFS_BY_MAJOR plus this entry are the declaration — route 3, the same shape as #4938 in this very file, #4834, #4988 and #5055. If host-implementer conformance becomes a real requirement it returns through the ENFORCE route: an adapter contract with a checker behind it, vocabulary second. ADR-0049, #5295.
- Done when: No source imports `ServerEvent`, `ServerEventType`, `ServerEventSchema`, `ServerCapabilities`, `ServerCapabilitiesSchema`, `ServerCapabilitiesParsed`, `ServerStatus` or `ServerStatusSchema` from `@objectstack/spec/system` — a grep over consumer code resolves none of them, and `tsc` reports TS2724/TS2305 on any that survives. The route-registration half of the same module still resolves (`RouteHandlerMetadataSchema`, `MiddlewareType`, `MiddlewareConfigSchema`, `MiddlewareConfig`), and `StackServerConfigSchema` — the one authorable server surface — is untouched: a stack declaring `server: { trustProxy, security }` parses exactly as it did in 16.x.
Expand Down
38 changes: 38 additions & 0 deletions packages/observability/src/__tests__/semconv.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SEMCONV, RUNTIME_METRICS } from '../semconv.js';

describe('SEMCONV', () => {
it('declares the two HTTP families the transport seam emits', () => {
expect(SEMCONV.httpRequestsTotal).toBe('http_requests_total');
expect(SEMCONV.httpRequestDurationMs).toBe('http_request_duration_ms');
});

/**
* The retirement pin for `http_request_errors_total` (#9834, ADR-0049
* enforce-or-remove, maintainer ruling 2026-08-20).
*
* It is asserted HERE, on the declaration, rather than only on the emission
* site in `@objectstack/runtime`: `SEMCONV` is the published namespace the
* docs point hosts at "so hosts can wire alerts/dashboards against it", so
* the name being back in this object is itself the regression — a host
* reading the constant would start naming a series nothing writes, which is
* the declared-not-enforced shape the retirement removed.
*
* Both directions are checked on purpose. The key check catches a re-add
* under the old member name; the VALUE check catches a re-add under a new
* member name that resurrects the same wire string, which is what an
* external dashboard actually keys on.
*/
it('does not declare the retired http_request_errors_total name (#9834)', () => {
expect(SEMCONV as Record<string, string>).not.toHaveProperty(
'httpRequestErrorsTotal',
);
expect(Object.values(SEMCONV) as string[]).not.toContain('http_request_errors_total');
expect(RUNTIME_METRICS as Record<string, string>).not.toHaveProperty(
'httpRequestErrorsTotal',
);
expect(Object.values(RUNTIME_METRICS) as string[]).not.toContain('http_request_errors_total');
});
});
Loading
Loading