Create spans for parsing and invocation of S.CL commands

baronfel · baronfel · commit c6097ae82d01 · 2025-03-13T11:01:38.000-05:00
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -24,7 +24,8 @@
     <PackageVersion Include="Microsoft.CSharp" Version="4.7.0" />
     <PackageVersion Include="Microsoft.DotNet.PlatformAbstractions" Version="3.1.6" />
     <PackageVersion Include="Newtonsoft.Json" Version="13.0.2" />
-    <PackageVersion Include="System.Memory" Version="4.5.4" />
+    <PackageVersion Include="System.Memory" Version="4.5.5" />
+    <PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="8.0.1" />
     <PackageVersion Include="system.reactive.core" Version="5.0.0" />
   </ItemGroup>
 
diff --git a/src/System.CommandLine/Activities.cs b/src/System.CommandLine/Activities.cs
@@ -0,0 +1,42 @@
+using System.Diagnostics;
+
+namespace System.CommandLine;
+
+internal static class DiagnosticsStrings
+{
+    internal const string LibraryNamespace = "System.CommandLine";
+    internal const string ParseMethod = LibraryNamespace + "Parse";
+    internal const string InvokeMethod = LibraryNamespace + "Invoke";
+    internal const string ExitCode = nameof(ExitCode);
+    internal const string Exception = nameof(Exception);
+    internal const string Command = nameof(Command);
+}
+
+internal static class Activities
+{
+    internal static readonly ActivitySource ActivitySource = new ActivitySource(DiagnosticsStrings.LibraryNamespace);
+}
+
+internal static class ActivityExtensions
+{
+
+    /// <summary>
+    /// Walks up the command tree to get the build the command name by prepending the parent command names to the 'leaf' command name.
+    /// </summary>
+    /// <param name="commandResult"></param>
+    /// <returns>The full command name, like 'dotnet package add'.</returns>
+    internal static string FullCommandName(this Parsing.CommandResult commandResult)
+    {
+        var command = commandResult.Command;
+        var path = command.Name;
+
+        while (commandResult.Parent is Parsing.CommandResult parent)
+        {
+            command = parent.Command;
+            path = $"{command.Name} {path}";
+            commandResult = parent;
+        }
+
+        return path;
+    }
+}
diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs
@@ -1,6 +1,7 @@
 ﻿// Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -10,8 +11,16 @@ internal static class InvocationPipeline
     {
         internal static async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
         {
+            using var invokeActivity = Activities.ActivitySource.StartActivity(DiagnosticsStrings.InvokeMethod);
+            if (invokeActivity is not null)
+            {
+                invokeActivity.DisplayName = parseResult.CommandResult.FullCommandName();
+                invokeActivity.AddTag(DiagnosticsStrings.Command, parseResult.CommandResult.Command.Name);
+            }
+
             if (parseResult.Action is null)
             {
+                invokeActivity?.SetStatus(Diagnostics.ActivityStatusCode.Error);
                 return ReturnCodeForMissingAction(parseResult);
             }
 
@@ -41,7 +50,9 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
                 switch (parseResult.Action)
                 {
                     case SynchronousCommandLineAction syncAction:
-                        return syncAction.Invoke(parseResult);
+                        var syncResult = syncAction.Invoke(parseResult);
+                        invokeActivity?.SetExitCode(syncResult);
+                        return syncResult;
 
                     case AsynchronousCommandLineAction asyncAction:
                         var startedInvocation = asyncAction.InvokeAsync(parseResult, cts.Token);
@@ -52,23 +63,30 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
 
                         if (terminationHandler is null)
                         {
-                            return await startedInvocation;
+                            var asyncResult = await startedInvocation;
+                            invokeActivity?.SetExitCode(asyncResult);
+                            return asyncResult;
                         }
                         else
                         {
                             // Handlers may not implement cancellation.
                             // In such cases, when CancelOnProcessTermination is configured and user presses Ctrl+C,
                             // ProcessTerminationCompletionSource completes first, with the result equal to native exit code for given signal.
                             Task<int> firstCompletedTask = await Task.WhenAny(startedInvocation, terminationHandler.ProcessTerminationCompletionSource.Task);
-                            return await firstCompletedTask; // return the result or propagate the exception
+                            var asyncResult = await firstCompletedTask;  // return the result or propagate the exception
+                            invokeActivity?.SetExitCode(asyncResult);
+                            return asyncResult;
                         }
 
                     default:
-                        throw new ArgumentOutOfRangeException(nameof(parseResult.Action));
+                        var error = new ArgumentOutOfRangeException(nameof(parseResult.Action));
+                        invokeActivity?.Error(error);
+                        throw error;
                 }
             }
             catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler)
             {
+                invokeActivity?.Error(ex);
                 return DefaultExceptionHandler(ex, parseResult.Configuration);
             }
             finally
@@ -79,9 +97,17 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
 
         internal static int Invoke(ParseResult parseResult)
         {
+            using var invokeActivity = Activities.ActivitySource.StartActivity(DiagnosticsStrings.InvokeMethod);
+            if (invokeActivity is not null)
+            {
+                invokeActivity.DisplayName = parseResult.CommandResult.FullCommandName();
+                invokeActivity.AddTag(DiagnosticsStrings.Command, parseResult.CommandResult.Command.Name);
+            }
+
             switch (parseResult.Action)
             {
                 case null:
+                    invokeActivity?.Error();
                     return ReturnCodeForMissingAction(parseResult);
 
                 case SynchronousCommandLineAction syncAction:
@@ -112,15 +138,20 @@ internal static int Invoke(ParseResult parseResult)
                             }
                         }
 
-                        return syncAction.Invoke(parseResult);
+                        var result = syncAction.Invoke(parseResult);
+                        invokeActivity?.SetExitCode(result);
+                        return result;
                     }
                     catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler)
                     {
+                        invokeActivity?.Error(ex);
                         return DefaultExceptionHandler(ex, parseResult.Configuration);
                     }
 
                 default:
-                    throw new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");
+                    var error = new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");
+                    invokeActivity?.Error(error);
+                    throw error;
             }
         }
 
@@ -150,5 +181,38 @@ private static int ReturnCodeForMissingAction(ParseResult parseResult)
                 return 0;
             }
         }
+
+        private static void Succeed(this Diagnostics.Activity activity)
+        {
+            activity.SetStatus(Diagnostics.ActivityStatusCode.Ok);
+            activity.AddTag(DiagnosticsStrings.ExitCode, 0);
+        }
+        private static void Error(this Diagnostics.Activity activity, int statusCode)
+        {
+            activity.SetStatus(Diagnostics.ActivityStatusCode.Error);
+            activity.AddTag(DiagnosticsStrings.ExitCode, statusCode);
+        }
+
+        private static void Error(this Diagnostics.Activity activity, Exception? exception = null)
+        {
+            activity.SetStatus(Diagnostics.ActivityStatusCode.Error);
+            activity.AddTag(DiagnosticsStrings.ExitCode, 1);
+            if (exception is not null)
+            {
+                activity.AddBaggage(DiagnosticsStrings.Exception, exception.ToString());
+            }
+        }
+
+        private static void SetExitCode(this Diagnostics.Activity activity, int exitCode)
+        {
+            if (exitCode == 0)
+            {
+                activity.Succeed();
+            }
+            else
+            {
+                activity.Error(exitCode);
+            }
+        }
     }
 }
diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs
@@ -236,6 +236,7 @@ public Task<int> InvokeAsync(CancellationToken cancellationToken = default)
         /// <returns>A value that can be used as a process exit code.</returns>
         public int Invoke()
         {
+
             var useAsync = false;
 
             if (Action is AsynchronousCommandLineAction)
diff --git a/src/System.CommandLine/Parsing/CommandLineParser.cs b/src/System.CommandLine/Parsing/CommandLineParser.cs
@@ -146,6 +146,8 @@ private static ParseResult Parse(
                 throw new ArgumentNullException(nameof(arguments));
             }
 
+            using var parseActivity = Activities.ActivitySource.StartActivity(DiagnosticsStrings.ParseMethod);
+
             configuration ??= new CommandLineConfiguration(command);
 
             arguments.Tokenize(
@@ -160,7 +162,12 @@ private static ParseResult Parse(
                 tokenizationErrors,
                 rawInput);
 
-            return operation.Parse();
+            var result = operation.Parse();
+            if (result.Errors.Count == 0)
+            {
+                parseActivity?.SetStatus(Diagnostics.ActivityStatusCode.Error);
+            }
+            return result;
         }
 
         private enum Boundary
diff --git a/src/System.CommandLine/System.CommandLine.csproj b/src/System.CommandLine/System.CommandLine.csproj
@@ -27,6 +27,7 @@
 
   <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
     <PackageReference Include="System.Memory" />
+    <PackageReference Include="System.Diagnostics.DiagnosticSource" />
   </ItemGroup>
 
   <ItemGroup>

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`// Copyright (c) .NET Foundation and contributors. All rights reserved.`
`2`	`2`	`// Licensed under the MIT license. See LICENSE file in the project root for full license information.`
`3`	`3`
	`4`	`+using System.Runtime.CompilerServices;`
`4`	`5`	`using System.Threading;`
`5`	`6`	`using System.Threading.Tasks;`
`6`	`7`
`@@ -10,8 +11,16 @@ internal static class InvocationPipeline`
`10`	`11`	`{`
`11`	`12`	`internal static async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)`
`12`	`13`	`{`
	`14`	`+ using var invokeActivity = Activities.ActivitySource.StartActivity(DiagnosticsStrings.InvokeMethod);`
	`15`	`+ if (invokeActivity is not null)`
	`16`	`+ {`
	`17`	`+ invokeActivity.DisplayName = parseResult.CommandResult.FullCommandName();`
	`18`	`+ invokeActivity.AddTag(DiagnosticsStrings.Command, parseResult.CommandResult.Command.Name);`
	`19`	`+ }`
	`20`	`+`
`13`	`21`	`if (parseResult.Action is null)`
`14`	`22`	`{`
	`23`	`+ invokeActivity?.SetStatus(Diagnostics.ActivityStatusCode.Error);`
`15`	`24`	`return ReturnCodeForMissingAction(parseResult);`
`16`	`25`	`}`
`17`	`26`
`@@ -41,7 +50,9 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio`
`41`	`50`	`switch (parseResult.Action)`
`42`	`51`	`{`
`43`	`52`	`case SynchronousCommandLineAction syncAction:`
`44`		`- return syncAction.Invoke(parseResult);`
	`53`	`+ var syncResult = syncAction.Invoke(parseResult);`
	`54`	`+ invokeActivity?.SetExitCode(syncResult);`
	`55`	`+ return syncResult;`
`45`	`56`
`46`	`57`	`case AsynchronousCommandLineAction asyncAction:`
`47`	`58`	`var startedInvocation = asyncAction.InvokeAsync(parseResult, cts.Token);`
`@@ -52,23 +63,30 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio`
`52`	`63`
`53`	`64`	`if (terminationHandler is null)`
`54`	`65`	`{`
`55`		`- return await startedInvocation;`
	`66`	`+ var asyncResult = await startedInvocation;`
	`67`	`+ invokeActivity?.SetExitCode(asyncResult);`
	`68`	`+ return asyncResult;`
`56`	`69`	`}`
`57`	`70`	`else`
`58`	`71`	`{`
`59`	`72`	`// Handlers may not implement cancellation.`
`60`	`73`	`// In such cases, when CancelOnProcessTermination is configured and user presses Ctrl+C,`
`61`	`74`	`// ProcessTerminationCompletionSource completes first, with the result equal to native exit code for given signal.`
`62`	`75`	`Task<int> firstCompletedTask = await Task.WhenAny(startedInvocation, terminationHandler.ProcessTerminationCompletionSource.Task);`
`63`		`- return await firstCompletedTask; // return the result or propagate the exception`
	`76`	`+ var asyncResult = await firstCompletedTask; // return the result or propagate the exception`
	`77`	`+ invokeActivity?.SetExitCode(asyncResult);`
	`78`	`+ return asyncResult;`
`64`	`79`	`}`
`65`	`80`
`66`	`81`	`default:`
`67`		`- throw new ArgumentOutOfRangeException(nameof(parseResult.Action));`
	`82`	`+ var error = new ArgumentOutOfRangeException(nameof(parseResult.Action));`
	`83`	`+ invokeActivity?.Error(error);`
	`84`	`+ throw error;`
`68`	`85`	`}`
`69`	`86`	`}`
`70`	`87`	`catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler)`
`71`	`88`	`{`
	`89`	`+ invokeActivity?.Error(ex);`
`72`	`90`	`return DefaultExceptionHandler(ex, parseResult.Configuration);`
`73`	`91`	`}`
`74`	`92`	`finally`
`@@ -79,9 +97,17 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio`
`79`	`97`
`80`	`98`	`internal static int Invoke(ParseResult parseResult)`
`81`	`99`	`{`
	`100`	`+ using var invokeActivity = Activities.ActivitySource.StartActivity(DiagnosticsStrings.InvokeMethod);`
	`101`	`+ if (invokeActivity is not null)`
	`102`	`+ {`
	`103`	`+ invokeActivity.DisplayName = parseResult.CommandResult.FullCommandName();`
	`104`	`+ invokeActivity.AddTag(DiagnosticsStrings.Command, parseResult.CommandResult.Command.Name);`
	`105`	`+ }`
	`106`	`+`
`82`	`107`	`switch (parseResult.Action)`
`83`	`108`	`{`
`84`	`109`	`case null:`
	`110`	`+ invokeActivity?.Error();`
`85`	`111`	`return ReturnCodeForMissingAction(parseResult);`
`86`	`112`
`87`	`113`	`case SynchronousCommandLineAction syncAction:`
`@@ -112,15 +138,20 @@ internal static int Invoke(ParseResult parseResult)`
`112`	`138`	`}`
`113`	`139`	`}`
`114`	`140`
`115`		`- return syncAction.Invoke(parseResult);`
	`141`	`+ var result = syncAction.Invoke(parseResult);`
	`142`	`+ invokeActivity?.SetExitCode(result);`
	`143`	`+ return result;`
`116`	`144`	`}`
`117`	`145`	`catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler)`
`118`	`146`	`{`
	`147`	`+ invokeActivity?.Error(ex);`
`119`	`148`	`return DefaultExceptionHandler(ex, parseResult.Configuration);`
`120`	`149`	`}`
`121`	`150`
`122`	`151`	`default:`
`123`		`- throw new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");`
	`152`	`+ var error = new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");`
	`153`	`+ invokeActivity?.Error(error);`
	`154`	`+ throw error;`
`124`	`155`	`}`
`125`	`156`	`}`
`126`	`157`
`@@ -150,5 +181,38 @@ private static int ReturnCodeForMissingAction(ParseResult parseResult)`
`150`	`181`	`return 0;`
`151`	`182`	`}`
`152`	`183`	`}`
	`184`	`+`
	`185`	`+ private static void Succeed(this Diagnostics.Activity activity)`
	`186`	`+ {`
	`187`	`+ activity.SetStatus(Diagnostics.ActivityStatusCode.Ok);`
	`188`	`+ activity.AddTag(DiagnosticsStrings.ExitCode, 0);`
	`189`	`+ }`
	`190`	`+ private static void Error(this Diagnostics.Activity activity, int statusCode)`
	`191`	`+ {`
	`192`	`+ activity.SetStatus(Diagnostics.ActivityStatusCode.Error);`
	`193`	`+ activity.AddTag(DiagnosticsStrings.ExitCode, statusCode);`
	`194`	`+ }`
	`195`	`+`
	`196`	`+ private static void Error(this Diagnostics.Activity activity, Exception? exception = null)`
	`197`	`+ {`
	`198`	`+ activity.SetStatus(Diagnostics.ActivityStatusCode.Error);`
	`199`	`+ activity.AddTag(DiagnosticsStrings.ExitCode, 1);`
	`200`	`+ if (exception is not null)`
	`201`	`+ {`
	`202`	`+ activity.AddBaggage(DiagnosticsStrings.Exception, exception.ToString());`
	`203`	`+ }`
	`204`	`+ }`
	`205`	`+`
	`206`	`+ private static void SetExitCode(this Diagnostics.Activity activity, int exitCode)`
	`207`	`+ {`
	`208`	`+ if (exitCode == 0)`
	`209`	`+ {`
	`210`	`+ activity.Succeed();`
	`211`	`+ }`
	`212`	`+ else`
	`213`	`+ {`
	`214`	`+ activity.Error(exitCode);`
	`215`	`+ }`
	`216`	`+ }`
`153`	`217`	`}`
`154`	`218`	`}`
Original file line number	Diff line number	Diff line change
`@@ -236,6 +236,7 @@ public Task<int> InvokeAsync(CancellationToken cancellationToken = default)`
`236`	`236`	`/// <returns>A value that can be used as a process exit code.</returns>`
`237`	`237`	`public int Invoke()`
`238`	`238`	`{`
	`239`	`+`
`239`	`240`	`var useAsync = false;`
`240`	`241`
`241`	`242`	`if (Action is AsynchronousCommandLineAction)`