Feat/telemetry#39
Conversation
| UUID: uuid.New().String(), | ||
| Warned: false, | ||
| } | ||
| writeTelemetry(filePath, telemetry) |
There was a problem hiding this comment.
writeTelemetry error silently discarded — UUID lost on every run
writeTelemetry returns an error, but the return value is ignored here and again at line 65 in Send. If writing the file fails (e.g. a permissions issue on the config dir), no UUID is ever persisted: every subsequent invocation calls getOrCreateTelemetry, finds no file, generates a fresh UUID, fails to write it, and repeats — so all events are sent with different, ephemeral UUIDs, making it impossible to deduplicate users in analytics. Separately, the write at line 65 persisting Warned = true also drops its error, so the consent notice is reprinted on every run whenever the config dir is not writable. Both call sites should check and at least log the returned error.
* bump up version code * bring back installer script guide in README.md * docs: update README.md to mention install.sh env vars * added new install.sh env vars to agents.sh * Feat/telemetry (#39) * add no-telemetry cli flag and toml config flag. read and call telemetry if false * send analytics in telemetry.Send * added telemetry message * read/write telemetry.json file * fixed cli-flags test * added tests for telemetry * docs: updated changelog * docs: added telemetry to README.md * removed debug prints * send telemetry in CI * use NoTelemetry instead of Debug in cli-flag test * fix no-telemetry flag name * fix: send erro when not nil * fix test error * send ci to analytics * docs: updated changelog * docs: added 1.3.8 release notes * added 1.3.8-alpha release notes to 1.3.8 * fix typos, handle error state
Pull Request
Description
Send anonymous telemetry usage only if user allows it. When running in CI environment, no consent will be shown, but telemetry can still be disabled with
--no-telemetrycli-flagChanges Made
no-telemetrycli-flagno-telemetryto repoconfig.tomltelemetry.jsonto${USER_CONFIG_DIR}/reposcan/Testing
filters,output, etc...Checklist
Greptile Summary
This PR introduces opt-in anonymous telemetry via Mixpanel, guarded by a
--no-telemetryCLI flag and atelemetry.jsonconsent file stored in the user config directory. A one-time notice is printed on first run; subsequent runs skip the notice.internal/telemetrypackage: handles UUID generation, consent-file persistence, and analytics dispatch; variables are injected for testability.--no-telemetryflag: wired throughConfig.NoTelemetry, read from bothconfig.tomland CLI flags, and checked inrun()before dispatching telemetry.CIenv var is set, telemetry is sent immediately without consulting the consent file — bypassing the user-opt-in flow described in the PR.Confidence Score: 4/5
Safe to merge with one fix:
writeTelemetryreturn values should be checked at both call sites before shipping.The core telemetry flow works correctly for the happy path. The one concrete defect is that
writeTelemetryerrors are silently discarded in bothgetOrCreateTelemetry(UUID persistence) andSend(Warned-state persistence). On any filesystem where the config dir is not writable, every run generates a fresh ephemeral UUID and reprints the consent notice, corrupting analytics data and degrading user experience. The rest of the integration is straightforward and correct.internal/telemetry/telemetry.go— specifically the twowriteTelemetrycall sites that drop their error returns.Important Files Changed
writeTelemetryreturn values silently discarded in two places, causing UUID and Warned-state loss on write failures.TestSend_CIEnvironment_PrintsMsgAndSkipsAnalyticscontradicts its assertion.no-telemetryflag reading and gatestelemetry.Sendbehindconfigs.NoTelemetry; logic is correct.no-telemetryflag in tests; assertion only tests the default value.NoTelemetry boolfield with correct TOML tag; no issues.--no-telemetryas aBoolPflag; matches pattern of existing flags, no issues.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[run configs] --> B{NoTelemetry?} B -- yes --> C[skip telemetry] B -- no --> D{CI env set?} D -- yes --> E[sendTelemetry directly] D -- no --> F[getTelemetryFilePath] F --> G{error?} G -- yes --> H[log error, return] G -- no --> I[getOrCreateTelemetry] I --> J{file exists?} J -- no --> K[create new UUID + write file] J -- yes --> L[read telemetry.json] K --> M{Warned?} L --> M M -- no --> N[print consent notice, set Warned=true, write file] M -- yes --> O[skip notice] N --> E O --> E E --> P[analyticsService.Send usage event]Reviews (4): Last reviewed commit: "send ci to analytics" | Re-trigger Greptile