feat(telemetry): workspace count and UI analytics#697
Merged
Conversation
…alytics Add a workspace_count gauge emitted from the CLI on create/delete so the number of workspaces a user manages is tracked, keyed on the existing pseudonymous machine-id distinct ID. The CLI is the single source of truth; desktop-initiated create/delete already shell out to it. Attach a hashed workspace_ref to desktop lifecycle events for per-workspace correlation without leaking raw names, and stop workspace_rename from sending the raw workspace id. Add renderer session tracking (session_start/session_end with idle-aware duration) and engagement events (ide_open, terminal_open, context_switch, settings_changed), plus human-readable screen names on page_view. Make analytics log messages vendor-opaque (no provider name in user-facing strings).
✅ Deploy Preview for images-devsy-sh canceled.
|
✅ Deploy Preview for devsydev canceled.
|
📝 WalkthroughWalkthroughWorkspace commands now emit local workspace-count telemetry. Desktop analytics adds hashed workspace references, session tracking, normalized screens, and engagement events. Port probing continues after availability errors, while desktop tests and provider markup receive formatting-only updates. ChangesCLI workspace telemetry
Desktop workspace analytics
Port search handling
Formatting and markup maintenance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant SessionTracking
participant Analytics
App->>SessionTracking: initSessionTracking()
App->>Analytics: track page_view with path and screen
SessionTracking->>Analytics: record session_start or session_end
App->>Analytics: trackEngagement(feature, properties)
App->>SessionTracking: teardown on destroy
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
FindAvailablePort bailed out whenever IsAvailable returned an error, so a port already in use (net.Listen fails with address-in-use) aborted the whole search instead of moving to the next candidate. Under CI port contention this flaked TestParseAddressAndPort_Empty. Treat a probe error as 'not this port' and keep scanning.
skevetter
marked this pull request as ready for review
July 20, 2026 21:49
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
pkg/telemetry/collect.go (1)
188-208: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winFix property overwrite and serialization in analytics payload.
Because both the
eventanduserpayloads store their stringified properties under the exact same key ("properties"), the analytics client'sbuildPropertiesfunction will overwrite the event properties with the user properties. This results in the complete loss of all event-specific data (e.g.,command,version,desktop,is_ci).Furthermore, stringifying these maps prevents the analytics backend from natively parsing and querying them as individual columns, as PostHog expects a flat dictionary of properties.
Flatten the properties directly into the payload map to resolve both issues.
🐛 Proposed fix to flatten the property maps
func (d *cliCollector) recordEvent( eventType string, eventProperties, userProperties map[string]any, ) { machineID := GetMachineID() - eventPropertiesRaw, _ := json.Marshal(eventProperties) - userPropertiesRaw, _ := json.Marshal(userProperties) + + eventPayload := map[string]any{ + "type": eventType, + "machine_id": machineID, + "timestamp": time.Now().Unix(), + } + for k, v := range eventProperties { + eventPayload[k] = v + } + + userPayload := map[string]any{ + "machine_id": machineID, + "timestamp": time.Now().Unix(), + } + for k, v := range userProperties { + userPayload[k] = v + } + d.analyticsClient.RecordEvent(analytics.Event{ - "event": { - "type": eventType, - "machine_id": machineID, - "properties": string(eventPropertiesRaw), - "timestamp": time.Now().Unix(), - }, - "user": { - "machine_id": machineID, - "properties": string(userPropertiesRaw), - "timestamp": time.Now().Unix(), - }, + "event": eventPayload, + "user": userPayload, }) }🤖 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 `@pkg/telemetry/collect.go` around lines 188 - 208, Update cliCollector.recordEvent to flatten eventProperties and userProperties directly into their respective analytics payload maps instead of serializing either map under the shared "properties" key. Preserve the existing event metadata and user metadata, ensure event-specific fields remain in the event payload, and remove the json.Marshal-based string conversion.
🤖 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.
Outside diff comments:
In `@pkg/telemetry/collect.go`:
- Around line 188-208: Update cliCollector.recordEvent to flatten
eventProperties and userProperties directly into their respective analytics
payload maps instead of serializing either map under the shared "properties"
key. Preserve the existing event metadata and user metadata, ensure
event-specific fields remain in the event payload, and remove the
json.Marshal-based string conversion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dd4974a7-2f45-46c6-8198-a1f067bd858d
📒 Files selected for processing (18)
cmd/workspace/delete.gocmd/workspace/up/up.godesktop/src/main/__tests__/cli.test.tsdesktop/src/main/__tests__/updater.test.tsdesktop/src/main/analytics.tsdesktop/src/main/ipc.tsdesktop/src/renderer/src/App.sveltedesktop/src/renderer/src/lib/analytics.tsdesktop/src/renderer/src/lib/components/provider/ProviderCard.sveltedesktop/src/renderer/src/pages/ContextsPage.sveltedesktop/src/renderer/src/pages/SettingsPage.sveltedesktop/src/renderer/src/pages/TerminalsPage.sveltedesktop/src/renderer/src/pages/WorkspaceDetailPage.sveltepkg/port/port.gopkg/telemetry/analytics/client.gopkg/telemetry/collect.gopkg/telemetry/noop.gopkg/workspace/list.go
recordEvent stored each property map as a JSON string under the same "properties" key in both the event and user payloads. buildProperties merges those two maps into one flat property set, so the user "properties" string overwrote the event "properties" string — dropping every event-specific field (command, version, is_ci, count) and sending the rest as an opaque JSON blob instead of queryable columns. Flatten both property maps directly into their payloads and exclude reserved routing keys (type, machine_id, timestamp) when building properties. Add regression tests for the merge and reserved-key handling.
….Copy Address goconst (repeated "type"/"machine_id"/"timestamp" literals) and modernize (map copy loop) lint findings: add KeyType/KeyMachineID/KeyTimestamp constants in the analytics package, use them across producer, consumer, and tests, and copy property maps with maps.Copy.
skevetter
marked this pull request as draft
July 20, 2026 22:44
skevetter
changed the title
skevetter
marked this pull request as ready for review
July 21, 2026 02:09
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/telemetry/analytics/client_test.go (1)
24-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the length of the resulting map to ensure no extra keys are present.
While the current loop verifies that all expected keys are present and have the correct values, it does not verify that the map only contains these expected keys. Asserting the length of the map adds an extra layer of robustness to the test.
♻️ Proposed refactor
want := map[string]any{ "count": 7, "version": "1.2.3", "os_name": "darwin", "os_arch": "arm64", } + + if len(got) != len(want) { + t.Errorf("got %d properties, want %d", len(got), len(want)) + } + for k, v := range want { if got[k] != v {🤖 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 `@pkg/telemetry/analytics/client_test.go` around lines 24 - 35, Update the test assertion around the want/got map comparison to first verify that len(got) equals len(want), then retain the existing per-key value checks. This ensures the result contains exactly the expected keys without changing the existing value validation.
🤖 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.
Nitpick comments:
In `@pkg/telemetry/analytics/client_test.go`:
- Around line 24-35: Update the test assertion around the want/got map
comparison to first verify that len(got) equals len(want), then retain the
existing per-key value checks. This ensures the result contains exactly the
expected keys without changing the existing value validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c397dfa3-b182-4d02-9ea2-0a73b4ad0cd5
📒 Files selected for processing (10)
cmd/workspace/delete.godesktop/src/main/analytics.tsdesktop/src/renderer/src/lib/analytics.tsdesktop/src/renderer/src/pages/SettingsPage.sveltepkg/port/port.gopkg/telemetry/analytics/client.gopkg/telemetry/analytics/client_test.gopkg/telemetry/analytics/types.gopkg/telemetry/collect.gopkg/workspace/list.go
💤 Files with no reviewable changes (5)
- pkg/port/port.go
- pkg/workspace/list.go
- desktop/src/main/analytics.ts
- cmd/workspace/delete.go
- desktop/src/renderer/src/pages/SettingsPage.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- desktop/src/renderer/src/lib/analytics.ts
- pkg/telemetry/collect.go
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.
Summary
Improves observability telemetry in two areas the current setup under-serves: tracking how many workspaces a user manages, and making the web UI (Electron desktop) analytics actually useful.
workspace_countgauge is emitted from the CLI on create and delete, keyed on the existing pseudonymous machine-id distinct ID (the same identity the desktop shares with spawned CLIs). The CLI is the single source of truth; because desktop create/delete shell out to the CLI, those are covered too. Only create and delete change the count, so both count-changing paths are instrumented.workspace_ref(HMAC of the workspace id, non-reversible) so a workspace's events can be correlated without leaking raw names.workspace_renameno longer sends the raw workspace id.session_start/session_endwith idle-aware duration) and engagement events (ide_open,terminal_open,context_switch,settings_changed, key only), plus human-readable screen names onpage_view.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Improvements