Skip to content

logging: log /admin/stats as its own route, status only (TC-1482) - #372

Merged
samgbafa merged 4 commits into
feat/tc-1482-deploy-wiringfrom
feat/tc-1482-log-route
Oct 19, 2026
Merged

samgbafa merged 4 commits into
feat/tc-1482-deploy-wiringfrom
feat/tc-1482-log-route

Conversation

@samgbafa

@samgbafa samgbafa commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Stacking

This PR is stacked on #371 (feat/tc-1482-deploy-wiring, head 03810e2944bd55f45124d07b43931fb262f895f1), which merges first. The stack base is unchanged: the base branch is still #371's, and #371's head is merged into this branch (merge commit ed81bd19; the branch held fe16c319 before, and #371 had moved on to 03810e29). The base is #371's branch so the diff shows only the logging change. After #371 merges, retarget the base to main. This PR contains a merge of #371's head and resolves the shared lock files so both changes are present.

Change

GET /admin/stats (#370) is logged as its own route, route template plus status only. Refs TC-1482. Production logs are public (TC-961), so no query string, Authorization header or token, request or response body, client address, space ID, DID or key is logged for it.

Re-pin after the tinycloud-dev PR merges (separate from the retarget)

This PR stays a draft until tinycloud-dev #163 merges.

  1. After tinycloud-dev perf(TC-286): bound Prometheus route-label cardinality #163 merges, re-pin canonical.json revision to its merge commit on main. sha256 stays cff33bf50586d8a4425ca1584f125bd891380b74eef76de8e5da690d442ce6d8 while ops/logging/fields.json is unchanged by review.
  2. If review changes ops/logging/fields.json, copy it byte-for-byte to log-shipper/fields.json, update sha256, regenerate the embedded VECTOR_CONFIG_JSON (compose and fixture), and move the N0 template lock again.

Other open PRs

#370 touches CHANGELOG.md and docs/releasing.md (textual overlap only). #353, #356 and #357 touch CHANGELOG.md only, #359 touches none of these files. #342 and #288 are older open PRs that touch log-shipper/* and the compose.

Verification (no Docker, no deploy)

  • Native Vector 0.59.0 (the CI tarball, checksum verified): vector test for errors_slow and all_requests, 95 cases each (92 + the three current_kv_c_index_* cases), 0 failed; vector validate of the rendered runtime config passes; log-shipper/native-check.mjs passes with 0 canary hits.
  • node --test for budget, profile, tc959-docker, tc959-readiness: 33 pass. scripts/ci/test-deploy-recovery.py: 25 pass (after merging TC-1482: wire optional NODE_STATS_READ_TOKEN into the production deploy (companion to #370) #371's head).
  • Pin and parity checks, replicated without Docker: the sha256 matches, the vendored bytes equal the canonical file at the pinned tinycloud-dev commit, the embedded Vector config equals the compiled config, the compose keeps the NODE_STATS_READ_TOKEN line, and the compose equals the fixture. Mutations: pointing current_kv_c_index_invalid at unclassified fails its Vector case; removing an enum entry from fields.json fails node --test and the pin hash. The lock in docker.yml equals the template hash computed with the workflow's own normalization.
  • Ingress: nginx -t passes on a local nginx 1.18 with redacted-logging.conf. With a stub upstream returning a body, /admin/stats?token=… with an Authorization header logs only {"method":"GET","route":"/admin/stats","status":401,…}. The token, Bearer, query, address, User-Agent and upstream body do not appear. /admin/stats/extra logs <other>.
  • Not run: scripts/tc959-check.mjs itself (needs docker compose config), deploy/ingress/test-redacted-logging.sh (Docker), the hosted compose proof (CI).

The /admin/stats ingress label reaches production only when the ingress image is rebuilt from deploy/ingress and re-pinned.

dryrun added 2 commits October 19, 2026 13:17
Adds the /admin/stats route label to the log-shipper field list (copy of the
tinycloud-dev canonical list), the ingress route map and the Vector boundary
cases. Re-pins log-shipper/canonical.json to the tinycloud-dev PR head, so the
pin MUST be updated after that PR merges. Regenerates the embedded Vector
config in the N0 compose and its fixture, and moves the N0 template lock.
Route template plus status only; no query, credential, body or address (TC-961).
# Conflicts:
#	.github/workflows/docker.yml
#	docs/releasing.md
#	docs/runbooks/tc-959-n0-log-shipping.md
@samgbafa
samgbafa changed the base branch from main to feat/tc-1482-deploy-wiring October 19, 2026 13:22
@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-1482 (reviewed exact head 2dc8e4b)

Independent review (Opus gate) of the logging delta fe16c319..2dc8e4b3 at exact head 2dc8e4b389d218eb9339bf07cfcb8aec8d3ab64f. The draft is stacked on #371.

Combined hashes, recomputed with docker.yml's n0_template method, with both the NODE_STATS_READ_TOKEN line and the regenerated VECTOR_CONFIG_JSON present:

  • raw d1fcd66534e3131c691935b7b1dc774d93f8b1e75dab52b178cafba59b2cbdbf, template 87b5d12a86f07f33ae68832c802894c061a6b4e194aea078cb5eb95a068719aa.
  • The fixture is byte-identical. docker.yml:672, releasing.md:344-345/414, runbook:150 and CHANGELOG all agree; no stale 8f67b3b2/ae497fed remains.
  • The decoded Vector config diff is only "/admin/stats" added to the two route allowlists. tc959-check.mjs confirms embedded/generator parity.
  • log-shipper/fields.json is byte-identical to tinycloud-dev 861f0be6 (sha e1cd16e9…154ec15).

TC-961, route + status only:

  • With the checksum-pinned Vector 0.59.0 (no Docker): vector test 92/92 for errors_slow and 92/92 for all_requests. native-check reports wire_safe with 0 canary hits; vector validate OK.
  • The new node and ingress cases assert the exact output key set, with a canary (bearer/email/did:pkh/address) in every extra field. The global safe assert also covers this.
  • The route /admin/stats?token=… is rejected and counted, never shipped.
  • Ingress: one map line; log_format has no $request/$uri/$args/$http_*/body variables. ?x=1 → /admin/stats; /admin/stats/extra → <other>. The ingress label is live only after the ingress image is rebuilt and its digest pinned (CHANGELOG says so); until then it logs <other>, which is safe.
  • Info: VRL warning E620 on keys!(.) (use keys(.)).

Merge-order precondition:

  1. tinycloud-dev perf(TC-286): bound Prometheus route-label cardinality #163 merges first.
  2. Re-pin log-shipper/canonical.json revision from the PR head 861f0be6 to the perf(TC-286): bound Prometheus route-label cardinality #163 merge commit on tinycloud-dev main (the sha256 e1cd16e9…154ec15 is unchanged).
  3. Retarget this PR to main after TC-1482: wire optional NODE_STATS_READ_TOKEN into the production deploy (companion to #370) #371 merges. TC-1482: wire optional NODE_STATS_READ_TOKEN into the production deploy (companion to #370) #371 itself waits for feat(admin): read-only GET /admin/stats owner list (TC-1482) #370's H1 fix.

(a)(b)(c) against the combined compose: unchanged from my #371 comment.

  • (a) No registration if production uses the centralized KMS (Amos confirms with phala cvms get first; an on-chain KMS needs addComposeHash of the new app-compose hash, or boot is denied). The dstack compose_hash changes at the deploy and again after the allowed_envs sync.
  • (b) DID unchanged: GetKey(path) from the app_id-derived key; the compose is not an input. Keep the post-deploy DID + canary gate.
  • (c) Omitting the secret later fails closed before mutation at docker.yml:1013/862 with ::error::allowed_envs synchronization failed during complete whitelist validation; response details withheld. (l.913). It also blocks rollback.

CI at 2dc8e4b when posting: pass: Build dstack ingress image, Version guard, at-rest harness, boundary, build (workspace), capabilities, clippy (workspace), fmt (workspace), hosted-compose-proof, m1-realdata, plan, separate workspace lockfiles, w5 workspace lockfile. Pending: build, test (workspace, PostgreSQL-backed). Skipped: announce, build-dstack, build-global-artifacts, build-local-artifacts, host, publish-homebrew-formula, rollback-audit. The verdict is code-only for the pending checks.

@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-1274 (reviewed exact head ed81bd1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant