feat(webapp): deployment lifecycle telemetry events - #4778
Conversation
Replaces the deployment.outcome span with a wide deployment.lifecycle event emitted once per terminal transition (DEPLOYED/FAILED/TIMED_OUT/ CANCELED), backdated createdAt-to-terminal, carrying build path (depot/ native/local_bundle), per-phase durations derived from the persisted timestamp chain, error class, org/project/env, runtime, CLI version and trigger source as attributes. A zero-duration deployment.initialized event at creation provides the funnel denominator for stuck-deployment detection. Events are emitted on ROOT_CONTEXT with the forceRecording attribute: the previous span was started under the ambient request context, where the parent-based sampler drops ~95% of traffic before the force-record check runs. SEMINTATTRS_FORCE_RECORDING is now exported for this. The fail, timeout and finalize transitions now use guarded updateMany writes so exactly one caller commits a terminal status and emits the event; this also stops a late timeout from overwriting DEPLOYED. The cancel path now emits too (it previously recorded nothing). Also: cliVersion is stamped onto WorkerDeployment at initialization from the x-trigger-cli-version header (previously only available post-index via BackgroundWorker, i.e. null for pre-index failures); an optional second OTLP exporter (INTERNAL_OTEL_DEPLOYMENT_EVENT_EXPORTER_URL) mirrors deployment.* spans into a dedicated dataset; the tracer provider is flushed on SIGTERM/SIGINT so shutdowns stop dropping the last batch.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (9)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (31)
WalkthroughAdds deployment lifecycle telemetry for initialization, success, failure, cancellation, and timeout events. Adds build-path and duration derivation helpers with tests. Persists the CLI version from the request header on deployments. Guards terminal database transitions against concurrent updates. Adds an optional OTLP exporter for 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 12 files. (2 skipped: 2 unsupported.) Full details: Description checkExplanation The description explains the telemetry changes and related fixes in detail, but it does not follow the repository template. It omits the issue reference, checklist, Testing section with test steps, Changelog section, and Screenshots section. ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/v3/services/failDeployment.server.ts (1)
49-60: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRefresh the deployment before recording its lifecycle.
failedDeploymentuses the row loaded beforeupdateMany. A concurrent phase update can changestartedAt,installedAt,builtAt, orbuildServerMetadata, sorecordDeploymentLifecyclemay record stale values.When
updatedCount === 1, reload the row withfindFirstand guard a missing result. Use the refreshed row for the lifecycle record. The event log uses onlyshortCodeand does not require this reload.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b8a1cfb5-79a0-4d98-882b-9659465a6c46
📒 Files selected for processing (2)
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (35)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: typecheck / typecheck
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: runops-guard / runops-guard
- GitHub Check: code-quality / code-quality
- GitHub Check: report
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: 🛡️ E2E Auth Tests (full)
🧰 Additional context used
📓 Path-based instructions (10)
New code must target Run Engine V2 through the singleton in `app/v3/runEngine.server.ts`; do not reintroduce V1 execution paths. V1 branches may only reject or finalize gracefully with a clean 4xx.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
Never use `request.signal` to detect client disconnects. Use `getRequestAbortSignal()` from `app/services/httpAsyncStorage.server.ts`, which is wired to Express response close events.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/app/v3/services/failDeployment.server.tsapps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts
🔇 Additional comments (2)
apps/webapp/app/v3/services/failDeployment.server.ts (1)
44-89: Add crumbs markers to this transition path.The guarded update and lifecycle emission have no
//@Crumbsmarker or `#region `@crumbsblock. This repeats the existing missing-crumbs finding for neighboring telemetry code.As per coding guidelines, add crumbs as you write code with
//@Crumbsor `#region `@crumbs, then remove them withagentcrumbs stripbefore merge.Source: Coding guidelines
apps/webapp/app/v3/services/recordDeploymentLifecycle.server.ts (1)
69-71: 🎯 Functional CorrectnessNo change needed for
TIMED_OUTterminal time.timeoutDeployment.server.tsassigns and persistsfailedAtbefore passingtimedOutDeploymenttorecordDeploymentLifecycle, soterminalAtresolves tofailedAt.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…w cancel emission Replaces the standalone DEPLOYMENT_TELEMETRY_ATTRIBUTES.md with short comments on the DeploymentTelemetryAttributes keys, and chains the canceled-lifecycle emission through the cancel ResultAsync pipeline instead of a fire-and-forget promise.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Deployments currently leave little analytical trace. This PR makes every deployment emit two analytics events to enable useful queries. It also enables comparing deployments across build paths, CLI versions, runtimes, and orgs.
Where the events come from
deployment.finishedfires exactly once, whichever way the deployment ends, and is backdated to cover the deployment's real lifetime. Not every path visits every state (Depot deploys skip PENDING/INSTALLING, for example) — a phase duration is simply omitted when its state was never entered.What each event carries
depot,native, ornative_local_bundleWith that, one query gives failure rate per build path, duration percentiles per phase, adoption per CLI version, or a per-org health table.
Fixes that ride along
deployment.outcomespan was silently dropped ~95% of the time (it was subject to trace sampling). The new events opt out of sampling explicitly, so every deployment is counted.