A handful of S.CL-related cleanups and reorgs from the OTel branch to minimize diffs #50164
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the dotnet CLI to improve System.CommandLine (S.CL) usage patterns and code organization. The changes prepare the codebase for OpenTelemetry integration by removing performance measurement handling from the main command processing flow and adopting S.CL's Action pipeline for cross-cutting behaviors.
Key Changes:
- Reorganizes Program.cs into focused sub-methods for better readability and maintainability
- Adopts S.CL's Action pipeline for options like --version, --info, and --diagnostic that have behaviors attached
- Removes performance measurement tracking from telemetry tests and command processing
- Adopts nullable reference types across several files to improve type safety
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/dotnet.Tests/TelemetryCommandTest.cs | Removes performance measurement testing and simplifies test assertions |
| src/Cli/dotnet/Telemetry/Telemetry.cs | Adopts nullable reference types for telemetry properties and parameters |
| src/Cli/dotnet/Program.cs | Major reorganization into focused methods with nullable adoption and Activity-based tracing |
| src/Cli/dotnet/PerformanceLogEventSource.cs | Adopts nullable reference types for startup information parameters |
| src/Cli/dotnet/Parser.cs | Implements S.CL Action classes for --version and --info options |
| src/Cli/dotnet/Extensions/ParseResultExtensions.cs | Simplifies extension methods and removes unused functionality |
| src/Cli/dotnet/CommonOptionsFactory.cs | Implements S.CL Action for diagnostic mode option |
| src/Cli/Microsoft.DotNet.Cli.Utils/Activities.cs | Adds tracing constants for cross-process activity tracking |
baronfel
commented
Aug 11, 2025
Comment on lines
+20
to
+31
| /// <summary> | ||
| /// The environment variable used to transfer the chain of parent activity IDs. | ||
| /// This should be used when constructing new sub-processes in order to | ||
| /// track spans across calls. | ||
| /// </summary> | ||
| public const string TRACEPARENT = nameof(TRACEPARENT); | ||
| /// <summary> | ||
| /// The environment variable used to transfer the trace state of the parent activities. | ||
| /// This should be used when constructing new sub-processes in order to | ||
| /// track spans across calls. | ||
| /// </summary> | ||
| public const string TRACESTATE = nameof(TRACESTATE); |
There was a problem hiding this comment.
TRACEPARENT and TRACESTATE are added in #50163 too. I'd prefer having them added in only one pull request.
Do you intend to insert any new entries into Activity.TraceStateString, or just pass it through unchanged?
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.
This slices off a few S.CL usability refactors off of #49409 to minimize the diff on that monstrosity.
This set of changes