Skip to content

feat(core): add enableCap query param to bound countLogs scan#8796

Merged
simeng-li merged 1 commit into
masterfrom
simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot
May 13, 2026
Merged

feat(core): add enableCap query param to bound countLogs scan#8796
simeng-li merged 1 commit into
masterfrom
simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot

Conversation

@simeng-li
Copy link
Copy Markdown
Contributor

@simeng-li simeng-li commented May 12, 2026

Summary

Refs LOG-13415.

A customer reported that GET /logs on the Management API consistently times out with a Slonik StatementTimeoutError. Tracing the stack pointed at countLogs (pool.one(...)), not findLogs — so the timeout is in select count(*) from logs ..., not the page fetch.

Why it times out

For a tenant with millions of logs rows, count(*) is unavoidably row-scanning on Postgres. Combined with the key LIKE 'prefix%' OR-chain that the route applies by default, the planner falls back to bitmap heap scans that exceed statement_timeout. The recently added logs__created_at_id index helps ORDER BY created_at DESC LIMIT in findLogs, but does nothing for count(*). The route also requires totalCount to be set, so even a fast page fetch can't return until the slow count resolves.

What changed

Per-request opt-in via a new ?enableCap=true query param on GET /logs and GET /hooks/:id/recent-logs. Default behavior is byte-for-byte identical to current master.

  • packages/core/src/queries/log.tscountLogs now accepts an optional { capped?: boolean } option. When capped is true, it runs select count(*) from (select 1 from logs <conds> limit 10001) s so Postgres halts at the cap; when false (or omitted), the legacy uncapped query is used. Both branches follow the canonical pool.one<{ count: string }> + Number(...) pattern (matches database/row-count.ts). Return shape gains an optional isCapped?: boolean.
  • packages/core/src/middleware/koa-pagination.tsPagination type gains an optional totalCountIsCapped?: boolean. When set, the middleware emits a Total-Number-Is-Capped: true response header and omits both Link: rel="last" and Link: rel="next". The saturated totalCount makes the derived totalPage unreliable, so neither count-derived rel can be answered honestly. Capped responses emit only first and prev.
  • packages/core/src/routes/log.ts and routes/hook.ts — accept enableCap via the zod guard, coerce with yes(...), pass { capped: ... } to countLogs, and propagate isCapped to ctx.pagination.totalCountIsCapped.

Client navigation in capped mode

This matches the Stripe / cursor-style convention for unknown totals: when the server can't honestly answer "what's the last page" or "is there a next page", it doesn't emit those rels. Clients in capped mode walk forward by constructing ?page=N+1 URLs themselves and stop on an empty response. This is the stable contract (documented in LOG-13434) — not a transitional state.

Why per-request, not a global flag

Earlier draft of this PR used EnvSet.values.isDevFeaturesEnabled as a global gate. A per-request query param is strictly better:

  • Existing third-party clients are guaranteed unaffected — no behavioral change unless they explicitly opt in.
  • The customer can self-remediate by deploying a one-line change to their HTTP client.
  • The Console's adoption (LOG-13417) is independent of any backend rollout coordination.
  • No release-day surprise window, no per-tenant feature-flag operations.

Backward compatibility

  • Default GET /logs request (no enableCap): identical to current master. Same SQL behavior (now with the canonical { count: string } typing applied to both branches), same { count } destructure-compatible return shape, same headers, same Link rels.
  • Pagination type gains an optional field; existing paginated routes that don't set it are unaffected.
  • countLogs signature gains an optional options parameter; both existing call sites in this PR are updated, no external callers.
  • No schema changes. No new env vars. No feature flag to flip.

Reviewer notes

  • Capped SQL uses select 1 ... limit N in a subquery so Postgres can stream and halt at N without sorting. No ORDER BY, no materialize node.
  • The Total-Number-Is-Capped header and the last/next rel omissions only fire when the handler sets totalCountIsCapped = true. Other paginated routes are unaffected because they don't set the field.
  • Capped-path coverage is at the unit-test level (mocked countLogs returning isCapped: true, middleware test setting totalCountIsCapped = true directly). Integration coverage of the actual saturated SQL would require seeding > 10k rows per run; we deemed the unit coverage sufficient.

Testing

Unit tests, integration tests

Checklist

  • .changeset
  • unit tests
  • integration tests
  • necessary TSDoc comments

Copilot AI review requested due to automatic review settings May 12, 2026 04:34
@github-actions github-actions Bot added the feature Cool stuff label May 12, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2026

COMPARE TO master

Total Size Diff 📈 +8.44 KB

Diff by File
Name Diff
.changeset/logs-enable-cap.md 📈 +696 Bytes
packages/core/src/middleware/koa-pagination.test.ts 📈 +2.06 KB
packages/core/src/middleware/koa-pagination.ts 📈 +533 Bytes
packages/core/src/queries/log.ts 📈 +1.03 KB
packages/core/src/routes/hook.test.ts 📈 +1.2 KB
packages/core/src/routes/hook.ts 📈 +198 Bytes
packages/core/src/routes/log.test.ts 📈 +1.02 KB
packages/core/src/routes/log.ts 📈 +225 Bytes
packages/integration-tests/src/api/logs.ts 📈 +152 Bytes
packages/integration-tests/src/tests/api/audit-logs/count-cap.test.ts 📈 +1.37 KB

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an opt-in enableCap=true query param to Management API log endpoints to cap the count(*) scan (avoiding statement_timeout on very large tenants) and updates pagination signaling when totals are capped.

Changes:

  • Add capped-count mode to countLogs and propagate “capped” state through routes and pagination context.
  • Update pagination middleware to emit Total-Number-Is-Capped: true, omit Link: rel="last", and keep next available when capped.
  • Add/adjust unit + integration tests and a changeset documenting the new opt-in behavior.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/core/src/queries/log.ts Adds capped count query option and returns isCapped when saturation occurs.
packages/core/src/middleware/koa-pagination.ts Adds totalCountIsCapped handling and new response/header/link behavior for capped totals.
packages/core/src/routes/log.ts Accepts enableCap query param and forwards cap option + capped state to pagination.
packages/core/src/routes/hook.ts Accepts enableCap on recent-logs and forwards cap option + capped state to pagination.
packages/core/src/routes/log.test.ts Updates expectations for new countLogs signature and adds enableCap coverage.
packages/core/src/routes/hook.test.ts Updates expectations for new countLogs signature.
packages/core/src/middleware/koa-pagination.test.ts Adds tests for capped total signaling and link behavior.
packages/integration-tests/src/api/logs.ts Adds getAuditLogsResponse helper to assert headers in integration tests.
packages/integration-tests/src/tests/api/audit-logs/count-cap.test.ts Integration coverage for “no enableCap” vs “enableCap but unsaturated” header/link behavior.
.changeset/logs-enable-cap.md Documents the new opt-in parameter and capped response signaling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/src/queries/log.ts
Comment thread packages/core/src/middleware/koa-pagination.ts Outdated
@simeng-li simeng-li force-pushed the simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot branch from 8ae79ba to 69a2b01 Compare May 12, 2026 06:59
@github-actions github-actions Bot added size/l and removed size/l labels May 12, 2026
@simeng-li simeng-li requested a review from Copilot May 12, 2026 07:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment thread packages/core/src/queries/log.ts Outdated
Comment thread .changeset/logs-enable-cap.md Outdated
@simeng-li simeng-li force-pushed the simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot branch from 69a2b01 to 9d53bca Compare May 12, 2026 07:20
@github-actions github-actions Bot added size/l and removed size/l labels May 12, 2026
@simeng-li simeng-li requested a review from Copilot May 12, 2026 07:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comment thread packages/core/src/routes/hook.ts
@simeng-li simeng-li force-pushed the simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot branch from 9d53bca to ad81e2f Compare May 12, 2026 07:40
@github-actions github-actions Bot added size/l and removed size/l labels May 12, 2026
@simeng-li simeng-li requested a review from Copilot May 12, 2026 07:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

@simeng-li simeng-li force-pushed the simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot branch from 2b13d61 to a178339 Compare May 13, 2026 03:49
@github-actions github-actions Bot added size/l and removed size/l labels May 13, 2026
simeng-li added a commit that referenced this pull request May 13, 2026
…guard

Updates the OpenAPI specs for `GET /api/logs` and
`GET /api/hooks/{id}/recent-logs` to document the `enableCap` query
parameter, `Total-Number-Is-Capped` response header, and the capped-mode
`Link` rel behavior introduced in LOG-13415.

Also removes the `isDevFeaturesEnabled` gate on the Console Audit Logs
page so capped mode is the production default — the backend (#8796) and
console UI (#8799) have both validated.

Refs LOG-13434
@simeng-li simeng-li enabled auto-merge (squash) May 13, 2026 03:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

simeng-li added a commit that referenced this pull request May 13, 2026
…guard

Updates the OpenAPI specs for `GET /api/logs` and
`GET /api/hooks/{id}/recent-logs` to document the `enableCap` query
parameter, `Total-Number-Is-Capped` response header, and the capped-mode
`Link` rel behavior introduced in LOG-13415.

Also removes the `isDevFeaturesEnabled` gate on the Console Audit Logs
page so capped mode is the production default — the backend (#8796) and
console UI (#8799) have both validated.

Refs LOG-13434
@simeng-li simeng-li force-pushed the simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot branch from a178339 to f0fcd56 Compare May 13, 2026 03:59
@github-actions github-actions Bot added size/l and removed size/l labels May 13, 2026
simeng-li added a commit that referenced this pull request May 13, 2026
…guard

Updates the OpenAPI specs for `GET /api/logs` and
`GET /api/hooks/{id}/recent-logs` to document the `enableCap` query
parameter, `Total-Number-Is-Capped` response header, and the capped-mode
`Link` rel behavior introduced in LOG-13415.

Also removes the `isDevFeaturesEnabled` gate on the Console Audit Logs
page so capped mode is the production default — the backend (#8796) and
console UI (#8799) have both validated.

Refs LOG-13434
Copilot AI review requested due to automatic review settings May 13, 2026 07:00
@simeng-li simeng-li force-pushed the simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot branch from f0fcd56 to d3169ec Compare May 13, 2026 07:00
simeng-li added a commit that referenced this pull request May 13, 2026
…guard

Updates the OpenAPI specs for `GET /api/logs` and
`GET /api/hooks/{id}/recent-logs` to document the `enableCap` query
parameter, `Total-Number-Is-Capped` response header, and the capped-mode
`Link` rel behavior introduced in LOG-13415.

Also removes the `isDevFeaturesEnabled` gate on the Console Audit Logs
page so capped mode is the production default — the backend (#8796) and
console UI (#8799) have both validated.

Refs LOG-13434
@github-actions github-actions Bot added size/l and removed size/l labels May 13, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment thread packages/integration-tests/src/api/logs.ts
Comment thread .changeset/logs-enable-cap.md Outdated
@simeng-li simeng-li disabled auto-merge May 13, 2026 07:04
simeng-li added a commit that referenced this pull request May 13, 2026
…guard

Updates the OpenAPI specs for `GET /api/logs` and
`GET /api/hooks/{id}/recent-logs` to document the `enableCap` query
parameter, `Total-Number-Is-Capped` response header, and the capped-mode
`Link` rel behavior introduced in LOG-13415.

Also removes the `isDevFeaturesEnabled` gate on the Console Audit Logs
page so capped mode is the production default — the backend (#8796) and
console UI (#8799) have both validated.

Refs LOG-13434
@simeng-li simeng-li force-pushed the simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot branch from d3169ec to 34903a6 Compare May 13, 2026 07:13
@simeng-li simeng-li enabled auto-merge (squash) May 13, 2026 07:13
@github-actions github-actions Bot added size/l and removed size/l labels May 13, 2026
@simeng-li simeng-li merged commit cc0d703 into master May 13, 2026
43 of 44 checks passed
@simeng-li simeng-li deleted the simeng-log-13415-core-cap-countlogs-with-a-bounded-subquery-so-it-cannot branch May 13, 2026 07:40
simeng-li added a commit that referenced this pull request May 14, 2026
…guard (#8802)

* docs(core): document enableCap contract; remove console dev-features guard

Updates the OpenAPI specs for `GET /api/logs` and
`GET /api/hooks/{id}/recent-logs` to document the `enableCap` query
parameter, `Total-Number-Is-Capped` response header, and the capped-mode
`Link` rel behavior introduced in LOG-13415.

Also removes the `isDevFeaturesEnabled` gate on the Console Audit Logs
page so capped mode is the production default — the backend (#8796) and
console UI (#8799) have both validated.

Refs LOG-13434

* chore: update changeset

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

Labels

Development

Successfully merging this pull request may close these issues.

4 participants