fix(core): capture streaming queries in query instrumentation#1580
Merged
Conversation
The per-query log (EMDASH_QUERY_LOG=1) flushed its recorder when middleware
returned, i.e. when the response headers were ready but before the body
streamed. Queries issued by components during streaming were appended to the
recorder but never emitted, so the query-count harness only saw pre-header
queries. Astro 7's queued rendering moves more queries into the streaming
phase, which made the gap obvious (the post-detail fixture route reported 9
queries while the request actually ran 24).
Flush the recorder when the body finishes streaming instead (in the same
stream-end transform that already snapshots the metrics), and keep a fallback
flush in the middleware finally for bodyless responses (redirects, 304s). The
flush is now idempotent so the two paths can't double-emit.
Also adds a companion query-text snapshot to the harness
(query-counts.queries.{target}.json): a per-route map of the actual SQL to its
occurrence count, so a count change shows which query appeared or vanished, not
just that the number moved. Snapshots regenerated for both targets now reflect
full per-request query counts.
🦋 Changeset detectedLatest commit: ae12c00 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
query-count changed
Contributor
Query-count snapshot changes36 routes changed, total Δ +221 queries. SQLite
D1
Comparing snapshot files between base and head. Updated automatically on each push. |
Contributor
Scope checkThis PR changes 858 lines across 10 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | ae12c00 | Jun 22 2026, 11:41 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
emdash-demo-cache | ae12c00 | Jun 22 2026, 11:41 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | ae12c00 | Jun 22 2026, 11:41 AM |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Merged
18 tasks
ascorbic
added a commit
that referenced
this pull request
Jun 22, 2026
…ixed harness The pre-merge snapshots were measured with the old query-counts harness that missed queries issued during streaming (fixed in #1580). Regenerated against the fixed harness so the numbers reflect reality: the per-isolate taxonomy-defs cache removes the repeated _emdash_taxonomy_defs read on every public render (-1 query per route).
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.
What does this PR do?
The query instrumentation (
EMDASH_QUERY_LOG=1) flushed its per-query recorder when middleware returned, i.e. when the response headers were ready but before the body streamed. Queries issued by components during streaming were appended to the recorder but never emitted, so thequery-countsharness only ever saw the pre-header queries.Astro 7 makes queued rendering the default, which shifts more queries into the streaming phase and made the gap obvious: the post-detail fixture route reported 9 queries in the snapshot while the request actually ran 24 (the
[emdash-stream-end]summary already knew the real number).Changes:
finallyfor bodyless responses (redirects, 304s), andflushRecorderis now idempotent so the two paths can't double-emit.query-counts.queries.{target}.json): a per route+phase map of the actual SQL to its occurrence count. A count change now shows which query appeared or vanished, not just that the number moved. Whitespace is normalized and variable-arityIN (?, ?, ...)lists fold toin (...)so the text stays stable.db.countthe stream-end line always reported.Why now / context
Surfaced while reviewing the Astro 7 upgrade (#1579): its CI showed the post page "dropping" from 16 to 9 queries. Investigation found the total per-request query count was unchanged (24 on both Astro 6 and 7) — queued rendering had simply moved queries past the point where the recorder was flushed. This is a pre-existing instrumentation bug the upgrade merely exposed; fixing it here makes the metric measure the whole request regardless of when queries run. Recommend landing this first so #1579 rebases to a no-op query-count diff.
Type of change
Checklist
pnpm typecheckpassespnpm lintpasses (0 diagnostics)pnpm testpasses (core: 4024, incl. 2 new stream-end tests)pnpm formathas been runAI-generated code disclosure
Screenshots / test output
Verified with the harness on both targets: every route's snapshot count now equals the stream-end
db.count(e.g.GET /6->11,GET /posts/building-for-the-long-term16->24 on sqlite). The query-text snapshot for the post page shows the expected breakdown (widget-area join x2, byline join x4, taxonomy join x4, comments, menus, settings, etc.) summing to 24.