diff --git a/eng/Versions.props b/eng/Versions.props index efa0f034ce6..af333cfdead 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -3,7 +3,7 @@ - 18.6.1release + 18.6.3release servicing 18.5.0-preview-26126-01 15.1.0.0 diff --git a/src/Build.UnitTests/BackEnd/MockHost.cs b/src/Build.UnitTests/BackEnd/MockHost.cs index 3326ddfd49e..dbe06caf816 100644 --- a/src/Build.UnitTests/BackEnd/MockHost.cs +++ b/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); } /// @@ -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), }; } diff --git a/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs b/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs index 0361e99ca0d..dc69afdc8da 100644 --- a/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs +++ b/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().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().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().ToList(); allTelemetryEvents.Count.ShouldBe(2); allTelemetryEvents[1].WorkerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(key2); diff --git a/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs b/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs index 5e255466d52..872b2a62ab9 100644 --- a/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs +++ b/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); diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs index c194974c746..3a9d12b8a70 100644 --- a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs +++ b/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(); }, diff --git a/src/Build/BackEnd/Components/IBuildComponentHost.cs b/src/Build/BackEnd/Components/IBuildComponentHost.cs index 5c46a8ef807..d87cfd64853 100644 --- a/src/Build/BackEnd/Components/IBuildComponentHost.cs +++ b/src/Build/BackEnd/Components/IBuildComponentHost.cs @@ -151,7 +151,7 @@ internal enum BuildComponentType /// /// The component which collects telemetry data in worker node and forwards it to the main node. /// - TelemetryForwarder, + TelemetryCollector, } /// diff --git a/src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs b/src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs index e03c8ed13e7..cad9599829a 100644 --- a/src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs +++ b/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; } + /// + /// Per- telemetry collector. + /// Null when telemetry collection is disabled. + /// + internal ITelemetryCollector TelemetryCollector { get; set; } + /// /// Log the completion of a build /// diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 5f337c99148..d3ea99d4dff 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/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, diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 570acca3683..1ba4d5053e3 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -181,8 +181,8 @@ - - + + diff --git a/src/Build/TelemetryInfra/ITelemetryCollector.cs b/src/Build/TelemetryInfra/ITelemetryCollector.cs new file mode 100644 index 00000000000..7593f345e97 --- /dev/null +++ b/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; + +/// +/// 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 and +/// are always called from a single thread. +/// Created per BuildRequestEngine by . +/// +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); + + /// + /// Sends accumulated telemetry as a and resets for the next build. + /// + void FinalizeProcessing(LoggingContext loggingContext); +} diff --git a/src/Build/TelemetryInfra/ITelemetryForwarder.cs b/src/Build/TelemetryInfra/ITelemetryForwarder.cs deleted file mode 100644 index 18000c48280..00000000000 --- a/src/Build/TelemetryInfra/ITelemetryForwarder.cs +++ /dev/null @@ -1,26 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.Build.BackEnd.Logging; -using Microsoft.Build.Framework.Telemetry; - -namespace Microsoft.Build.TelemetryInfra; - -/// -/// A build component responsible for accumulating telemetry data from worker node and then sending it to main node -/// at the end of the build. -/// -internal interface ITelemetryForwarder -{ - bool IsTelemetryCollected { get; } - - /// - /// Merges a batch of telemetry data into this forwarder's accumulated state. - /// - void MergeWorkerData(IWorkerNodeTelemetryData data); - - /// - /// Sends accumulated telemetry and resets internal state. - /// - void FinalizeProcessing(LoggingContext loggingContext); -} diff --git a/src/Build/TelemetryInfra/TelemetryCollectorProvider.cs b/src/Build/TelemetryInfra/TelemetryCollectorProvider.cs new file mode 100644 index 00000000000..0c5aa8b958c --- /dev/null +++ b/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; + +/// +/// Build component that creates per-BuildRequestEngine instances. +/// Registered as a singleton, but holds no mutable state — each BuildRequestEngine gets its own +/// collector via . +/// +internal class TelemetryCollectorProvider : IBuildComponent +{ + private bool _telemetryEnabled; + + /// + /// Creates a new scoped to one 's build lifetime. + /// Returns a no-op collector when telemetry is disabled. + /// + 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() + { + } + + /// + /// Collects task/target telemetry for one BuildRequestEngine. Not thread-safe — + /// only one RequestBuilder is active at a time per BuildRequestEngine. + /// + 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) { } + } +} diff --git a/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs b/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs deleted file mode 100644 index dd851779bd9..00000000000 --- a/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs +++ /dev/null @@ -1,105 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.Build.BackEnd; -using Microsoft.Build.BackEnd.Logging; -using Microsoft.Build.Framework.Telemetry; -using Microsoft.Build.Shared; - -namespace Microsoft.Build.TelemetryInfra; - -/// -/// A build component responsible for accumulating telemetry data from worker node and then sending it to main node -/// at the end of the build. -/// -internal class TelemetryForwarderProvider : IBuildComponent -{ - private ITelemetryForwarder? _instance; - - public ITelemetryForwarder Instance => _instance ?? new NullTelemetryForwarder(); - - internal static IBuildComponent CreateComponent(BuildComponentType type) - { - ErrorUtilities.VerifyThrow(type == BuildComponentType.TelemetryForwarder, "Cannot create components of type {0}", type); - return new TelemetryForwarderProvider(); - } - - public void InitializeComponent(IBuildComponentHost host) - { - ErrorUtilities.VerifyThrow(host != null, "BuildComponentHost was null"); - - if (_instance == null) - { - if (host!.BuildParameters.IsTelemetryEnabled) - { - _instance = new TelemetryForwarder(); - } - else - { - _instance = new NullTelemetryForwarder(); - } - } - } - - public void ShutdownComponent() - { - /* Too late here for any communication to the main node or for logging anything. Just cleanup. */ - _instance = null; - } - - /// - /// Active telemetry forwarder that accumulates worker node telemetry. - /// - /// - /// Thread-safe: in /m /mt mode, multiple instances share a single - /// singleton, so and - /// may be called concurrently from different node threads. - /// - public class TelemetryForwarder : ITelemetryForwarder - { - private WorkerNodeTelemetryData _workerNodeTelemetryData = new(); - private readonly LockType _lock = new(); - - // in future, this might be per event type - public bool IsTelemetryCollected => true; - - public void MergeWorkerData(IWorkerNodeTelemetryData data) - { - lock (_lock) - { - _workerNodeTelemetryData.Add(data); - } - } - - - public void FinalizeProcessing(LoggingContext loggingContext) - { - WorkerNodeTelemetryData snapshot; - - lock (_lock) - { - // Nothing accumulated since the last call — skip sending. - if (_workerNodeTelemetryData.IsEmpty) - { - return; - } - - snapshot = _workerNodeTelemetryData; - _workerNodeTelemetryData = new(); - } - - WorkerNodeTelemetryEventArgs telemetryArgs = new(snapshot) - { BuildEventContext = loggingContext.BuildEventContext }; - loggingContext.LogBuildEvent(telemetryArgs); - } - } - - public class NullTelemetryForwarder : ITelemetryForwarder - { - public bool IsTelemetryCollected => false; - - public void MergeWorkerData(IWorkerNodeTelemetryData data) { } - - public void FinalizeProcessing(LoggingContext loggingContext) { } - } -}