Fix telemetry for VS scenarios #3414
Merged
Conversation
This file contains 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
…PERIENCE` environment variable is set and the `dotnetInteractiveFirstUseSentinel` file is missing. We already had the correct checks in place (inside `CommandLineParser.Create()`) to disable sentinel file creation when the environment variable is set. But when initializing `TelemetrySender._enabled`, we were only checking for the presence of the sentinel file and were missing the environment variable check. This meant that if the environment variable is set, then `TelemetrySender._enabled` would be initialized to `false` for the first launch of a particular version of the tool. And it would remain `false` during subsequent launches of the same tool version (since the aforementioned check inside `CommandLineParser.Create()` would ensure than a sentinel file is never created when the environment variable is set). The environment variable is always set when the tool is launched via `StdIoKernelConnector` which is used in the VS scenario. The above bug therefore meant that we would fail to send any telemetry when the tool is launched by VS. However, the environment variable is not set when the tool is launched by the polyglot notebooks VS Code extension. This led to some very confusing behavior where we would occasionally see some telemetry for VS when some user happens to be running both VS and polyglot notebooks on the same machine, *and when the versions of the tool launched by VS and polyglot notebooks extension happen to align exactly* 😄 In such cases, the first polyglot notebooks launch of the tool on the machine would create a sentinel file on disk for the corresponding tool version thereby unblocking telemetry for subsequent VS launches of the same tool version.
jonsequitur
approved these changes
Jan 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Don't disable telemetry if the
DOTNET_INTERACTIVE_SKIP_FIRST_TIME_EXPERIENCEenvironment variable is set and the.dotnetInteractiveFirstUseSentinelfile is missing.We already had the correct checks in place (inside
CommandLineParser.Create()) to disable sentinel file creation when the environment variable is set. But when initializingTelemetrySender._enabled, we were only checking for the presence of the sentinel file and were missing the environment variable check. This meant that if the environment variable is set, thenTelemetrySender._enabledwould be initialized tofalsefor the first launch of a particular version of the tool. And it would remainfalseduring subsequent launches of the same tool version (since the aforementioned check insideCommandLineParser.Create()would ensure than a sentinel file is never created when the environment variable is set).The environment variable is always set when the tool is launched via
StdIoKernelConnectorwhich is used in the VS scenario. The above bug therefore meant that we would fail to send any telemetry when the tool is launched by VS.However, the environment variable is not set when the tool is launched by the polyglot notebooks VS Code extension. This led to some very confusing behavior where we would occasionally see some telemetry for VS when some user happens to be running both VS and polyglot notebooks on the same machine, and when the versions of the tool launched by VS and polyglot notebooks extension happen to align exactly 😄 In such cases, the first polyglot notebooks launch of the tool on the machine would create a sentinel file on disk for the corresponding tool version thereby unblocking telemetry for subsequent VS launches of the same tool version.