Fix telemetry logger to use distributed logging pattern with central …

…and forwarding loggers

Co-authored-by: baronfel <[email protected]>

src/Cli/dotnet/Commands/Run/RunCommand.cs

@@ -484,7 +484,9 @@ static ProjectInstance EvaluateProject(string? projectFilePath, Func<ProjectColl
 
             var globalProperties = CommonRunHelpers.GetGlobalPropertiesFromArgs(msbuildArgs);
 
-            var collection = new ProjectCollection(globalProperties: globalProperties, loggers: binaryLogger is null ? null : [binaryLogger], toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
+            // Include telemetry logger for evaluation
+            var loggers = ProjectInstanceExtensions.CreateLoggersWithTelemetry(binaryLogger is null ? null : [binaryLogger]);
+            var collection = new ProjectCollection(globalProperties: globalProperties, loggers: loggers, toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
 
             if (projectFilePath is not null)
             {

src/Cli/dotnet/Commands/Test/MTP/MSBuildUtility.cs

@@ -67,7 +67,9 @@ public static (IEnumerable<ParallelizableTestModuleGroupWithSequentialInnerModul
 
         FacadeLogger? logger = LoggerUtility.DetermineBinlogger([.. buildOptions.MSBuildArgs], dotnetTestVerb);
 
-        using var collection = new ProjectCollection(globalProperties, loggers: logger is null ? null : [logger], toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
+        // Include telemetry logger for evaluation
+        var loggers = ProjectInstanceExtensions.CreateLoggersWithTelemetry(logger is null ? null : [logger]);
+        using var collection = new ProjectCollection(globalProperties: globalProperties, loggers: loggers, toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
         var evaluationContext = EvaluationContext.Create(EvaluationContext.SharingPolicy.Shared);
         ConcurrentBag<ParallelizableTestModuleGroupWithSequentialInnerModules> projects = GetProjectsProperties(collection, evaluationContext, projectPaths, buildOptions);
         logger?.ReallyShutdown();
@@ -89,7 +91,9 @@ public static (IEnumerable<ParallelizableTestModuleGroupWithSequentialInnerModul
 
         var msbuildArgs = MSBuildArgs.AnalyzeMSBuildArguments(buildOptions.MSBuildArgs, CommonOptions.PropertiesOption, CommonOptions.RestorePropertiesOption, CommonOptions.MSBuildTargetOption(), CommonOptions.VerbosityOption(), CommonOptions.NoLogoOption());
 
-        using var collection = new ProjectCollection(globalProperties: CommonRunHelpers.GetGlobalPropertiesFromArgs(msbuildArgs), logger is null ? null : [logger], toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
+        // Include telemetry logger for evaluation
+        var loggers = ProjectInstanceExtensions.CreateLoggersWithTelemetry(logger is null ? null : [logger]);
+        using var collection = new ProjectCollection(globalProperties: CommonRunHelpers.GetGlobalPropertiesFromArgs(msbuildArgs), loggers, toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
         var evaluationContext = EvaluationContext.Create(EvaluationContext.SharingPolicy.Shared);
         IEnumerable<ParallelizableTestModuleGroupWithSequentialInnerModules> projects = SolutionAndProjectUtility.GetProjectProperties(projectFilePath, collection, evaluationContext, buildOptions, configuration: null, platform: null);
         logger?.ReallyShutdown();

src/Cli/dotnet/Extensions/ProjectInstanceExtensions.cs

@@ -3,6 +3,7 @@
 
 using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
+using Microsoft.Build.Logging;
 using Microsoft.DotNet.Cli.Commands.MSBuild;
 
 namespace Microsoft.DotNet.Cli.Extensions;
@@ -51,17 +52,18 @@ public static IEnumerable<string> GetConfigurations(this ProjectInstance project
     }
 
     /// <summary>
-    /// Creates telemetry loggers for API-based MSBuild usage if telemetry is enabled.
-    /// Returns null if telemetry is not enabled or if there's an error creating the loggers.
+    /// Creates the central telemetry logger for API-based MSBuild usage if telemetry is enabled.
+    /// This logger should be used for evaluation (ProjectCollection) and as a central logger for builds.
+    /// Returns null if telemetry is not enabled or if there's an error creating the logger.
     /// </summary>
-    /// <returns>A list of loggers to use with ProjectInstance.Build, or null if telemetry is disabled.</returns>
-    public static ILogger[]? CreateTelemetryLoggers()
+    /// <returns>The central telemetry logger, or null if telemetry is disabled.</returns>
+    public static ILogger? CreateTelemetryCentralLogger()
     {
         if (Telemetry.Telemetry.CurrentSessionId != null)
         {
             try
             {
-                return [new MSBuildLogger()];
+                return new MSBuildLogger();
             }
             catch (Exception)
             {
@@ -72,32 +74,72 @@ public static IEnumerable<string> GetConfigurations(this ProjectInstance project
     }
 
     /// <summary>
-    /// Builds the project with the specified targets, automatically including telemetry loggers.
+    /// Creates the forwarding logger record for distributed builds if telemetry is enabled.
+    /// This should be used with the remoteLoggers parameter of ProjectInstance.Build.
+    /// Returns an empty collection if telemetry is not enabled or if there's an error creating the logger.
+    /// </summary>
+    /// <returns>An array containing the forwarding logger record, or empty array if telemetry is disabled.</returns>
+    public static ForwardingLoggerRecord[] CreateTelemetryForwardingLoggerRecords()
+    {
+        if (Telemetry.Telemetry.CurrentSessionId != null)
+        {
+            try
+            {
+                var forwardingLogger = new MSBuildForwardingLogger();
+                var loggerRecord = new ForwardingLoggerRecord(forwardingLogger, new Microsoft.Build.Logging.LoggerDescription(
+                    loggerClassName: typeof(MSBuildLogger).FullName!,
+                    loggerAssemblyName: typeof(MSBuildLogger).Assembly.Location,
+                    loggerAssemblyFile: null,
+                    loggerSwitchParameters: null,
+                    verbosity: LoggerVerbosity.Normal));
+                return [loggerRecord];
+            }
+            catch (Exception)
+            {
+                // Exceptions during telemetry shouldn't cause anything else to fail
+            }
+        }
+        return [];
+    }
+
+    /// <summary>
+    /// Builds the project with the specified targets, automatically including telemetry loggers
+    /// as a distributed logger (central logger + forwarding logger).
     /// </summary>
     public static bool BuildWithTelemetry(
         this ProjectInstance projectInstance,
         string[] targets,
         IEnumerable<ILogger>? additionalLoggers = null)
     {
         var loggers = new List<ILogger>();
+        var forwardingLoggers = new List<ForwardingLoggerRecord>();
 
-        var telemetryLoggers = CreateTelemetryLoggers();
-        if (telemetryLoggers != null)
+        // Add central telemetry logger
+        var telemetryCentralLogger = CreateTelemetryCentralLogger();
+        if (telemetryCentralLogger != null)
         {
-            loggers.AddRange(telemetryLoggers);
+            loggers.Add(telemetryCentralLogger);
+
+            // Add forwarding logger for distributed builds
+            forwardingLoggers.AddRange(CreateTelemetryForwardingLoggerRecords());
         }
 
         if (additionalLoggers != null)
         {
             loggers.AddRange(additionalLoggers);
         }
 
-        return projectInstance.Build(targets, loggers.Count > 0 ? loggers : null);
+        // Use the overload that accepts forwarding loggers for proper distributed logging
+        return projectInstance.Build(
+            targets,
+            loggers.Count > 0 ? loggers : null,
+            forwardingLoggers.Count > 0 ? forwardingLoggers : null,
+            out _);
     }
 
     /// <summary>
-    /// Builds the project with the specified targets, automatically including telemetry loggers.
-    /// Overload for Build with targetOutputs parameter.
+    /// Builds the project with the specified targets, automatically including telemetry loggers
+    /// as a distributed logger (central logger + forwarding logger).
     /// </summary>
     public static bool BuildWithTelemetry(
         this ProjectInstance projectInstance,
@@ -106,45 +148,94 @@ public static bool BuildWithTelemetry(
         out IDictionary<string, TargetResult> targetOutputs)
     {
         var allLoggers = new List<ILogger>();
+        var forwardingLoggers = new List<ForwardingLoggerRecord>();
 
-        var telemetryLoggers = CreateTelemetryLoggers();
-        if (telemetryLoggers != null)
+        // Add central telemetry logger
+        var telemetryCentralLogger = CreateTelemetryCentralLogger();
+        if (telemetryCentralLogger != null)
         {
-            allLoggers.AddRange(telemetryLoggers);
+            allLoggers.Add(telemetryCentralLogger);
+
+            // Add forwarding logger for distributed builds
+            forwardingLoggers.AddRange(CreateTelemetryForwardingLoggerRecords());
         }
 
         if (loggers != null)
         {
             allLoggers.AddRange(loggers);
         }
 
-        return projectInstance.Build(targets, allLoggers.Count > 0 ? allLoggers : null, out targetOutputs);
+        // Use the overload that accepts forwarding loggers for proper distributed logging
+        return projectInstance.Build(
+            targets,
+            allLoggers.Count > 0 ? allLoggers : null,
+            forwardingLoggers.Count > 0 ? forwardingLoggers : null,
+            out targetOutputs);
     }
 
     /// <summary>
-    /// Builds the project with the specified targets, automatically including telemetry loggers.
-    /// Overload for Build with loggers, remoteLoggers, and targetOutputs parameters.
+    /// Builds the project with the specified targets, automatically including telemetry loggers
+    /// as a distributed logger (central logger + forwarding logger).
     /// </summary>
     public static bool BuildWithTelemetry(
         this ProjectInstance projectInstance,
         string[] targets,
         IEnumerable<ILogger>? loggers,
-        IEnumerable<Microsoft.Build.Logging.ForwardingLoggerRecord>? remoteLoggers,
+        IEnumerable<ForwardingLoggerRecord>? remoteLoggers,
         out IDictionary<string, TargetResult> targetOutputs)
     {
         var allLoggers = new List<ILogger>();
+        var allForwardingLoggers = new List<ForwardingLoggerRecord>();
 
-        var telemetryLoggers = CreateTelemetryLoggers();
-        if (telemetryLoggers != null)
+        // Add central telemetry logger
+        var telemetryCentralLogger = CreateTelemetryCentralLogger();
+        if (telemetryCentralLogger != null)
         {
-            allLoggers.AddRange(telemetryLoggers);
+            allLoggers.Add(telemetryCentralLogger);
+
+            // Add forwarding logger for distributed builds
+            allForwardingLoggers.AddRange(CreateTelemetryForwardingLoggerRecords());
         }
 
         if (loggers != null)
         {
             allLoggers.AddRange(loggers);
         }
 
-        return projectInstance.Build(targets, allLoggers.Count > 0 ? allLoggers : null, remoteLoggers, out targetOutputs);
+        if (remoteLoggers != null)
+        {
+            allForwardingLoggers.AddRange(remoteLoggers);
+        }
+
+        return projectInstance.Build(
+            targets,
+            allLoggers.Count > 0 ? allLoggers : null,
+            allForwardingLoggers.Count > 0 ? allForwardingLoggers : null,
+            out targetOutputs);
+    }
+
+    /// <summary>
+    /// Creates a logger collection that includes the telemetry central logger.
+    /// This is useful for ProjectCollection scenarios where evaluation needs telemetry.
+    /// </summary>
+    /// <param name="additionalLoggers">Additional loggers to include in the collection.</param>
+    /// <returns>An array of loggers including telemetry logger if enabled, or null if no loggers.</returns>
+    public static ILogger[]? CreateLoggersWithTelemetry(IEnumerable<ILogger>? additionalLoggers = null)
+    {
+        var loggers = new List<ILogger>();
+
+        // Add central telemetry logger for evaluation
+        var telemetryCentralLogger = CreateTelemetryCentralLogger();
+        if (telemetryCentralLogger != null)
+        {
+            loggers.Add(telemetryCentralLogger);
+        }
+
+        if (additionalLoggers != null)
+        {
+            loggers.AddRange(additionalLoggers);
+        }
+
+        return loggers.Count > 0 ? loggers.ToArray() : null;
     }
 }

test/dotnet.Tests/CommandTests/MSBuild/GivenProjectInstanceExtensions.cs

@@ -12,30 +12,29 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests;
 public class GivenProjectInstanceExtensions
 {
     [Fact]
-    public void CreateTelemetryLoggers_WhenTelemetryDisabled_ReturnsNull()
+    public void CreateTelemetryCentralLogger_WhenTelemetryDisabled_ReturnsNull()
     {
         // Ensure telemetry is disabled
         Telemetry.Telemetry.CurrentSessionId = null;
 
-        var loggers = ProjectInstanceExtensions.CreateTelemetryLoggers();
+        var logger = ProjectInstanceExtensions.CreateTelemetryCentralLogger();
 
-        loggers.Should().BeNull();
+        logger.Should().BeNull();
     }
 
     [Fact]
-    public void CreateTelemetryLoggers_WhenTelemetryEnabled_ReturnsLoggers()
+    public void CreateTelemetryCentralLogger_WhenTelemetryEnabled_ReturnsLogger()
     {
         // Enable telemetry with a session ID
         var originalSessionId = Telemetry.Telemetry.CurrentSessionId;
         try
         {
             Telemetry.Telemetry.CurrentSessionId = Guid.NewGuid().ToString();
 
-            var loggers = ProjectInstanceExtensions.CreateTelemetryLoggers();
+            var logger = ProjectInstanceExtensions.CreateTelemetryCentralLogger();
 
-            loggers.Should().NotBeNull();
-            loggers.Should().HaveCount(1);
-            loggers[0].Should().BeOfType<Commands.MSBuild.MSBuildLogger>();
+            logger.Should().NotBeNull();
+            logger.Should().BeOfType<Commands.MSBuild.MSBuildLogger>();
         }
         finally
         {
@@ -45,32 +44,55 @@ public void CreateTelemetryLoggers_WhenTelemetryEnabled_ReturnsLoggers()
     }
 
     [Fact]
-    public void BuildWithTelemetry_WhenTelemetryDisabled_CallsBuildWithoutTelemetryLogger()
+    public void CreateTelemetryForwardingLoggerRecords_WhenTelemetryDisabled_ReturnsEmpty()
     {
-        // This is a basic smoke test to ensure the extension method doesn't throw
-        // We can't easily test the actual build without setting up a full project
-
         // Ensure telemetry is disabled
         Telemetry.Telemetry.CurrentSessionId = null;
 
-        // CreateTelemetryLoggers should return null when telemetry is disabled
-        var loggers = ProjectInstanceExtensions.CreateTelemetryLoggers();
-        loggers.Should().BeNull();
+        var loggerRecords = ProjectInstanceExtensions.CreateTelemetryForwardingLoggerRecords();
+
+        loggerRecords.Should().BeEmpty();
     }
 
     [Fact]
-    public void BuildWithTelemetry_WhenTelemetryEnabled_CreatesTelemetryLogger()
+    public void CreateTelemetryForwardingLoggerRecords_WhenTelemetryEnabled_ReturnsLoggerRecords()
     {
         // Enable telemetry with a session ID
         var originalSessionId = Telemetry.Telemetry.CurrentSessionId;
         try
         {
             Telemetry.Telemetry.CurrentSessionId = Guid.NewGuid().ToString();
 
-            // CreateTelemetryLoggers should return logger when telemetry is enabled
-            var loggers = ProjectInstanceExtensions.CreateTelemetryLoggers();
-            loggers.Should().NotBeNull();
-            loggers.Should().HaveCount(1);
+            var loggerRecords = ProjectInstanceExtensions.CreateTelemetryForwardingLoggerRecords();
+
+            loggerRecords.Should().NotBeEmpty();
+            loggerRecords.Should().HaveCount(1);
+            // ForwardingLoggerRecord contains the ForwardingLogger and LoggerDescription
+            loggerRecords[0].Should().NotBeNull();
+        }
+        finally
+        {
+            // Restore original session ID
+            Telemetry.Telemetry.CurrentSessionId = originalSessionId;
+        }
+    }
+
+    [Fact]
+    public void BuildWithTelemetry_WhenTelemetryEnabled_CreatesDistributedLogger()
+    {
+        // Enable telemetry with a session ID
+        var originalSessionId = Telemetry.Telemetry.CurrentSessionId;
+        try
+        {
+            Telemetry.Telemetry.CurrentSessionId = Guid.NewGuid().ToString();
+
+            // CreateTelemetryCentralLogger should return logger when telemetry is enabled
+            var centralLogger = ProjectInstanceExtensions.CreateTelemetryCentralLogger();
+            centralLogger.Should().NotBeNull();
+
+            // CreateTelemetryForwardingLoggerRecords should return forwarding logger when telemetry is enabled
+            var forwardingLoggers = ProjectInstanceExtensions.CreateTelemetryForwardingLoggerRecords();
+            forwardingLoggers.Should().NotBeEmpty();
         }
         finally
         {