[vs18.6] Fix telemetry allocation regression: per-engine collector ownership

eng/Versions.props

@@ -3,7 +3,7 @@
   <Import Project="Version.Details.props" />
 
   <PropertyGroup>
-    <VersionPrefix>18.6.1</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind><!-- Keep next to VersionPrefix to create a conflict in forward-flow -->
+    <VersionPrefix>18.6.3</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind><!-- Keep next to VersionPrefix to create a conflict in forward-flow -->
     <PreReleaseVersionLabel>servicing</PreReleaseVersionLabel>
     <PackageValidationBaselineVersion>18.5.0-preview-26126-01</PackageValidationBaselineVersion>
     <AssemblyVersion>15.1.0.0</AssemblyVersion>

src/Build.UnitTests/BackEnd/MockHost.cs

@@ -65,7 +65,7 @@ internal sealed class MockHost : MockLoggingService, IBuildComponentHost, IBuild
 
         private IBuildCheckManagerProvider _buildCheckManagerProvider;
 
-        private TelemetryForwarderProvider _telemetryForwarder;
+        private TelemetryCollectorProvider _telemetryCollector;
 
         #region SystemParameterFields
 
@@ -136,8 +136,8 @@ public MockHost(BuildParameters buildParameters, ConfigCache overrideConfigCache
             _buildCheckManagerProvider = new NullBuildCheckManagerProvider();
             ((IBuildComponent)_buildCheckManagerProvider).InitializeComponent(this);
 
-            _telemetryForwarder = new TelemetryForwarderProvider();
-            ((IBuildComponent)_telemetryForwarder).InitializeComponent(this);
+            _telemetryCollector = new TelemetryCollectorProvider();
+            ((IBuildComponent)_telemetryCollector).InitializeComponent(this);
         }
 
         /// <summary>
@@ -207,7 +207,7 @@ public IBuildComponent GetComponent(BuildComponentType type)
                 BuildComponentType.RequestBuilder => (IBuildComponent)_requestBuilder,
                 BuildComponentType.SdkResolverService => (IBuildComponent)_sdkResolverService,
                 BuildComponentType.BuildCheckManagerProvider => (IBuildComponent)_buildCheckManagerProvider,
-                BuildComponentType.TelemetryForwarder => (IBuildComponent)_telemetryForwarder,
+                BuildComponentType.TelemetryCollector => (IBuildComponent)_telemetryCollector,
                 _ => throw new ArgumentException("Unexpected type " + type),
             };
         }

src/Build.UnitTests/Telemetry/Telemetry_Tests.cs

@@ -486,39 +486,35 @@ public void IsEmpty_TrueForDefault_FalseAfterAdd()
         }
 
         [Fact]
-        public void FinalizeProcessing_AfterMerge_ResetsState()
+        public void TelemetryCollector_AccumulatesAndSendsOnFinalize()
         {
-            var forwarder = new TelemetryForwarderProvider.TelemetryForwarder();
+            var collector = new TelemetryCollectorProvider.TelemetryCollector();
             var loggingService = new EventRecordingLoggingService();
 
             var loggingContext = new MockLoggingContext(
                 loggingService,
                 new BuildEventContext(1, 2, BuildEventContext.InvalidProjectContextId, 4));
 
-            // Merge some data.
-            var localData = new WorkerNodeTelemetryData();
+            // Add data via the collector API.
             var key = new TaskOrTargetTelemetryKey("TestTarget", isCustom: true, isFromNugetCache: false, isFromMetaProject: false);
-            localData.AddTarget(key, wasExecuted: true);
-            forwarder.MergeWorkerData(localData);
+            collector.AddTarget(key, wasExecuted: true);
 
             // First FinalizeProcessing should emit a telemetry event.
-            forwarder.FinalizeProcessing(loggingContext);
+            collector.FinalizeProcessing(loggingContext);
             var telemetryEvents = loggingService.RecordedEvents.OfType<WorkerNodeTelemetryEventArgs>().ToList();
             telemetryEvents.Count.ShouldBe(1);
             telemetryEvents[0].WorkerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(key);
 
-            // Second FinalizeProcessing on an empty forwarder should be a no-op (state was reset).
-            forwarder.FinalizeProcessing(loggingContext);
+            // Second FinalizeProcessing on an empty collector should be a no-op (state was reset).
+            collector.FinalizeProcessing(loggingContext);
             loggingService.RecordedEvents.OfType<WorkerNodeTelemetryEventArgs>().Count().ShouldBe(1, "No new event should be emitted after reset");
 
-            // Merge new data after reset — forwarder should still work.
-            var localData2 = new WorkerNodeTelemetryData();
+            // Add new data after reset — collector should still work.
             var key2 = new TaskOrTargetTelemetryKey("TestTarget2", isCustom: false, isFromNugetCache: false, isFromMetaProject: false);
-            localData2.AddTarget(key2, wasExecuted: false, skipReason: TargetSkipReason.ConditionWasFalse);
-            forwarder.MergeWorkerData(localData2);
+            collector.AddTarget(key2, wasExecuted: false, skipReason: TargetSkipReason.ConditionWasFalse);
 
             // Third FinalizeProcessing should emit only the new data.
-            forwarder.FinalizeProcessing(loggingContext);
+            collector.FinalizeProcessing(loggingContext);
             var allTelemetryEvents = loggingService.RecordedEvents.OfType<WorkerNodeTelemetryEventArgs>().ToList();
             allTelemetryEvents.Count.ShouldBe(2);
             allTelemetryEvents[1].WorkerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(key2);

src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs

@@ -82,7 +82,7 @@ public void RegisterDefaultFactories()
             _componentEntriesByType[BuildComponentType.RequestBuilder] = new BuildComponentEntry(BuildComponentType.RequestBuilder, RequestBuilder.CreateComponent, CreationPattern.CreateAlways);
             // Following two conditionally registers real or no-op implementation based on BuildParameters
             _componentEntriesByType[BuildComponentType.BuildCheckManagerProvider] = new BuildComponentEntry(BuildComponentType.BuildCheckManagerProvider, BuildCheckManagerProvider.CreateComponent, CreationPattern.Singleton);
-            _componentEntriesByType[BuildComponentType.TelemetryForwarder] = new BuildComponentEntry(BuildComponentType.TelemetryForwarder, TelemetryForwarderProvider.CreateComponent, CreationPattern.Singleton);
+            _componentEntriesByType[BuildComponentType.TelemetryCollector] = new BuildComponentEntry(BuildComponentType.TelemetryCollector, TelemetryCollectorProvider.CreateComponent, CreationPattern.Singleton);
             _componentEntriesByType[BuildComponentType.TargetBuilder] = new BuildComponentEntry(BuildComponentType.TargetBuilder, TargetBuilder.CreateComponent, CreationPattern.CreateAlways);
             _componentEntriesByType[BuildComponentType.TaskBuilder] = new BuildComponentEntry(BuildComponentType.TaskBuilder, TaskBuilder.CreateComponent, CreationPattern.CreateAlways);
             _componentEntriesByType[BuildComponentType.RegisteredTaskObjectCache] = new BuildComponentEntry(BuildComponentType.RegisteredTaskObjectCache, RegisteredTaskObjectCache.CreateComponent, CreationPattern.Singleton);

src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs

@@ -202,6 +202,11 @@ public void InitializeForBuild(NodeLoggingContext loggingContext)
 
             _nodeLoggingContext = loggingContext;
 
+            // Create a per-BuildRequestEngine telemetry collector via the provider.
+            // Each BuildRequestEngine owns its collector — no cross-engine sharing, no singleton contention.
+            var telemetryProvider = (TelemetryCollectorProvider)_componentHost.GetComponent(BuildComponentType.TelemetryCollector);
+            _nodeLoggingContext.TelemetryCollector = telemetryProvider.CreateCollector();
+
             // Create a work queue that will take an action and invoke it.  The generic parameter is the type which ActionBlock.Post() will
             // take (an Action in this case) and the parameter to this constructor is a function which takes that parameter of type Action
             // (which we have named action) and does something with it (in this case calls invoke on it.)
@@ -296,9 +301,8 @@ public void CleanupForBuild()
                     IBuildCheckManagerProvider buildCheckProvider = (_componentHost.GetComponent(BuildComponentType.BuildCheckManagerProvider) as IBuildCheckManagerProvider);
                     var buildCheckManager = buildCheckProvider!.Instance;
                     buildCheckManager.FinalizeProcessing(_nodeLoggingContext);
-                    // Flush and send the final telemetry data if they are being collected
-                    ITelemetryForwarder telemetryForwarder = (_componentHost.GetComponent(BuildComponentType.TelemetryForwarder) as TelemetryForwarderProvider)!.Instance;
-                    telemetryForwarder.FinalizeProcessing(_nodeLoggingContext);
+                    // Flush and send the per-BuildRequestEngine telemetry data if any was collected.
+                    _nodeLoggingContext.TelemetryCollector?.FinalizeProcessing(_nodeLoggingContext);
                     // Clears the instance so that next call (on node reuse) to 'GetComponent' leads to reinitialization.
                     buildCheckProvider.ShutdownComponent();
                 },

src/Build/BackEnd/Components/IBuildComponentHost.cs

@@ -151,7 +151,7 @@ internal enum BuildComponentType
         /// <summary>
         /// The component which collects telemetry data in worker node and forwards it to the main node.
         /// </summary>
-        TelemetryForwarder,
+        TelemetryCollector,
     }
 
     /// <summary>

src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs

@@ -4,6 +4,7 @@
 using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
+using Microsoft.Build.TelemetryInfra;
 
 #nullable disable
 
@@ -34,6 +35,12 @@ internal NodeLoggingContext(ILoggingService loggingService, int nodeId, bool inP
             this.IsValid = true;
         }
 
+        /// <summary>
+        /// Per-<see cref="BuildRequestEngine"/> telemetry collector.
+        /// Null when telemetry collection is disabled.
+        /// </summary>
+        internal ITelemetryCollector TelemetryCollector { get; set; }
+
         /// <summary>
         /// Log the completion of a build
         /// </summary>

src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs

@@ -1297,11 +1297,9 @@ BuildResult CopyTargetResultsFromProxyTargetsToRealTargets(BuildResult resultFro
 
         private void UpdateStatisticsPostBuild()
         {
-            ITelemetryForwarder telemetryForwarder =
-                ((TelemetryForwarderProvider)_componentHost.GetComponent(BuildComponentType.TelemetryForwarder))
-                ?.Instance;
+            ITelemetryCollector telemetryCollector = _nodeLoggingContext?.TelemetryCollector;
 
-            if (telemetryForwarder == null || !telemetryForwarder.IsTelemetryCollected)
+            if (telemetryCollector is null || !telemetryCollector.IsTelemetryCollected)
             {
                 return;
             }
@@ -1316,9 +1314,6 @@ private void UpdateStatisticsPostBuild()
                 return;
             }
 
-            // Accumulate all telemetry into a local instance, then merge into the shared singleton once.
-            WorkerNodeTelemetryData telemetryData = new();
-
             foreach (var projectTargetInstance in _requestEntry.RequestConfiguration.Project.Targets)
             {
                 bool wasExecuted =
@@ -1351,14 +1346,12 @@ private void UpdateStatisticsPostBuild()
 
                 var key = new TaskOrTargetTelemetryKey(
                     projectTargetInstance.Key, isCustom, isFromNuget, isMetaprojTarget);
-                telemetryData.AddTarget(key, wasExecuted, skipReason);
+                telemetryCollector.AddTarget(key, wasExecuted, skipReason);
             }
 
             TaskRegistry taskReg = _requestEntry.RequestConfiguration.Project.TaskRegistry;
             CollectTasksStats(taskReg);
 
-            telemetryForwarder.MergeWorkerData(telemetryData);
-
             void CollectTasksStats(TaskRegistry taskRegistry)
             {
                 if (taskRegistry == null)
@@ -1373,7 +1366,7 @@ void CollectTasksStats(TaskRegistry taskRegistry)
                         registeredTaskRecord.ComputeIfCustom(),
                         registeredTaskRecord.IsFromNugetCache,
                         isFromMetaProject: false);
-                    telemetryData.AddTask(
+                    telemetryCollector.AddTask(
                         key,
                         registeredTaskRecord.Statistics.ExecutedTime,
                         registeredTaskRecord.Statistics.ExecutedCount,

src/Build/Microsoft.Build.csproj

@@ -181,8 +181,8 @@
     <Compile Include="Logging\TerminalLogger\**\*.cs" />
     <Compile Include="Logging\ReusableLogger.cs" />
     <Compile Include="TelemetryInfra\InternalTelemetryConsumingLogger.cs" />
-    <Compile Include="TelemetryInfra\ITelemetryForwarder.cs" />
-    <Compile Include="TelemetryInfra\TelemetryForwarderProvider.cs" />
+    <Compile Include="TelemetryInfra\ITelemetryCollector.cs" />
+    <Compile Include="TelemetryInfra\TelemetryCollectorProvider.cs" />
     <Compile Include="Utilities\AwaitExtensions.cs" />
     <Compile Include="Utilities\EncodingStringWriter.cs" />
     <Compile Include="Utilities\ProjectWriter.cs" />

src/Build/TelemetryInfra/ITelemetryCollector.cs

@@ -0,0 +1,36 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Microsoft.Build.BackEnd.Logging;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Framework.Telemetry;
+
+namespace Microsoft.Build.TelemetryInfra;
+
+/// <summary>
+/// Collects task and target telemetry for a single BuildRequestEngine's lifetime.
+/// Not thread-safe: only one RequestBuilder is active at a time per
+/// BuildRequestEngine, so <see cref="AddTarget"/> and <see cref="AddTask"/>
+/// are always called from a single thread.
+/// Created per BuildRequestEngine by <see cref="TelemetryCollectorProvider.CreateCollector"/>.
+/// </summary>
+internal interface ITelemetryCollector
+{
+    bool IsTelemetryCollected { get; }
+
+    void AddTarget(TaskOrTargetTelemetryKey key, bool wasExecuted, TargetSkipReason skipReason = TargetSkipReason.None);
+
+    void AddTask(
+        TaskOrTargetTelemetryKey key,
+        TimeSpan cumulativeExecutionTime,
+        int executionsCount,
+        long totalMemoryConsumed,
+        string? taskFactoryName,
+        string? taskHostRuntime);
+
+    /// <summary>
+    /// Sends accumulated telemetry as a <see cref="WorkerNodeTelemetryEventArgs"/> and resets for the next build.
+    /// </summary>
+    void FinalizeProcessing(LoggingContext loggingContext);
+}

src/Build/TelemetryInfra/TelemetryCollectorProvider.cs

@@ -0,0 +1,93 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Microsoft.Build.BackEnd;
+using Microsoft.Build.BackEnd.Logging;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Framework.Telemetry;
+using Microsoft.Build.Shared;
+
+namespace Microsoft.Build.TelemetryInfra;
+
+/// <summary>
+/// Build component that creates per-BuildRequestEngine <see cref="ITelemetryCollector"/> instances.
+/// Registered as a singleton, but holds no mutable state — each BuildRequestEngine gets its own
+/// collector via <see cref="CreateCollector"/>.
+/// </summary>
+internal class TelemetryCollectorProvider : IBuildComponent
+{
+    private bool _telemetryEnabled;
+
+    /// <summary>
+    /// Creates a new <see cref="ITelemetryCollector"/> scoped to one <see cref="BuildRequestEngine"/>'s build lifetime.
+    /// Returns a no-op collector when telemetry is disabled.
+    /// </summary>
+    internal ITelemetryCollector CreateCollector()
+        => _telemetryEnabled ? new TelemetryCollector() : NullTelemetryCollector.Instance;
+
+    internal static IBuildComponent CreateComponent(BuildComponentType type)
+    {
+        ErrorUtilities.VerifyThrow(type == BuildComponentType.TelemetryCollector, "Cannot create components of type {0}", type);
+        return new TelemetryCollectorProvider();
+    }
+
+    public void InitializeComponent(IBuildComponentHost host)
+    {
+        ErrorUtilities.VerifyThrow(host != null, "BuildComponentHost was null");
+        _telemetryEnabled = host!.BuildParameters.IsTelemetryEnabled;
+    }
+
+    public void ShutdownComponent()
+    {
+    }
+
+    /// <summary>
+    /// Collects task/target telemetry for one BuildRequestEngine. Not thread-safe —
+    /// only one RequestBuilder is active at a time per BuildRequestEngine.
+    /// </summary>
+    internal class TelemetryCollector : ITelemetryCollector
+    {
+        private WorkerNodeTelemetryData _data = new();
+
+        public bool IsTelemetryCollected => true;
+
+        public void AddTarget(TaskOrTargetTelemetryKey key, bool wasExecuted, TargetSkipReason skipReason = TargetSkipReason.None)
+        {
+            _data.AddTarget(key, wasExecuted, skipReason);
+        }
+
+        public void AddTask(TaskOrTargetTelemetryKey key, TimeSpan cumulativeExecutionTime, int executionsCount, long totalMemoryConsumed, string? taskFactoryName, string? taskHostRuntime)
+        {
+            _data.AddTask(key, cumulativeExecutionTime, executionsCount, totalMemoryConsumed, taskFactoryName, taskHostRuntime);
+        }
+
+        public void FinalizeProcessing(LoggingContext loggingContext)
+        {
+            if (_data.IsEmpty)
+            {
+                return;
+            }
+
+            WorkerNodeTelemetryData snapshot = _data;
+            _data = new();
+
+            WorkerNodeTelemetryEventArgs telemetryArgs = new(snapshot)
+            { BuildEventContext = loggingContext.BuildEventContext };
+            loggingContext.LogBuildEvent(telemetryArgs);
+        }
+    }
+
+    internal class NullTelemetryCollector : ITelemetryCollector
+    {
+        internal static readonly NullTelemetryCollector Instance = new();
+
+        public bool IsTelemetryCollected => false;
+
+        public void AddTarget(TaskOrTargetTelemetryKey key, bool wasExecuted, TargetSkipReason skipReason = TargetSkipReason.None) { }
+
+        public void AddTask(TaskOrTargetTelemetryKey key, TimeSpan cumulativeExecutionTime, int executionsCount, long totalMemoryConsumed, string? taskFactoryName, string? taskHostRuntime) { }
+
+        public void FinalizeProcessing(LoggingContext loggingContext) { }
+    }
+}