Sign up
View stargazers13Sign up

fix(observability): desktop dev version and web environment match the other services#23975

merged1
main
GitHub fork: yordis/fix-otel-desktop-dev-version-web-env
opened3d agobyYyordis
4 files changed
View on GitHubTwo-way sync enabledPR last imported 2d ago

Problem

  • A development desktop run reports Electron's own version as its service version, so it cannot be grouped with the server by version.
  • The UI has no deployment.environment.name, so its spans cannot be filtered by tier like the server and desktop main process.

Change

  • An unpackaged desktop reports the app's own version, and the UI reports development under the Vite dev server and production otherwise.

Scope and approval

  • Small, focused fix to resources T3 Code already exports. No new signal or setting.

Verification

  • The new DesktopObservability.test.ts assertion on the exported service.version passes (5 of 5) and fails with the fix reverted. The UI environment line has no test. Format, lint, knip and typecheck pass for the changed scope.
  • vp run check:changed with GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null: format, lint, knip and typecheck pass; 1753 tests pass and 2 fail in files this change does not touch (ForgejoSourceControlProvider.test.ts "reads Forgejo jobs that did not run as finished checks" and storageCleanup.test.ts "compares the checked-out branch without a full status"). I did not check those two against a clean upstream/main.

Checks

28 successful · 9 skipped
All checks passed
d97dd85

Added 1 commit

  • fix(observability): desktop dev version and web environment match the other servicesd97dd85
CcoderabbitaiBot3d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 56c365cc29de58746d9888567aed1a71f07d9370 and d97dd8529823398168ad8773cdfb7abb1a68728e.

📒 Files selected for processing: 4
  • apps/desktop/src/app/DesktopObservability.test.ts
  • apps/desktop/src/app/DesktopObservability.ts
  • apps/web/src/observability/clientTracing.ts
  • docs/operations/observability.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Desktop observability uses the package version for unpackaged runs. Web tracing sets the deployment environment to development or production based on the Vite build mode. A desktop test and observability documentation cover these values.

Changes: Observability metadata

Layer / File(s)Summary
Desktop service version
apps/desktop/src/app/DesktopObservability.ts, apps/desktop/src/app/DesktopObservability.test.ts		Unpackaged desktop runs use the desktop package version for service.version. Packaged runs continue to use environment.appVersion. A test checks the version in the exported OTLP log request.
Web deployment environment
apps/web/src/observability/clientTracing.ts, docs/operations/observability.md		The web tracing resource reports development when import.meta.env.DEV is true and production otherwise. The documentation describes this behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested labels: bugfix

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to d97dd

The metadata changes preserve the documented desktop version and web environment classification, with no actionable merge-blocking risk identified.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title follows the conventional commit format and names the two changes: the desktop dev service version and the web environment tag. It is specific and matches the changeset. It does not name the …
Description checkPassedThe description follows the template's Problem, Change, Scope and approval, and Verification sections. The Problem and Change sections state the two fixes clearly. The Scope section says the change is…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title follows the conventional commit format and names the two changes: the desktop dev service version and the web environment tag. It is specific and matches the changeset. It does not name the shared mechanism (OpenTelemetry resource attributes), but the title is accurate and scannable.

Full details: Description check

Explanation

The description follows the template's Problem, Change, Scope and approval, and Verification sections. The Problem and Change sections state the two fixes clearly. The Scope section says the change is a small fix to resources already exported and adds no new signal or setting. The Verification section gives the focused test result, the UI gap, and the repo check output. It reports two unrelated test failures and says they were not checked against upstream/main. It does not list the reproduction steps or environment, and it does not end with the model and harness line the template asks for. Those gaps are minor for a fix this small.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

MmacroscopeappBot3d ago
Synced from GitHub

Approvability

Verdict: Approved at d97dd85

Macroscope's review found this PR approvable — This is a focused observability metadata correction: unpackaged desktop runs report the app version, while web spans identify development versus production builds. Packaged behavior, product functionality, APIs, and deployment behavior remain unchanged, and the desktop branch has focused test coverage.

You can add or adjust custom eligibility rules. Learn more.

apps/web/src/observability/clientTracing.tsline 21Open thread
……@@ -18,6 +18,7 @@ const CLIENT_TRACING_RESOURCE = {
1818 serviceName: "t3code-web",
1919 attributes: {
2020 "service.namespace": "t3code",
21+ "deployment.environment.name": import.meta.env.DEV ? "development" : "production",
JJulius Marmingeon line 212d ago

do we wanna differentaite different stages of production here? development, preview, nightly, stable ? or save that for another attribute? i guess it's already kidna included in service.version