Add Cloudflare D1 (local) adapter for wrangler dev tracing#22
Merged
Conversation
… dev
D1 runs inside workerd, so the mysqlnd hook cannot see it. This adds
d1-adapter/, a two-part adapter that feeds D1 queries into the existing
profiler pipeline:
- index.js: Worker-side traceD1() wrapper for the D1Database binding.
Intercepts prepare/bind/first/run/all/raw/batch/exec/withSession and
reports query, bound params, ok/err status, optional JS backtrace and
context tags (d1ProfilerTag/Untag/GetTag) to a local collector via
fire-and-forget fetch. enabled:false returns the original binding.
- collector.js: dependency-free Node HTTP collector that honors
jobs.json active jobs and appends {key}.jsonl / {key}.raw.log in the
exact format of profiler_log.c, so the CLI tool and the VSCode /
JetBrains plugins display D1 queries unchanged.
Includes node:test coverage for both parts, format-compatibility
assertions, docs, and a CI job (Node 18/20/22).
https://claude.ai/code/session_01X8GQfc7ymjHGGUaHdaeReN
|
Warning Review limit reached
More reviews will be available in 31 minutes and 29 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
WalkthroughCloudflare D1（ローカル開発）向けのプロファイリング・アダプタ機能を新規追加します。HTTP イベント収集サーバー、Worker 側クエリトレーシング、包括的なテスト、およびドキュメンテーションを実装しています。 ChangesCloudflare D1 ローカルプロファイリング・アダプタ
Sequence Diagram(s)sequenceDiagram
participant Worker as Worker (fetch)
participant TraceProxy as traceD1 Proxy
participant D1 as D1 Binding
participant Collector as Collector Server
participant LogFS as Log Files
Worker->>TraceProxy: prepare(sql).bind(...).first()
TraceProxy->>D1: execute (original)
D1-->>TraceProxy: result or error
TraceProxy->>TraceProxy: build event {q, params, trace, s, ts, tag}
TraceProxy->>Collector: POST /events (JSON)
note over Collector: getActiveJobs from jobs.json
loop for each active job
Collector->>LogFS: append JSONL line
Collector->>LogFS: append Raw Log (if enabled)
end
Collector-->>TraceProxy: {written: N}
note over TraceProxy: ctx.waitUntil(promise)
TraceProxy-->>Worker: return result or throw
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
d1-adapter/test/trace-d1.test.mjs (1)
152-162: ⚡ Quick win
fetchの同期 throw を検証する回帰テストを追加してください。Line 155 は
Promise.rejectケースのみで、同期 throw の注入ケースが未検証です。送信失敗を常に握りつぶす契約を固定化するため、fetch: () => { throw new Error('boom') }のテストを1件追加したいです。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@d1-adapter/test/trace-d1.test.mjs` around lines 152 - 162, Add a new test alongside the existing "queries are dropped silently when collector is down" that injects a synchronously throwing fetch to verify we also swallow sync throws; specifically call traceD1(makeFakeD1(), { fetch: () => { throw new Error('boom') }, ctx: { waitUntil: (p) => pending.push(p) } }) and run db.prepare('SELECT 1').run() asserting the result is { success: true } and that awaiting Promise.all(pending) does not reject, mirroring the async reject case to ensure traceD1/prepare/run handles synchronous throws from fetch.d1-adapter/collector.js (2)
248-265: 💤 Low valueCLI 引数パース時の境界チェック欠如
argv[++i]で値を取得する際、引数リストの末尾で値が欠落している場合（例：--portのみ指定）、undefinedがNumber()に渡されNaNになります。また、--hostや--log-dirはundefinedがそのまま設定されます。致命的ではありませんが、ユーザーフレンドリーなエラーメッセージの表示を検討してください。
🔧 引数検証の追加例
for (let i = 0; i < argv.length; i++) { switch (argv[i]) { - case '--port': opts.port = Number(argv[++i]); break; - case '--host': opts.host = argv[++i]; break; - case '--log-dir': opts.logDir = argv[++i]; break; + case '--port': + if (i + 1 >= argv.length) { console.error('--port requires a value'); process.exit(1); } + opts.port = Number(argv[++i]); + if (Number.isNaN(opts.port)) { console.error('--port must be a number'); process.exit(1); } + break; + case '--host': + if (i + 1 >= argv.length) { console.error('--host requires a value'); process.exit(1); } + opts.host = argv[++i]; + break; + case '--log-dir': + if (i + 1 >= argv.length) { console.error('--log-dir requires a value'); process.exit(1); } + opts.logDir = argv[++i]; + break;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@d1-adapter/collector.js` around lines 248 - 265, The CLI parsing loop that uses argv[++i] (inside the for loop / switch in collector.js) lacks boundary checks and validation; update the switch cases for '--port', '--host', '--log-dir', and '--job-check-interval' to first verify that argv[i+1] exists (otherwise print a clear error like "Missing value for --port" and process.exit(1)), validate numeric values for port and jobCheckInterval (ensure Number(...) yields a finite integer in range and error out on NaN), and ensure host/logDir are non-empty strings before assigning to opts; keep the same switch structure but add these presence and value checks to avoid setting undefined or NaN.
83-89: 💤 Low valuejobKey のパス・トラバーサル検証が必要
handleEventでjobs.jsonから取得したjobKeyをそのままファイル名として使用しています。jobs.jsonは CLI ツールによって管理されるため信頼できる入力ですが、万一../などのパス区切り文字を含む場合、logDir外への書き込みが可能になります。Worker 側からのイベントには
kフィールドが含まれず、Collector がjobs.jsonから読み取るため、リスクは低いですが、防御的なサニタイズを検討してください。🛡️ 防御的サニタイズの提案
_append(filename, text) { + // Prevent path traversal via malformed job keys + if (filename.includes('/') || filename.includes('\\') || filename.includes('..')) { + return; + } try { fs.appendFileSync(path.join(this.logDir, filename), text);Also applies to: 134-140
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@d1-adapter/collector.js` around lines 83 - 89, The loop in handleEvent passes unvalidated job keys from jobs.json into file-writing helpers (_appendJsonl and _appendRaw), allowing potential path traversal; before calling these methods, sanitize/validate each job key (e.g., reject or canonicalize keys containing path separators like '/' or '..', or use path.basename, or resolve with path.join and confirm the resolved path is inside the intended logDir) and throw or skip invalid keys; apply the same validation to the other occurrence mentioned (lines 134-140) so all uses of job keys are protected.d1-adapter/test/collector.test.mjs (1)
8-19: 💤 Low valueテスト後の一時ディレクトリがクリーンアップされていない
makeLogDir()で作成された一時ディレクトリ（os.tmpdir()配下）は、テスト完了後に削除されていません。テストを繰り返し実行すると、一時ディレクトリが蓄積する可能性があります。CI 環境では問題になりにくいですが、ローカル開発では考慮が必要かもしれません。
Also applies to: 118-152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@d1-adapter/test/collector.test.mjs` around lines 8 - 19, makeLogDir currently creates a temp directory with fs.mkdtempSync and returns its path but tests never remove it; modify tests to remove the created temp dirs after each test (or after all) by using fs.rmSync(dir, { recursive: true, force: true }) or Node's tmp.dirSync/with cleanup, e.g., update the tests that call makeLogDir (including usages around makeLogDir and the blocks referenced at lines 118-152) to either register the returned dir for cleanup in afterEach/afterAll or have makeLogDir accept and auto-register a cleanup callback so the temp directory is deleted when the test finishes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/tests.yml:
- Around line 49-68: The test job "test-d1-adapter" currently relies on default
GitHub Actions permissions; add an explicit minimal permissions stanza to the
job (e.g., set permissions: contents: read) to enforce least-privilege. Locate
the job named test-d1-adapter in the workflow and insert a permissions block at
the job level (above or next to runs-on/strategy) so the job explicitly requests
only the required permission.
- Around line 59-64: Replace the tag references for external GitHub Actions with
specific commit SHAs: change uses: actions/checkout@v4 to uses:
actions/checkout@<commit-SHA> and uses: actions/setup-node@v4 to uses:
actions/setup-node@<commit-SHA>; additionally, add persist-credentials: false
under the actions/checkout step to disable credential persistence and reduce
token exposure (refer to the checkout step and setup-node usage in the
workflow).
In `@d1-adapter/index.js`:
- Around line 29-53: The global tagStack causes cross-request leakage; replace
it with a request-scoped store and update d1ProfilerTag, d1ProfilerUntag, and
d1ProfilerGetTag to use that scope (e.g., use Node's AsyncLocalStorage to keep a
per-request stack, or convert this module to export a factory that returns
tag-handling functions bound to a per-request array) so that each request gets
its own stack and tags no longer interfere across concurrent requests; refactor
references to tagStack inside d1ProfilerTag/d1ProfilerUntag/d1ProfilerGetTag to
read/write the stack from the per-request storage.
- Around line 197-209: The current call to cfg.fetch is invoked directly
(assigned to const promise) so a synchronous throw from an injected fetch breaks
the "silently drop collector errors" contract; change the invocation to be
scheduled through Promise.resolve().then(() => cfg.fetch(cfg.collectorUrl, {
method: 'POST', headers: { 'content-type': 'application/json' }, body:
JSON.stringify(event) })).then(...).catch(() => {}) so any synchronous
exceptions from cfg.fetch are turned into a rejected promise and swallowed; keep
the existing success handler that drains res.body.cancel and the existing empty
rejection handler semantics (i.e., ensure the final catch absorbs errors).
In `@d1-adapter/README.md`:
- Around line 9-19: The fenced ASCII diagram block containing "wrangler dev
(workerd)", "traceD1(env.DB)", and "collector.js (Node)" is missing a language
specifier causing MD040; fix it by changing the opening fence to include a
language (e.g., ```text) so the code block is explicitly marked as plain text
and the markdownlint warning is resolved.
---
Nitpick comments:
In `@d1-adapter/collector.js`:
- Around line 248-265: The CLI parsing loop that uses argv[++i] (inside the for
loop / switch in collector.js) lacks boundary checks and validation; update the
switch cases for '--port', '--host', '--log-dir', and '--job-check-interval' to
first verify that argv[i+1] exists (otherwise print a clear error like "Missing
value for --port" and process.exit(1)), validate numeric values for port and
jobCheckInterval (ensure Number(...) yields a finite integer in range and error
out on NaN), and ensure host/logDir are non-empty strings before assigning to
opts; keep the same switch structure but add these presence and value checks to
avoid setting undefined or NaN.
- Around line 83-89: The loop in handleEvent passes unvalidated job keys from
jobs.json into file-writing helpers (_appendJsonl and _appendRaw), allowing
potential path traversal; before calling these methods, sanitize/validate each
job key (e.g., reject or canonicalize keys containing path separators like '/'
or '..', or use path.basename, or resolve with path.join and confirm the
resolved path is inside the intended logDir) and throw or skip invalid keys;
apply the same validation to the other occurrence mentioned (lines 134-140) so
all uses of job keys are protected.
In `@d1-adapter/test/collector.test.mjs`:
- Around line 8-19: makeLogDir currently creates a temp directory with
fs.mkdtempSync and returns its path but tests never remove it; modify tests to
remove the created temp dirs after each test (or after all) by using
fs.rmSync(dir, { recursive: true, force: true }) or Node's tmp.dirSync/with
cleanup, e.g., update the tests that call makeLogDir (including usages around
makeLogDir and the blocks referenced at lines 118-152) to either register the
returned dir for cleanup in afterEach/afterAll or have makeLogDir accept and
auto-register a cleanup callback so the temp directory is deleted when the test
finishes.
In `@d1-adapter/test/trace-d1.test.mjs`:
- Around line 152-162: Add a new test alongside the existing "queries are
dropped silently when collector is down" that injects a synchronously throwing
fetch to verify we also swallow sync throws; specifically call
traceD1(makeFakeD1(), { fetch: () => { throw new Error('boom') }, ctx: {
waitUntil: (p) => pending.push(p) } }) and run db.prepare('SELECT 1').run()
asserting the result is { success: true } and that awaiting Promise.all(pending)
does not reject, mirroring the async reject case to ensure traceD1/prepare/run
handles synchronous throws from fetch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a8bd926-ac69-4b8f-800c-4bd023cc0fde
📒 Files selected for processing (9)
.github/workflows/tests.ymlREADME.mdd1-adapter/README.mdd1-adapter/collector.jsd1-adapter/index.d.tsd1-adapter/index.jsd1-adapter/package.jsond1-adapter/test/collector.test.mjsd1-adapter/test/trace-d1.test.mjs
The D1 collector now persists the worker's db marker into the jsonl log
("db":"d1"); entries written by the PHP extension have no db field and
are treated as MariaDB, so existing logs stay compatible.
- VSCode: source label in each query row's description and a Source:
metadata item in the expanded details
- JetBrains: new DB column in the query table (colored, searchable via
the text filter) and a Source: row in the detail panel
- CLI: job show passes the db field through
- Tests: collector format/sanitization, fromRaw/getDbLabel (vitest),
kotlinx deserialization of the db marker
https://claude.ai/code/session_01X8GQfc7ymjHGGUaHdaeReN
- traceD1() wrappers now expose profilerTag/profilerUntag/profilerGetTag with a per-instance tag stack, so tags no longer interleave across concurrent requests; the module-level functions remain as the shared fallback and the instance stack takes precedence - Absorb synchronous throws from an injected fetch implementation so collector failures can never reach the caller - tests.yml: explicit workflow-level 'permissions: contents: read' and persist-credentials: false on the D1 job's checkout - README: language on the diagram code fence, document the new request-scoped tag API https://claude.ai/code/session_01X8GQfc7ymjHGGUaHdaeReN
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for tracing Cloudflare D1 queries during local development (wrangler dev) into the existing MariaDB Profiler job log format, and updates the IDE tooling (VSCode + JetBrains) plus CLI output to recognize and display the query source.
Changes:
- Introduces a new
d1-adapter/package with a Worker-side D1 proxy (traceD1) and a local Node collector that writes profiler-compatible.jsonland.raw.logfiles. - Extends log consumers (VSCode extension, JetBrains plugin, and CLI
job show) to pass through and render a new optional"db"source marker (defaulting to MariaDB when absent). - Adds unit tests for the adapter + collector and wires them into CI across Node 18/20/22.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
vscode-extension/test/unit/QueryEntry.test.ts |
Adds tests for parsing and labeling the new db marker and getDbLabel() behavior. |
vscode-extension/src/provider/QueryTreeProvider.ts |
Displays DB source in the tree (metadata + entry description) via getDbLabel(). |
vscode-extension/src/model/QueryEntry.ts |
Adds db?: string to query entry types and implements getDbLabel(). |
README.md |
Documents how to trace D1 locally and points to d1-adapter/ docs. |
jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/service/JsonParsingTest.kt |
Adds coverage for parsing db and deriving a UI label. |
jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/ui/table/QueryTableModel.kt |
Adds a “DB” column and includes DB label in text filtering. |
jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/ui/table/QueryCellRenderer.kt |
Colors/centers the DB column rendering. |
jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/ui/panel/QueryLogPanel.kt |
Adjusts table column sizing to include the new DB column. |
jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/ui/panel/QueryDetailPanel.kt |
Adds “Source” to the query detail metadata panel. |
jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt |
Adds db field and dbLabel computed property for UI usage. |
d1-adapter/test/trace-d1.test.mjs |
Unit tests for Worker-side proxy behavior (params, batch, errors, tags, backtrace). |
d1-adapter/test/collector.test.mjs |
Unit tests for collector formatting, job gating, db sanitization, and HTTP server behavior. |
d1-adapter/README.md |
Standalone documentation for the adapter/collector usage and API. |
d1-adapter/package.json |
Defines the standalone package, exports, bin entry, and Node engine/test script. |
d1-adapter/index.js |
Implements the Worker-side D1 wrapper that emits events to the collector. |
d1-adapter/index.d.ts |
TypeScript definitions for traceD1 options and tagging APIs. |
d1-adapter/collector.js |
Implements the local Node collector that writes profiler-compatible job logs. |
cli/mariadb_profiler.php |
Passes through db in job show JSON output when present. |
.github/workflows/tests.yml |
Adds CI job to run D1 adapter tests on Node 18/20/22 and sets read-only contents permission. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+92
to
+94
| _appendJsonl(jobKey, event, ts) { | ||
| /* Field order matches profiler_log_jsonl(): k, q, tag, params, trace, s, ts */ | ||
| let line = `{"k":${JSON.stringify(jobKey)},"q":${JSON.stringify(event.q)}`; |
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
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.
Summary
Adds a new D1 adapter that enables tracing of Cloudflare D1 queries executed during local development (
wrangler dev) into the MariaDB Profiler's job logs. This allows developers to use the same CLI tool and IDE plugins (VSCode, JetBrains) to profile D1 queries as they do for PHP/mysqlnd queries.Key Changes
Worker-side adapter (
d1-adapter/index.js): ExportstraceD1()function that wraps a D1Database binding to intercept and log all query methods (prepare,bind,first,run,all,raw,batch,exec,withSession). Includes tag stack management (d1ProfilerTag,d1ProfilerUntag,d1ProfilerGetTag) and optional JS backtrace capture.Collector process (
d1-adapter/collector.js): Node.js HTTP server that receives query events from the Worker and writes them to job log files in the exact same format as the PHP extension (.jsonland.raw.logfiles). Respects active jobs fromjobs.jsonand only records queries while at least one job is active.Comprehensive test suite: Unit tests for both the Worker adapter and collector, covering prepared statements, batch operations, error handling, tag management, backtrace capture, and HTTP server functionality.
Documentation and types: README with setup instructions, API reference, and TypeScript type definitions (
index.d.ts). Package.json exports both the adapter and collector as a standalone CLI tool.CI integration: Added GitHub Actions workflow to run D1 adapter tests across Node.js 18, 20, and 22.
Implementation Details
errand the error is re-thrownjobs.jsonreads with configurable interval to minimize I/O.jsonloutput matches the PHP extension'sprofiler_log_jsonl()function for compatibility with existing IDE parsershttps://claude.ai/code/session_01X8GQfc7ymjHGGUaHdaeReN
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests