-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add LLM environment detection telemetry data point with rule-based detection system and multiple environment support #50663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
09091ca
Initial plan
Copilot 57b7837
Add LLM environment detection telemetry with CLAUDECODE support
Copilot dc5494e
Change LLM telemetry to use simpler property name and descriptive values
Copilot e4c52b4
Simplify LLM telemetry tests to only test essential scenarios
Copilot c242c38
Merge branch 'main' into copilot/fix-50641
baronfel 92ec5d8
Implement rule-based environment detection system with nullable refer…
Copilot 7736af5
Update src/Cli/dotnet/Telemetry/LLMEnvironmentDetectorForTelemetry.cs
baronfel 984a512
Fix LLM detection syntax and add multiple environment support with te…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| #nullable disable | ||
|
|
||
| namespace Microsoft.DotNet.Cli.Telemetry; | ||
|
|
||
| internal interface ILLMEnvironmentDetector | ||
| { | ||
| string GetLLMEnvironment(); | ||
| } |
28 changes: 28 additions & 0 deletions
28
src/Cli/dotnet/Telemetry/LLMEnvironmentDetectorForTelemetry.cs
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| #nullable disable | ||
|
|
||
| namespace Microsoft.DotNet.Cli.Telemetry; | ||
|
|
||
| internal class LLMEnvironmentDetectorForTelemetry : ILLMEnvironmentDetector | ||
| { | ||
| // Systems where the variable must be present and not-null | ||
| private static readonly string[] _claudeVariables = [ | ||
| // Claude Code | ||
| "CLAUDECODE" | ||
| ]; | ||
|
|
||
| public string GetLLMEnvironment() | ||
| { | ||
| foreach (var variable in _claudeVariables) | ||
| { | ||
| if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable(variable))) | ||
| { | ||
| return "claude"; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| } | ||
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sure this will be expanded over time to support numerous environments. Given that assumption, it might make sense to define rules that encapsule everything in a single place and support different conditions. Of course we can start simple (use a simple dictionary<rulename, envs> and build up as the specific requirements arise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes total sense to me.
@copilot implement a design like this where we can define abstract rules for matching. Also convert the existing CI detection to use similar (not necessarily duplicated) logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot implement @MichaelSimons's feedback, and make sure that you don't disable nullability on the source code you create.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented @MichaelSimons's feedback with a rule-based environment detection system. Created abstract
EnvironmentDetectionRuleclasses that encapsulate matching logic in a single place with support for different conditions (boolean variables, all-present, any-present, and result-returning rules). Converted both CI and LLM detection to use this system while maintaining existing functionality. All source code uses the globally-enabled nullable reference types. Commit 92ec5d8There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reminds me a bit of this old proposal: dotnet/designs#19.
A virtuous flow would look like this:
DOTNET_DO_GOOD_THINGSIt's possible we need to target the specific LLMs for the moment and that's fine. It's more a question of what the long-term plan is.