telemetry: document env var, add --no-telemetry flag#2519
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
The LIGHTPANDA_DISABLE_TELEMETRY environment variable already opted out of telemetry but was missing from --help and only mentioned briefly in the README. This commit: - Adds a --no-telemetry common option (works for serve, fetch, and mcp). - Threads a CLI override into telemetry.isDisabled() so the crash reporter, which runs outside the App lifecycle, also honors the flag. - Documents both opt-outs in help.zon and the README, and clarifies that LIGHTPANDA_DISABLE_TELEMETRY treats any value (including empty) as truthy — the env var only needs to be present. - Extends the debug-build telemetry test to cover the CLI override.
4ebf806 to
9c092ea
Compare
|
I do like that this is more discoverable. My only reservation is that there's a window (before App.init is called) where the flag is ignored. So it would be possible for --no-telemetry to be set but a crash report would still happen. I assume this is likely common. Punting this one to @krichprollsch also. |
|
Thanks for the contribution. After discussion and while we understand your points, as an open source project, telemetry is vital to sustaining development in the long run. That's why we prefer to close this change and keep only the env var for now.
For CI config it's common to setup env vars, we do that ourself. On-off invocation is also quite easy to do: And in mcp configuration, for example, with Claude you can setup env vars easily. So IMO the env var is well supported and good enough in all these cases. |
Problem
LIGHTPANDA_DISABLE_TELEMETRYalready opts out of telemetry, but:lightpanda --help(any subcommand).=true, which suggests the value matters — it does not. The current implementation usesstd.process.hasEnvVarConstant, so the variable just needs to be present (any value, including empty).Changes
--no-telemetryas a common option forserve,fetch, andmcp. Wired into the existingCommonOptionsblock inConfig.zigso it picks up the standard--kebab-case/--snake_casehandling and unknown-flag validation for free.crash_handler.zigcallstelemetry.isDisabled()from a panic context where theApp/Configare not reachable, so I added a module-levelcli_disabledflag +setDisabledByClisetter thatApp.initpopulates fromconfig.noTelemetry()beforeTelemetry.initruns. This guarantees--no-telemetrysuppresses crash reports too.src/help.zonnow lists--no-telemetryunder common options and references the env var. The README clarifies that the env var only needs to be present (no specific value required) and that debug builds always disable telemetry.isDisabled().Verification
Notes
LIGHTPANDA_DISABLE_TELEMETRYare unchanged; only its documentation is updated to match the actual implementation.telemetry: always disabled in debug builds.