Add activity spans for template creation and most tool usage #50163

baronfel · 2025-08-11T02:12:20Z

This PR continues the slicing off of the sub-parts of #49409 because it's so big.

This part builds on the previous PR, which introduced the ActivitySource for the CLI and the System.Diagnostics.DiagnosticsSource PackageReference. It adds activity usage to dotnet new invocation, and many parts of dotnet tool usage (including dnx and package downloading).

None of these actually light up in the current main branch because there are no listeners for the activities created.

Copilot

Pull Request Overview

This PR adds activity spans for observability to template creation and tool operations in the .NET CLI. It builds on previous work by adding diagnostic activity tracking to dotnet new template instantiation and various dotnet tool commands including installation, execution, and package downloading.

Key changes include:

Adding activity spans to tool package downloading and extraction operations
Instrumenting template creation workflow with diagnostic activities
Adding activity tracking to tool installation and execution commands
Adding new localization strings for improved error messaging

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`ToolPackageDownloaderBase.cs`	Adds activity spans for moving global tool content and updating runtime config
`ToolPackageDownloader.cs`	Instruments package download and extraction operations with activities
`ToolRunCommand.cs`	Adds activity span for local tool execution and removes nullable disable directive
`ToolInstallGlobalOrToolPathCommand.cs`	Comprehensive refactoring with nullable reference types and activity instrumentation for tool installation
`ToolExecuteCommand.cs`	Adds activity spans for tool location and execution operations
`TemplateInvoker.cs`	Instruments template invocation and creation processes with activities
`TemplateCommand.cs`	Adds activity spans for template constraint validation and invocation
`InstantiateCommand.cs`	Instruments template group creation and command parsing with activities
`Activities.cs`	Adds constants for trace parent environment variables
Various `.xlf` files	Adds new localization entries for tool installation error message
`CliCommandStrings.resx`	Adds new error message for missing package ID in tool installation

src/Cli/dotnet/Commands/Tool/Install/ToolInstallGlobalOrToolPathCommand.cs

KalleOlaviNiemitalo · 2025-08-11T04:26:01Z

src/Cli/Microsoft.TemplateEngine.Cli/Commands/create/InstantiateCommand.cs

@@ -204,6 +206,8 @@ private static async Task<NewCommandStatus> ExecuteIntAsync(

                return await templateListCoordinator.DisplayCommandDescriptionAsync(instantiateArgs, cancellationToken).ConfigureAwait(false);
            }
+            using var createActivity = Activities.Source.StartActivity("instantiate-command");
+            createActivity?.DisplayName = $"Invoke '{instantiateArgs.ShortName}'";


Is instantiateArgs.ShortName already part of the telemetry?

OpenTelemetry.Exporter.OpenTelemetryProtocol exports Activity.DisplayName, not Activity.OperationName; so DisplayName is what matters for privacy.

With regards to privacy - our overall plan for these spans is to not actually send them to the .NET SDK's telemetry collection. We plan to only ever collect the single 'main' activity at Program start, and that mostly because we need an Activity active in order for ActivityEvents to be sent, which is what our Telemetry data points become in an OTel world.

All other Activities we will conditionally disable with a Sampler when we do the full OTel integration - so they'll only be visible when local users are using the OTLP exporter, and at that point you as a user/operator are opting in to all of that data on your system so I believe the privacy constraints are satisfied.

KalleOlaviNiemitalo · 2025-08-11T04:29:40Z

src/Cli/dotnet/Commands/Tool/Execute/ToolExecuteCommand.cs

+        toolLocationActivity?.SetTag("packageId", packageId.ToString());
+        toolLocationActivity?.SetTag("versionRange", versionRange?.ToString() ?? "latest");


Are packageId and versionRange already part of the telemetry? packageId could easily refer to a private package. I assume these strings won't be hashed before they are exported over OTLP.

This PR is adding packageId to other activities too. I won't repeat this comment for those.

The hashing of data is something that we will probably end up doing at the AzureMonitorCollector configuration in a comprehensive way - because that is a constraint that we have on telemetry that makes its way to us. If a user is expressly using the OLTP collector to export data in environments/scenarios they control, they will likely want full-fidelity data, and if they need masking of any kind they can configure it at the OLTP collector configuraiton-level.

KalleOlaviNiemitalo · 2025-08-11T04:30:38Z

src/Cli/dotnet/Commands/Tool/Execute/ToolExecuteCommand.cs

+        using var toolExecuteActivity = Activities.Source.StartActivity("execute-tool");
+        toolExecuteActivity?.SetTag("packageId", packageId.ToString());
+        toolExecuteActivity?.SetTag("version", toolPackage.Version.ToString());
+        toolExecuteActivity?.SetTag("source", toolPackage.Command.Runner);


"source" seems a strange tag name for toolPackage.Command.Runner.

Addressed in 33b5b56

KalleOlaviNiemitalo · 2025-08-11T04:35:38Z

src/Cli/Microsoft.DotNet.Cli.Utils/Activities.cs

+    /// <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);


TRACEPARENT and TRACESTATE don't seem to be used in this PR yet.

I wouldn't use nameof here. If the constants are ever renamed (e.g. to mixed case) in the source code, their string values should remain unchanged. Instead #pragma warning disable CA1507.

In #49668 (comment), I asked about possible interference between instrumentation of the .NET SDK and instrumentation of tools. For example, when a system sets the TRACEPARENT environment variable, runs a tool via dotnet, and expects the tool to receive this TRACEPARENT value. If dotnet changes TRACEPARENT, and sends its own telemetry to Microsoft rather than to the same endpoint as the tool, then the trace will be somewhat broken as it will not be possible to find the correct parent span. I expect the trace ID will be preserved though.

MiYanni · 2025-08-19T23:57:51Z

src/Cli/Microsoft.TemplateEngine.Cli/Commands/create/TemplateCommand.cs

+#pragma warning disable CA2025 // Do not pass 'IDisposable' instances into unawaited tasks
            Task<IReadOnlyList<TemplateConstraintResult>> constraintsEvaluation = ValidateConstraintsAsync(constraintManager, args.Template, args.IsForceFlagSpecified ? cancellationTokenSource.Token : cancellationToken);
+#pragma warning restore CA2025 // Do not pass 'IDisposable' instances into unawaited tasks


Is this something we cannot fix because of interfaces, S.CL, etc? I don't know what the ramifications are.

This isn't a S.CL thing - this is a design choice made by the original Template Engine authors. The constraint evaluation Task needs the constraint manager, which is Disposable mainly to automatically trigger cancellation of any in-flight work. The rest of this method ensures that at some point the Task is awaited, so there's never any actual risk of the disposable member's lifetime being shorter than that of the Task, but the analyzer can't see that because of the way the rest of the method is coded.

Though after rebasing just now, I'm not seeing this warning show up anymore, so maybe something changed?

baronfel requested review from MiYanni and Copilot August 11, 2025 02:12

baronfel requested a review from a team as a code owner August 11, 2025 02:12

baronfel added the Area-Telemetry label Aug 11, 2025

Copilot AI reviewed Aug 11, 2025

View reviewed changes

src/Cli/dotnet/Commands/Tool/Install/ToolInstallGlobalOrToolPathCommand.cs Show resolved Hide resolved

KalleOlaviNiemitalo reviewed Aug 11, 2025

View reviewed changes

KalleOlaviNiemitalo mentioned this pull request Aug 11, 2025

A handful of S.CL-related cleanups and reorgs from the OTel branch to minimize diffs #50164

Open

MiYanni approved these changes Aug 20, 2025

View reviewed changes

baronfel added 4 commits September 3, 2025 21:54

add activities for some template creation pathways

1a2426b

Add some constants used for parent/child process communication

5804b1c

add activities for some tool download-and-usage pathways

e72fa0e

add span for dnx confirmation

b8ecc9a

baronfel force-pushed the activity-usage-on-interesting-pathways branch from 4847bba to b8ecc9a Compare September 4, 2025 03:06

Use more consistent naming scheme for activity properties

33b5b56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add activity spans for template creation and most tool usage #50163

Add activity spans for template creation and most tool usage #50163

Uh oh!

baronfel commented Aug 11, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

KalleOlaviNiemitalo Aug 11, 2025

Uh oh!

KalleOlaviNiemitalo Aug 11, 2025

Uh oh!

baronfel Sep 4, 2025

Uh oh!

KalleOlaviNiemitalo Aug 11, 2025

Uh oh!

baronfel Sep 4, 2025

Uh oh!

KalleOlaviNiemitalo Aug 11, 2025

Uh oh!

KalleOlaviNiemitalo Sep 4, 2025

Uh oh!

KalleOlaviNiemitalo Aug 11, 2025

Uh oh!

MiYanni Aug 19, 2025

Uh oh!

baronfel Sep 4, 2025

Uh oh!

baronfel Sep 4, 2025

Uh oh!

Uh oh!

		toolLocationActivity?.SetTag("packageId", packageId.ToString());
		toolLocationActivity?.SetTag("versionRange", versionRange?.ToString() ?? "latest");

Add activity spans for template creation and most tool usage #50163

Are you sure you want to change the base?

Add activity spans for template creation and most tool usage #50163

Uh oh!

Conversation

baronfel commented Aug 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!