Skip to content

Add note about telemetry for CodeQL extension settings#15333

Merged
shati-patel merged 1 commit intomainfrom
shati-patel/settings-telemetry
Jan 16, 2024
Merged

Add note about telemetry for CodeQL extension settings#15333
shati-patel merged 1 commit intomainfrom
shati-patel/settings-telemetry

Conversation

@shati-patel
Copy link
Copy Markdown
Contributor

@shati-patel shati-patel commented Jan 15, 2024

We plan to start collecting telemetry about which settings users are using in the CodeQL extension for VS Code, to help inform which features are useful or not. We currently only check for the codeQL.canary setting, but we'd like to extend this to others too, hence the slight change in wording!

For example, an upcoming change (github/vscode-codeql#3238) will add telemetry for the codeQL.addingDatabases.addDatabaseSourceToWorkspace setting.

@shati-patel shati-patel marked this pull request as ready for review January 15, 2024 17:01
@shati-patel shati-patel requested a review from a team as a code owner January 15, 2024 17:01
@intrigus-lgtm
Copy link
Copy Markdown
Contributor

Please don't forget that a non-zero amount of people has likely disabled telemetry.

Copy link
Copy Markdown
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one non-blocking thought I had

- Randomly generated GUID that uniquely identifies a CodeQL extension installation. (Discarded before aggregation.)
- IP address of the client sending the telemetry data. (Discarded before aggregation.)
- Whether or not the ``codeQL.canary`` setting is enabled and set to ``true``.
- Whether any :doc:`CodeQL extension settings <customizing-settings>` are configured.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

codeQL.canary is also a CodeQL extension setting, so from a technical perspective it's odd to mention them in two separate lines. But I don't know what's more understandable for the user. I thought I'd mention this, but I think what's written here already is a perfectly fine explanation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I wondered about that too! @jf205, any preference on whether we continue to call out the canary setting separately, or just include the general line about settings?

@shati-patel
Copy link
Copy Markdown
Contributor Author

Merging this now, we can always adjust the wording later.

@shati-patel shati-patel merged commit e50a0ee into main Jan 16, 2024
@shati-patel shati-patel deleted the shati-patel/settings-telemetry branch January 16, 2024 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants