feat: add ztd-cli command phase telemetry spans#527
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughInstruments ztd-cli commands (query uses, model-gen, ddl diff) with phase-level telemetry spans, adds a synchronous span helper, exports span-name constants, restructures some flows to propagate resolved context, and adds tests validating emitted span lifecycles and attributes. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Command
participant Telemetry as Telemetry\n(withSpanSync/withSpan)
participant FS as Filesystem
participant DB as Remote DB\n(pgDump/probe)
CLI->>Telemetry: start root span (command)
CLI->>Telemetry: withSpanSync collect-local-ddl
Telemetry->>FS: discover SQL files
FS-->>Telemetry: file list, counts
CLI->>Telemetry: withSpanSync pull-remote-ddl
Telemetry->>DB: run pg_dump / probe
DB-->>Telemetry: remote DDL
CLI->>Telemetry: withSpanSync compute-diff-plan
Telemetry->>Telemetry: detect changes, build patch/header
CLI->>Telemetry: withSpanSync emit-diff-plan
Telemetry->>FS: write patch file / stdout
FS-->>Telemetry: write result
Telemetry-->>CLI: end root span (ok/error)
sequenceDiagram
participant CLI as Model-Gen Command
participant Telemetry as Telemetry\n(withSpanSync/withSpan)
participant FS as Filesystem
participant DB as Probe Client
CLI->>Telemetry: start root span (model-gen)
CLI->>Telemetry: withSpanSync resolve-model-gen-inputs
Telemetry->>FS: validate paths, scan placeholders
CLI->>Telemetry: withSpanSync probe-client-connect
Telemetry->>DB: open probe client
CLI->>Telemetry: withSpanSync probe-query-columns
Telemetry->>DB: execute probe queries
CLI->>Telemetry: withSpanSync type-inference
Telemetry->>Telemetry: infer types, render output
CLI->>Telemetry: withSpanSync file-emit
Telemetry->>FS: write generated file
Telemetry-->>CLI: end root span (ok/error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ztd-cli/tests/telemetry.unit.test.ts (1)
71-92: LGTM!The test validates the core synchronous span lifecycle:
- Span-start emission with attributes
- Correct return value from the wrapped function
- Span-end emission with 'ok' status
Consider adding an error-path test for
withSpanSyncsimilar to the async test at lines 52-55, verifying that exceptions are recorded and the span ends with 'error' status.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/tests/telemetry.unit.test.ts` around lines 71 - 92, Add a new unit test mirroring the async error-path to verify withSpanSync records exceptions: create a test that enables telemetry (setTelemetryEnabled(true); configureTelemetry(); beginCommandSpan('ddl diff')), call withSpanSync('compute-diff-plan', () => { throw new Error('boom'); }, { localFileCount: 2 }) inside an expect(() => ...).toThrow(), and assert the captured stderr JSON payloads include a span-start for 'compute-diff-plan' with the attributes and a span-end for 'compute-diff-plan' with status 'error' (and optionally error details); reference withSpanSync, beginCommandSpan, setTelemetryEnabled, configureTelemetry, and finishCommandSpan when adding the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ztd-cli/tests/telemetry.unit.test.ts`:
- Around line 71-92: Add a new unit test mirroring the async error-path to
verify withSpanSync records exceptions: create a test that enables telemetry
(setTelemetryEnabled(true); configureTelemetry(); beginCommandSpan('ddl diff')),
call withSpanSync('compute-diff-plan', () => { throw new Error('boom'); }, {
localFileCount: 2 }) inside an expect(() => ...).toThrow(), and assert the
captured stderr JSON payloads include a span-start for 'compute-diff-plan' with
the attributes and a span-end for 'compute-diff-plan' with status 'error' (and
optionally error details); reference withSpanSync, beginCommandSpan,
setTelemetryEnabled, configureTelemetry, and finishCommandSpan when adding the
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4610f48-b066-4cc6-a636-771ec832df59
📒 Files selected for processing (7)
packages/ztd-cli/src/commands/diff.tspackages/ztd-cli/src/commands/modelGen.tspackages/ztd-cli/src/commands/query.tspackages/ztd-cli/src/query/report.tspackages/ztd-cli/src/utils/telemetry.tspackages/ztd-cli/tests/commandTelemetry.unit.test.tspackages/ztd-cli/tests/telemetry.unit.test.ts
5f9b684 to
85bced6
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ztd-cli/src/commands/query.ts (1)
131-135: Minor formatting: comment and function declaration on same line.The JSDoc comment ending on line 134 and the function declaration starting on the same line 135 creates an unusual formatting pattern. Consider separating them for readability.
🔧 Suggested formatting fix
-/** - * Keep outline/graph aligned with the existing query surface from main. - * Telemetry instrumentation in this file is intentionally limited to query uses so conflict resolution - * does not regress the established outline/graph commands while issue `#518` focuses on the uses workflow. - */function runQueryUsesCommand(kind: 'table' | 'column', target: string | undefined, options: QueryUsesOptions): void { +/** + * Keep outline/graph aligned with the existing query surface from main. + * Telemetry instrumentation in this file is intentionally limited to query uses so conflict resolution + * does not regress the established outline/graph commands while issue `#518` focuses on the uses workflow. + */ +function runQueryUsesCommand(kind: 'table' | 'column', target: string | undefined, options: QueryUsesOptions): void {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/src/commands/query.ts` around lines 131 - 135, The JSDoc block is currently merged with the function declaration on the same line; separate the comment and the function signature so the JSDoc ending (the comment that documents the query uses behavior) sits on its own line(s) and the function declaration runQueryUsesCommand(kind: 'table' | 'column', target: string | undefined, options: QueryUsesOptions): void starts on the next line for proper formatting and readability — update the placement so the /** ... */ block is immediately above and not trailing into the function line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ztd-cli/src/commands/query.ts`:
- Around line 131-135: The JSDoc block is currently merged with the function
declaration on the same line; separate the comment and the function signature so
the JSDoc ending (the comment that documents the query uses behavior) sits on
its own line(s) and the function declaration runQueryUsesCommand(kind: 'table' |
'column', target: string | undefined, options: QueryUsesOptions): void starts on
the next line for proper formatting and readability — update the placement so
the /** ... */ block is immediately above and not trailing into the function
line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a8e7c8b2-285c-43e6-9a7f-1e3da56e6e98
📒 Files selected for processing (8)
packages/ztd-cli/src/commands/diff.tspackages/ztd-cli/src/commands/modelGen.tspackages/ztd-cli/src/commands/query.tspackages/ztd-cli/src/query/report.tspackages/ztd-cli/src/utils/telemetry.tspackages/ztd-cli/tests/commandTelemetry.unit.test.tspackages/ztd-cli/tests/queryUses.unit.test.tspackages/ztd-cli/tests/telemetry.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ztd-cli/src/utils/telemetry.ts
- packages/ztd-cli/tests/commandTelemetry.unit.test.ts
|
If all GithubActions are successful, merge. |
Summary
Verification
Notes
Summary by CodeRabbit
Tests
Chores