Repository navigation
feat: wire metrics end to end, retire the Pages site, six executed tutorials - #44
Merged
Merged
Conversation
…cs true ## /metrics went from a constant to a scrape 1.1.0 shipped a metrics API and nothing called it. The Helm HPAs scaled `sync` on `connections` and `worker` on `queue_depth` — series nothing emitted, from pods that in the worker's case had no HTTP port at all. Three call sites, each in the package that owns the event. `recordRequest` sits in a `finally` around `execute` rather than after it: `execute` absorbs app throws, but a finalize stage can still throw on its own, and a `finally` is the only placement that counts every request exactly once. `recordConnection` hangs off `SocketRegistry`'s add/remove because that map is the only definition of a live connection on a node — and `sweepIdle` deleted from the map directly, which was the one path that leaked the gauge. `recordQueueDepth` is throttled to 15s: `stats()` aggregates the whole jobs table and a scrape reads it every ~15s anyway, so polling it per tick would multiply the queue's read load by sixty. `/metrics` is served by every role on `METRICS_PORT` (default 9090), not the role's HTTP port. The Helm ingress routes `/` `Prefix` to the web Service with no path exclusion, so mounting it beside `/healthz` publishes route patterns, request volumes and error rates to the internet. Nothing in the chart fronts 9090, so it is cluster-internal by construction rather than by an exclusion somebody has to remember to write. Every role rather than just `web`, because worker, scheduler and replicator open no HTTP socket and `queue_depth` belongs to the worker. Labels are route patterns, never concrete paths — a scanner hitting /wp-admin and /.env collapses to one `unmatched` series instead of minting one each. ## The site is gone; the wiki is the surface `site/`, `pages.yml` and the custom domain removed, repo homepage repointed. Three tests in scripts/verify.test.ts asserted the root tsconfig referenced ./site and that site/tsconfig.json emptied `paths` — they went with it. Axiom 6 for generated apps is unaffected: that is the `site/` -> `app/` boundary, a different thing with the same name, still enforced by `boundaries`. ## Six tutorials, executed rather than imagined Every command and every pasted output was run against the published 1.1.0 packages. That is also how eleven undocumented defects surfaced, the worst being that `x db gen` and `x db migrate` shell out to `bunx drizzle-kit` — which `x new` neither installs nor configures — so both fail in every scaffolded app, and `bin/setup` fails with them. They are named in wiki/Known-Gaps.md rather than left to be discovered. ## Docs that had drifted from the code wiki/Theming.md and the admin dev server both hardcoded the pre-retune palette, including the `line` value that measured 1.16:1 in dark; the admin server now derives from `colorTokens` through the same cached dynamic import `/_x` already uses, so it cannot drift again. `ROLE=all` was documented in three places and does not exist. `X_MIGRATE_CONCURRENT` was described as "takes no advisory lock" — the lock is real, the code simply has no throw site, because concurrent migrators wait rather than race. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7VehLeMPyirrfxLvu3Laf
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (106)
📝 WalkthroughWalkthroughThe change adds dedicated Prometheus metrics endpoints and runtime metric recording, removes the GitHub Pages site, and updates the 1.1.0 release documentation. It also adds six Wiki tutorials, observability guidance, deployment limitations, testing details, and theming documentation. ChangesRuntime observability
Release and documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HTTPPipeline
participant SocketRegistry
participant JobWorker
participant MetricsEndpoint
HTTPPipeline->>MetricsEndpoint: record request metrics
SocketRegistry->>MetricsEndpoint: record connection gauge
JobWorker->>MetricsEndpoint: record queue depth
MetricsEndpoint-->>MetricsEndpoint: serve Prometheus metrics
Possibly related PRs
Suggested labels: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1.
/metricswent from a constant to an actual scrape1.1.0 shipped a metrics API and nothing called it. The Helm HPAs scaled
synconconnectionsandworkeronqueue_depth— series nothing emitted, from pods that in the worker's case had no HTTP port at all.Three call sites, each in the package that owns the event:
recordRequestpipeline.ts, afinallyaroundexecuteexecuteabsorbs app throws, but a finalize stage can still throw on its own. Afinallyis the only placement that counts every request exactly oncerecordConnectionSocketRegistry.add/removesweepIdledeleted from the map directly — the one path that leaked the gaugerecordQueueDepthtick(), throttled 15sstats()aggregates the whole jobs table and a scrape reads it every ~15s anyway; per-tick would multiply the queue's read load by 60/metricsis on its own port (METRICS_PORT, default 9090), not the role's HTTP port. The Helm ingress routes/Prefixto the web Service with no path exclusion — mounting it beside/healthzwould publish your route patterns, request volumes and error rates to the internet. Nothing in the chart fronts 9090, so it is cluster-internal by construction rather than by an exclusion somebody has to remember to write. Every role, not justweb, because worker/scheduler/replicator open no HTTP socket andqueue_depthbelongs to the worker.Cardinality is guarded: labels are route patterns (
/posts/:id), never concrete paths, and a scanner hitting/wp-adminand/.envcollapses to oneunmatchedseries.Real scrape, not an assertion
Four distinct post ids → one series. Three sockets minus one abnormal close →
connections 2.2. The Pages site is gone; the wiki is the surface
site/,.github/workflows/pages.ymland the custom domain removed; repo homepage repointed. Three tests inscripts/verify.test.tsasserted the root tsconfig referenced./site— they went with it.Axiom 6 is unaffected: the
site/→app/boundary in a generated app is a different thing with the same name, still a build error.3. Six tutorials — executed, not imagined
Every command and every pasted output was run against the published 1.1.0 packages. That is how eleven undocumented defects surfaced. The worst:
Others:
generateMigrationmangles composite indexes; a two-statement migration cannot be applied; every generated entity fails typecheck, not just the example slice;x g taskwithout--featureemits a slice that cannot load. All named inwiki/Known-Gaps.mdwith tested workarounds.4. Docs that had drifted from the code
wiki/Theming.mdand the admin dev server both hardcoded the pre-retune palette — including thelinevalue that measured 1.16:1 in dark. The admin server now derives fromcolorTokens, so it cannot drift again.Theming.mdclaimed "nine tokens is the whole palette". There are 24 roles, and it pointed every path at a file that does not exist.ROLE=allwas documented in three places and does not exist — it throwsX_ROLE_UNKNOWNat boot.x status,x deploy static --to,x build --target docker --helmwere documented as shipped. None are.X_MIGRATE_CONCURRENTwas described as "ROLE=migratetakes no advisory lock, serialise them in your pipeline". The lock is real (pg_advisory_lock); the code has no throw site because concurrent migrators wait rather than race. That doc would have made someone build serialisation they already had.Gate
bun run verify— 17/17 green.🤖 Generated with Claude Code
https://claude.ai/code/session_01H7VehLeMPyirrfxLvu3Laf
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Documentation
Chores