Fix MSBuild telemetry when the server is enabled

.github/copilot-instructions.md

@@ -68,10 +68,21 @@ runtime-library generators or `dotnet/sdk` for SDK analyzers.
 
 ### Architecture and major components
 
-The managed CLI dispatches commands registered in
-[`Parser.cs`](../src/Cli/dotnet/Parser.cs). Unmatched input goes through external command
-resolution and then file-based app fallback, as implemented by
-[`Program.cs`](../src/Cli/dotnet/Program.cs).
+SDK-owned CLI code has three process entry points of equal importance:
+
+- The managed CLI dispatches commands that
+  [`Parser.cs`](../src/Cli/dotnet/Parser.cs) registers.
+  [`Program.cs`](../src/Cli/dotnet/Program.cs) handles unmatched input through external
+  command resolution and file-based app fallback.
+- The Native AOT CLI starts in
+  [`NativeEntryPoint.cs`](../src/Cli/dotnet-aot/NativeEntryPoint.cs). It handles supported
+  commands directly. Unsupported operations continue in the managed CLI.
+- MSBuild loads [`MSBuildLogger`](../src/Cli/dotnet/Commands/MSBuild/MSBuildLogger.cs) from
+  `dotnet.dll` as an `INodeLogger`. The logger can run in the CLI process, a child MSBuild
+  process, or a persistent MSBuild server. Code called through the logger must not assume
+  that a CLI bootstrap initialized process-wide state. Use `BuildStarted` and
+  `BuildFinished` as request boundaries. `Shutdown` completes one logger instance. It does
+  not necessarily end the process.
 
 Major source areas under [`src/`](../src/):
 

documentation/project-docs/developer-guide.md

@@ -57,6 +57,25 @@ Run the following command from the root of the repository:
 
 The build script will output a .NET Core installation to `artifacts\bin\redist\Debug\dotnet` that will include any local changes to the .NET Core CLI.
 
+## SDK process entry points
+
+Changes under `src/Cli` must support three process entry points of equal importance:
+
+| Entry point | Source | Host |
+| --- | --- | --- |
+| Managed CLI | [`src/Cli/dotnet/Program.cs`](../../src/Cli/dotnet/Program.cs) | CoreCLR runs the full command implementation. |
+| Native AOT CLI | [`src/Cli/dotnet-aot/NativeEntryPoint.cs`](../../src/Cli/dotnet-aot/NativeEntryPoint.cs) | The native `dotnet` host calls the exported `dotnet_execute`. Unsupported operations can continue in the managed CLI. See the [NativeAOT design](../../src/Cli/dotnet-aot/DESIGN.md). |
+| MSBuild logger | [`src/Cli/dotnet/Commands/MSBuild/MSBuildLogger.cs`](../../src/Cli/dotnet/Commands/MSBuild/MSBuildLogger.cs) | MSBuild loads the logger type from `dotnet.dll` as an `INodeLogger`. [`MSBuildForwardingApp`](../../src/Cli/dotnet/Commands/MSBuild/MSBuildForwardingApp.cs) adds the `-distributedlogger` argument. |
+
+The MSBuild logger is a separate process entry point. It is not a standalone executable.
+The logger can run in the managed CLI process, a child MSBuild process, or a persistent
+MSBuild server. Code called through the logger must not assume that a CLI bootstrap
+initialized process-wide state.
+
+Use `BuildStarted` and `BuildFinished` to manage state for one build. `Shutdown` completes
+one logger instance. It does not necessarily end the process. A persistent server can run
+multiple builds in one process. Refresh the environment and trace context for each build.
+
 ## Running tests
 
 ### Windows

documentation/project-docs/telemetry.md

@@ -274,6 +274,14 @@ Every telemetry event automatically includes these common properties:
 
 ### SDK-Collected Build Events
 
+[`MSBuildLogger`](../../src/Cli/dotnet/Commands/MSBuild/MSBuildLogger.cs) collects the
+existing events below. MSBuild loads the logger from the SDK. The logger can run in the
+CLI process, a child MSBuild process, or a persistent MSBuild server.
+
+The logger creates an internal activity for each build. When trace context is available,
+the activity is a child of the invoking CLI trace. Server reuse changes telemetry delivery
+and correlation only. It does not change the listed data points or properties.
+
 #### `msbuild/targetframeworkeval`
 
 **When fired**: When target framework is evaluated

src/Cli/AGENTS.md

@@ -2,7 +2,24 @@
 
 Guidance for changes under `src/Cli`.
 
-## Three-project split
+## SDK process entry points
+
+SDK-owned CLI code has three process entry points of equal importance:
+
+| Entry point | Source | Host and lifecycle |
+|-------------|--------|--------------------|
+| Managed CLI | `dotnet/Program.cs` | CoreCLR calls `Program.Main`. The process ends after the command completes. |
+| Native AOT CLI | `dotnet-aot/NativeEntryPoint.cs` | The native host calls the exported `dotnet_execute`. Unsupported operations can continue in the managed CLI. |
+| MSBuild logger | `dotnet/Commands/MSBuild/MSBuildLogger.cs` | MSBuild loads the SDK assembly as an `INodeLogger`. `MSBuildForwardingApp` adds the `-distributedlogger` argument. The logger can run in the CLI process, a child process, or a persistent server. |
+
+Treat the logger as an independent entry point. Do not assume that it runs after managed
+`Program.Main` or the Native AOT bootstrap. Initialize process-wide telemetry and tracing
+when MSBuild loads the logger directly. Use `BuildStarted` and `BuildFinished` for
+request-specific state. `Shutdown` completes one logger instance. It does not necessarily
+end the process. Refresh the environment and trace context for each persistent-server
+request.
+
+## Three-project command split
 
 A `dotnet` command or option spans three cooperating projects:
 

src/Cli/dotnet/CommandFactory/CommandResolution/ActivityContextFactory.cs

@@ -29,7 +29,14 @@ public static class ActivityContextFactory
 #if TARGET_WINDOWS
         var propagationContext = new PropagationContext(activityContext, Baggage.Current);
         Propagators.DefaultTextMapPropagator.Inject(propagationContext, environment, WriteTraceStateIntoEnvironment);
+#else
+        environment[Activities.TRACEPARENT] = $"00-{activityContext.TraceId}-{activityContext.SpanId}-{(byte)activityContext.TraceFlags:x2}";
+        if (!string.IsNullOrEmpty(activityContext.TraceState))
+        {
+            environment[Activities.TRACESTATE] = activityContext.TraceState;
+        }
 #endif
+
         return environment;
     }
 

src/Cli/dotnet/Commands/MSBuild/MSBuildForwardingApp.cs

@@ -7,6 +7,7 @@
 using System.Reflection;
 #endif
 using Microsoft.DotNet.Cli.Commands.Run;
+using Microsoft.DotNet.Cli.CommandFactory.CommandResolution;
 using Microsoft.DotNet.Cli.Telemetry;
 using Microsoft.DotNet.Cli.Utils;
 using Microsoft.DotNet.Cli.Utils.Extensions;
@@ -112,6 +113,14 @@ public void EnvironmentVariable(string name, string? value)
     private void InitializeRequiredEnvironmentVariables()
     {
         EnvironmentVariable(EnvironmentVariableNames.DOTNET_CLI_TELEMETRY_SESSIONID, TelemetryClient.CurrentSessionId);
+
+        if (ActivityContextFactory.MakeActivityContextEnvironment() is { } activityContextEnvironment)
+        {
+            foreach ((string name, string value) in activityContextEnvironment)
+            {
+                EnvironmentVariable(name, value);
+            }
+        }
     }
 
     /// <summary>

src/Cli/dotnet/Commands/MSBuild/MSBuildLogger.cs

@@ -1,17 +1,47 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Globalization;
 using Microsoft.Build.Framework;
 using Microsoft.DotNet.Cli.Telemetry;
+using Microsoft.DotNet.Cli.Utils;
 using Microsoft.DotNet.Utilities;
 
 namespace Microsoft.DotNet.Cli.Commands.MSBuild;
 
+/// <summary>
+/// Receives telemetry from MSBuild and SDK build logic. The logger sends the telemetry
+/// through the .NET SDK telemetry pipeline.
+/// </summary>
+/// <remarks>
+/// MSBuild loads this type from <c>dotnet.dll</c> as a distributed logger. The logger is a
+/// separate SDK entry point. It can run in the managed CLI process, a child MSBuild
+/// process, or a persistent MSBuild server. Some hosts do not run either CLI bootstrap.
+/// The logger initializes process-wide telemetry when necessary. It creates and clears
+/// request-specific activity state at <c>BuildStarted</c> and <c>BuildFinished</c>.
+/// </remarks>
 public sealed class MSBuildLogger : INodeLogger
 {
+    /// <summary>
+    /// The process-wide telemetry client used by this logger instance.
+    /// </summary>
+    /// <remarks>
+    /// The managed CLI initializes this client for in-process builds. Other hosts use the
+    /// parameterless constructor to initialize the same process-wide client.
+    /// </remarks>
     private readonly ITelemetryClient? _telemetry;
 
+    /// <summary>
+    /// The activity owned by the current build.
+    /// </summary>
+    /// <remarks>
+    /// This activity belongs to one build. It must not remain active after
+    /// <c>BuildFinished</c>. A persistent server can run later builds with unrelated parent
+    /// trace contexts in the same process.
+    /// </remarks>
+    private Activity? _activity;
+
     internal const string TargetFrameworkTelemetryEventName = "targetframeworkeval";
     internal const string BuildTelemetryEventName = "build";
     internal const string LoggingConfigurationTelemetryEventName = "loggingConfiguration";
@@ -59,16 +89,28 @@ public sealed class MSBuildLogger : INodeLogger
     /// </remarks>
     private Dictionary<string, Dictionary<string, int>> _aggregatedEvents = new();
 
+    /// <summary>
+    /// Initializes telemetry for the process hosting the logger.
+    /// </summary>
+    /// <remarks>
+    /// MSBuild uses the parameterless constructor to create loggers. The managed CLI can
+    /// initialize telemetry before an in-process build. A child process or persistent
+    /// server cannot depend on that initialization. The constructor reuses an existing
+    /// client to preserve CLI state. If no client exists, it creates one with the same
+    /// enablement and session behavior. Telemetry failures must not fail the build.
+    /// </remarks>
     public MSBuildLogger()
     {
         try
         {
             string? sessionId = Environment.GetEnvironmentVariable(EnvironmentVariableNames.DOTNET_CLI_TELEMETRY_SESSIONID);
 
-            if (sessionId != null)
+            if (!TelemetryClient.IsInitialized)
             {
-                _telemetry = new TelemetryClient(sessionId);
+                _ = new TelemetryClient(sessionId);
             }
+
+            _telemetry = TelemetryClient.Instance;
         }
         catch (Exception)
         {
@@ -84,11 +126,27 @@ internal MSBuildLogger(ITelemetryClient telemetry)
         _telemetry = telemetry;
     }
 
+    /// <summary>
+    /// Connects this node logger to MSBuild's event lifecycle.
+    /// </summary>
+    /// <remarks>
+    /// <see cref="INodeLogger"/> requires this node-count overload. Both overloads use the
+    /// same event subscriptions. Build events control the activity lifetime because a
+    /// server can run multiple builds. Each build can have different request context.
+    /// </remarks>
     public void Initialize(IEventSource eventSource, int nodeCount)
     {
         Initialize(eventSource);
     }
 
+    /// <summary>
+    /// Connects this logger to the events needed to collect telemetry and delimit a build.
+    /// </summary>
+    /// <remarks>
+    /// The logger subscribes to telemetry events and <c>BuildStarted</c> only when telemetry
+    /// is enabled. This avoids work for opted-out builds. The logger always subscribes to
+    /// <c>BuildFinished</c>. This lets the logger clear its activity before a later request.
+    /// </remarks>
     public void Initialize(IEventSource eventSource)
     {
         // Declare lack of dependency on having properties/items in ProjectStarted events
@@ -106,6 +164,8 @@ public void Initialize(IEventSource eventSource)
                 {
                     eventSource2.TelemetryLogged += OnTelemetryLogged;
                 }
+
+                eventSource.BuildStarted += OnBuildStarted;
             }
 
             eventSource.BuildFinished += OnBuildFinished;
@@ -116,11 +176,52 @@ public void Initialize(IEventSource eventSource)
         }
     }
 
+    /// <summary>
+    /// Starts the activity that contains telemetry for one MSBuild request.
+    /// </summary>
+    /// <remarks>
+    /// A persistent server can receive different environment and trace context for each
+    /// request. This method resolves the parent at <c>BuildStarted</c>, not in the
+    /// constructor. It uses the ambient activity for an in-process build. Otherwise, it
+    /// reads the context that the invoking CLI forwarded. The activity is internal because
+    /// it represents SDK work in the invoking command, not a remote client call.
+    /// </remarks>
+    private void OnBuildStarted(object sender, BuildStartedEventArgs e)
+    {
+        ActivityContext parentContext =
+            Activity.Current?.Context
+            ?? TelemetryClient.GetParentActivityContext()
+            ?? TelemetryClient.ParentActivityContext;
+        _activity = Activities.Source.StartActivity(
+            "msbuild",
+            ActivityKind.Internal,
+            parentContext);
+    }
+
+    /// <summary>
+    /// Completes telemetry and activity state for one MSBuild request.
+    /// </summary>
+    /// <remarks>
+    /// The logger attaches MSBuild events before it stops the activity. This order ensures
+    /// that exporters include the events when they capture the stopped activity. The method
+    /// sets the span status from the overall build result. It then clears the activity so a
+    /// later server build cannot use the completed activity as its parent.
+    /// </remarks>
     private void OnBuildFinished(object sender, BuildFinishedEventArgs e)
     {
         SendAggregatedEventsOnBuildFinished(_telemetry);
+        _activity?.SetStatus(e.Succeeded ? ActivityStatusCode.Ok : ActivityStatusCode.Error);
+        StopActivity();
     }
 
+    /// <summary>
+    /// Emits telemetry that is intentionally accumulated across nodes during a build.
+    /// </summary>
+    /// <remarks>
+    /// A persistent server retains process state for the next build. This method removes
+    /// each aggregate after it sends the aggregate. The next build cannot reuse counts from
+    /// the completed request.
+    /// </remarks>
     internal void SendAggregatedEventsOnBuildFinished(ITelemetryClient? telemetry)
     {
         if (telemetry is null) return;
@@ -250,7 +351,16 @@ private static void TrackEvent(ITelemetryClient? telemetry, string eventName, ID
             }
         }
 
-        telemetry?.TrackEvent(eventName, properties ?? eventProperties);
+        if (telemetry is TelemetryClient telemetryClient)
+        {
+            // Add production events before BuildFinished stops the activity.
+            // Test clients use ITelemetryClient without a real telemetry client.
+            telemetryClient.ThreadBlockingTrackEvent(eventName, properties ?? eventProperties);
+        }
+        else
+        {
+            telemetry?.TrackEvent(eventName, properties ?? eventProperties);
+        }
     }
 
     private void OnTelemetryLogged(object sender, TelemetryEventArgs args)
@@ -265,8 +375,40 @@ private void OnTelemetryLogged(object sender, TelemetryEventArgs args)
         }
     }
 
+    /// <summary>
+    /// Completes this MSBuild logger instance and writes its diagnostic telemetry log.
+    /// </summary>
+    /// <remarks>
+    /// <c>BuildFinished</c> normally stops the build activity. <see cref="Shutdown"/> also
+    /// stops the activity if an aborted build did not deliver <c>BuildFinished</c>. MSBuild
+    /// calls this method when the logger instance ends. The method waits for queued events
+    /// before it writes the diagnostic log. In a persistent server, logger shutdown does not
+    /// end the process. A later logger instance can reuse the process-wide telemetry client.
+    /// </remarks>
     public void Shutdown()
     {
+        StopActivity();
+
+        if (_telemetry is TelemetryClient telemetryClient)
+        {
+            telemetryClient.WaitForPendingEvents();
+        }
+
+        TelemetryClient.WriteLogIfNecessary();
+    }
+
+    /// <summary>
+    /// Stops only the activity owned by this logger and clears the reference.
+    /// </summary>
+    /// <remarks>
+    /// The invoking host owns the ambient parent activity. This method does not stop the
+    /// parent. Because this method clears the field, both <c>BuildFinished</c> and
+    /// <see cref="Shutdown"/> can call it safely.
+    /// </remarks>
+    private void StopActivity()
+    {
+        _activity?.Stop();
+        _activity = null;
     }
 
     public LoggerVerbosity Verbosity { get; set; }