feat: add configuration option to disable CLI telemetry

[ascorbic]

Changes

This change adds a new configuration option to disable telemetry for the project.

Fixes #11820

Testing

Adds tests

Docs

Adds a config option

[changeset-bot]

🦋 Changeset detected

Latest commit: f690a58

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

[bluwy]

Wouldn't this config option still has the caveat mentioned at #11820 (comment) (and the telemetry is disabled later than the process.env.ASTRO_TELEMETRY_DISABLED = '1' solution)?

The absolute solution would be what Matthew suggested or a new CLI flag (which isn't exactly the cleanest either). I can't think of a cleaner way around this 🤔

[ascorbic]

From what I can see, the only problem is the notification that it prints about use of telemetry. There's nothing that logs before the config is loaded. Perhaps if we immediately printed that telemetry is disabled for this site that would be ok.

[florian-lefebvre]

I think it would be nicer to have telemetry: boolean, defaulting to true

[ascorbic]

The reason I didn;t go with that is because it might imply that it overrides the global setting

[florian-lefebvre]

Because telemetry starts before the config?

[ascorbic]

No, I mean that naming it like that might make users thing that true would override their global values, but actually true would mean "true unless there's a global override or environment variable to disable it".

[florian-lefebvre]

mmm i see. I think I'd still prefer telemetry: boolean but log a warning if there's a mismatch with a global override

[bluwy]

From what I can see, the only problem is the notification that it prints about use of telemetry. There's nothing that logs before the config is loaded. Perhaps if we immediately printed that telemetry is disabled for this site that would be ok.

I think you're right. Looks like the places where we call telemetry.record() often happens after the config has been loaded, so maybe we should only print out the telemetry notice on the first record?

However there's an edge case here where we possibly can't based on the config to determine that:

astro/packages/astro/src/cli/throw-and-exit.ts

Line 22 in de35daa

telemetryPromise = telemetry.record(eventError({ cmd, err: errorWithMetadata, isFatal: true }));

If we can figure out how to log the telemetry state correctly, and always respect the config, then that would be the best I think.