diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 6949271d1d5..c880d138cf1 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -11,8 +11,90 @@ New files should use nullable types but don't refactor aggressively existing cod Generate tests for new codepaths, and add tests for any bugs you fix. Use the existing test framework, which is xUnit with Shouldly assertions. Use Shouldly assertions for all assertions in modified code, even if the file is predominantly using xUnit assertions. +When making changes, check if related documentation exists in the `documentation/` folder (including `documentation/specs/`) and update it to reflect your changes. Keep documentation in sync with code changes, especially for telemetry, APIs, and architectural decisions. + Always reference these instructions first and fallback to search or bash commands only when you encounter unexpected information that does not match the info here. +## Performance Best Practices + +MSBuild is performance-critical infrastructure. Follow these patterns: + +### Switch Expressions for Dispatch Logic +Use tuple switch expressions for multi-condition dispatch instead of if-else chains: +```csharp +// GOOD: Clean, O(1) dispatch +return (c0, c1) switch +{ + ('C', 'S') => Category.CSharp, + ('F', 'S') => Category.FSharp, + ('V', 'B') when value.Length >= 3 && value[2] == 'C' => Category.VB, + _ => Category.Other +}; + +// AVOID: Verbose if-else chains +if (c0 == 'C' && c1 == 'S') return Category.CSharp; +else if (c0 == 'F' && c1 == 'S') return Category.FSharp; +// ... +``` + +### Range Pattern Matching +Use range patterns for numeric categorization: +```csharp +// GOOD: Clear and efficient +return errorNumber switch +{ + >= 3001 and <= 3999 => Category.Tasks, + >= 4001 and <= 4099 => Category.General, + >= 4100 and <= 4199 => Category.Evaluation, + _ => Category.Other +}; +``` + +### String Comparisons +- Use `StringComparer.OrdinalIgnoreCase` for case-insensitive HashSets/Dictionaries when the source data may vary in casing +- Use `char.ToUpperInvariant()` for single-character comparisons +- Use `ReadOnlySpan` and `Slice()` to avoid string allocations when parsing substrings +- Use `int.TryParse(span, out var result)` on .NET Core+ for allocation-free parsing + +### Inlining +Mark small, hot-path methods with `[MethodImpl(MethodImplOptions.AggressiveInlining)]`: +```csharp +[MethodImpl(MethodImplOptions.AggressiveInlining)] +private static bool IsCompilerPrefix(string value) => ... +``` + +### Conditional Compilation for Framework Differences +Use `#if NET` for APIs that differ between .NET Framework and .NET Core: +```csharp +#if NET + return int.TryParse(span, out errorNumber); +#else + return int.TryParse(span.ToString(), out errorNumber); +#endif +``` + +### Immutable Collections +Choose the right immutable collection type based on usage pattern: + +**Build once, read many times** (most common in MSBuild): +- Use `ImmutableArray` instead of `ImmutableList` - significantly faster for read access +- Use `FrozenDictionary` instead of `ImmutableDictionary` - optimized for read-heavy scenarios + +**Build incrementally over time** (adding items one by one): +- Use `ImmutableList` and `ImmutableDictionary` - designed for efficient `Add` operations returning new collections + +```csharp +// GOOD: Build once from LINQ, then read many times +ImmutableArray items = source.Select(x => x.Name).ToImmutableArray(); +FrozenDictionary lookup = pairs.ToFrozenDictionary(x => x.Key, x => x.Value); + +// AVOID for read-heavy scenarios: +ImmutableList items = source.Select(x => x.Name).ToImmutableList(); +ImmutableDictionary lookup = pairs.ToImmutableDictionary(x => x.Key, x => x.Value); +``` + +Note: `ImmutableArray` is a value type. Use `IsDefault` property to check for uninitialized arrays, or use nullable `ImmutableArray?` with `.Value` to unwrap. + ## Working Effectively #### Bootstrap and Build the Repository diff --git a/documentation/VS-Telemetry-Data.md b/documentation/VS-Telemetry-Data.md new file mode 100644 index 00000000000..5f7693bcbae --- /dev/null +++ b/documentation/VS-Telemetry-Data.md @@ -0,0 +1,138 @@ +# MSBuild Visual Studio Telemetry Data + +This document describes the telemetry data collected by MSBuild and sent to Visual Studio telemetry infrastructure. The telemetry helps the MSBuild team understand build patterns, identify issues, and improve the build experience. + +## Overview + +MSBuild collects telemetry at multiple levels: +1. **Build-level telemetry** - Overall build metrics and outcomes +2. **Task-level telemetry** - Information about task execution +3. **Target-level telemetry** - Information about target execution and incrementality +4. **Error categorization telemetry** - Classification of build failures + +All telemetry events use the `VS/MSBuild/` prefix as required by VS exporting/collection. Properties use the `VS.MSBuild.` prefix. + +--- + +## 1. Build Telemetry (`build` event) + +The primary telemetry event capturing overall build information. + +### Properties + +| Property | Type | Description | +|----------|------|-------------| +| `BuildDurationInMilliseconds` | double | Total build duration from start to finish | +| `InnerBuildDurationInMilliseconds` | double | Duration from when BuildManager starts (excludes server connection time) | +| `BuildEngineHost` | string | Host environment: "VS", "VSCode", "Azure DevOps", "GitHub Action", "CLI", etc. | +| `BuildSuccess` | bool | Whether the build succeeded | +| `BuildTarget` | string | The target(s) being built | +| `BuildEngineVersion` | Version | MSBuild engine version | +| `BuildEngineDisplayVersion` | string | Display-friendly engine version | +| `BuildEngineFrameworkName` | string | Runtime framework name | +| `BuildCheckEnabled` | bool | Whether BuildCheck (static analysis) was enabled | +| `MultiThreadedModeEnabled` | bool | Whether multi-threaded build mode was enabled | +| `SACEnabled` | bool | Whether Smart Application Control was enabled | +| `IsStandaloneExecution` | bool | True if MSBuild runs from command line | +| `InitialMSBuildServerState` | string | Server state before build: "cold", "hot", or null | +| `ServerFallbackReason` | string | If server was bypassed: "ServerBusy", "ConnectionError", or null | +| `ProjectPath` | string | Path to the project file being built | +| `FailureCategory` | string | Primary failure category when build fails (see Error Categorization) | +| `ErrorCounts` | object | Breakdown of errors by category (see Error Categorization) | + +--- + +## 2. Error Categorization + +When a build fails, errors are categorized to help identify the source of failures. + +### Error Categories + +| Category | Error Code Prefixes | Description | +|----------|---------------------|-------------| +| `Compiler` | CS, FS, VBC | C#, F#, and Visual Basic compiler errors | +| `MsBuildGeneral` | MSB4001-MSB4099, MSB4500-MSB4999 | General MSBuild errors | +| `MsBuildEvaluation` | MSB4100-MSB4199 | Project evaluation errors | +| `MsBuildExecution` | MSB4300-MSB4399, MSB5xxx-MSB6xxx | Build execution errors | +| `MsBuildGraph` | MSB4400-MSB4499 | Static graph build errors | +| `Task` | MSB3xxx | Task-related errors | +| `SdkResolvers` | MSB4200-MSB4299 | SDK resolution errors | +| `NetSdk` | NETSDK | .NET SDK errors | +| `NuGet` | NU | NuGet package errors | +| `BuildCheck` | BC | BuildCheck rule violations | +| `NativeToolchain` | LNK, C1xxx-C4xxx, CL | Native C/C++ toolchain errors (linker, compiler) | +| `CodeAnalysis` | CA, IDE | Code analysis and IDE analyzer errors | +| `Razor` | RZ | Razor compilation errors | +| `Wpf` | XC, MC | WPF/XAML compilation errors | +| `AspNet` | ASP, BL | ASP.NET and Blazor errors | +| `Other` | (all others) | Uncategorized errors | + +## 3. Task Telemetry + +### Task Factory Event (`build/tasks/taskfactory`) + +Tracks which task factories are being used. + +| Property | Type | Description | +|----------|------|-------------| +| `AssemblyTaskFactoryTasksExecutedCount` | int | Tasks loaded via AssemblyTaskFactory | +| `IntrinsicTaskFactoryTasksExecutedCount` | int | Built-in intrinsic tasks | +| `CodeTaskFactoryTasksExecutedCount` | int | Tasks created via CodeTaskFactory | +| `RoslynCodeTaskFactoryTasksExecutedCount` | int | Tasks created via RoslynCodeTaskFactory | +| `XamlTaskFactoryTasksExecutedCount` | int | Tasks created via XamlTaskFactory | +| `CustomTaskFactoryTasksExecutedCount` | int | Tasks from custom task factories | + +## 4. Build Incrementality Telemetry + +Classifies builds as full or incremental based on target execution patterns. + +### Incrementality Info (Activity Property) + +| Field | Type | Description | +|-------|------|-------------| +| `Classification` | enum | `Full`, `Incremental`, or `Unknown` | +| `TotalTargetsCount` | int | Total number of targets | +| `ExecutedTargetsCount` | int | Targets that ran | +| `SkippedTargetsCount` | int | Targets that were skipped | +| `SkippedDueToUpToDateCount` | int | Skipped because outputs were current | +| `SkippedDueToConditionCount` | int | Skipped due to false condition | +| `SkippedDueToPreviouslyBuiltCount` | int | Skipped because already built | +| `IncrementalityRatio` | double | Ratio of skipped to total (0.0-1.0) | + +A build is classified as **Incremental** when more than 70% of targets are skipped. + +--- + +## Privacy Considerations + +### Data Hashing + +Custom and potentially sensitive data is hashed using SHA-256 before being sent: +- **Custom task names** - Hashed to protect proprietary task names +- **Custom target names** - Hashed to protect proprietary target names +- **Custom task factory names** - Hashed if not in the known list +- **Metaproj target names** - Hashed to protect solution structure + +### Known Task Factory Names (Not Hashed) + +The following Microsoft-owned task factory names are sent in plain text: +- `AssemblyTaskFactory` +- `TaskHostFactory` +- `CodeTaskFactory` +- `RoslynCodeTaskFactory` +- `XamlTaskFactory` +- `IntrinsicTaskFactory` + +## Related Files + +| File | Description | +|------|-------------| +| [BuildTelemetry.cs](../src/Framework/Telemetry/BuildTelemetry.cs) | Main build telemetry class | +| [BuildInsights.cs](../src/Framework/Telemetry/BuildInsights.cs) | Container for detailed insights | +| [TelemetryDataUtils.cs](../src/Framework/Telemetry/TelemetryDataUtils.cs) | Data transformation utilities | +| [BuildErrorTelemetryTracker.cs](../src/Build/BackEnd/Components/Logging/BuildErrorTelemetryTracker.cs) | Error categorization | +| [ProjectTelemetry.cs](../src/Build/BackEnd/Components/Logging/ProjectTelemetry.cs) | Per-project task telemetry | +| [LoggingConfigurationTelemetry.cs](../src/Framework/Telemetry/LoggingConfigurationTelemetry.cs) | Logger configuration | +| [BuildCheckTelemetry.cs](../src/Framework/Telemetry/BuildCheckTelemetry.cs) | BuildCheck telemetry | +| [KnownTelemetry.cs](../src/Framework/Telemetry/KnownTelemetry.cs) | Static telemetry accessors | +| [TelemetryConstants.cs](../src/Framework/Telemetry/TelemetryConstants.cs) | Telemetry naming constants | diff --git a/src/Build.UnitTests/BackEnd/BuildTelemetryErrorCategorization_Tests.cs b/src/Build.UnitTests/BackEnd/BuildTelemetryErrorCategorization_Tests.cs index e829d22461e..19e28f562b1 100644 --- a/src/Build.UnitTests/BackEnd/BuildTelemetryErrorCategorization_Tests.cs +++ b/src/Build.UnitTests/BackEnd/BuildTelemetryErrorCategorization_Tests.cs @@ -9,6 +9,7 @@ using Microsoft.Build.Shared; using Shouldly; using Xunit; +using static Microsoft.Build.BackEnd.Logging.BuildErrorTelemetryTracker; #nullable disable @@ -17,19 +18,19 @@ namespace Microsoft.Build.UnitTests.BackEnd; public class BuildTelemetryErrorCategorization_Tests { [Theory] - [InlineData("CS0103", null, "Compiler")] - [InlineData("CS1002", "CS", "Compiler")] - [InlineData("VBC30451", "VBC", "Compiler")] - [InlineData("FS0039", null, "Compiler")] - [InlineData("MSB4018", null, "MSBuildEngine")] - [InlineData("MSB4236", null, "SDKResolvers")] - [InlineData("MSB3026", null, "Tasks")] - [InlineData("NETSDK1045", null, "NETSDK")] - [InlineData("NU1101", null, "NuGet")] - [InlineData("BC0001", null, "BuildCheck")] - [InlineData("CUSTOM001", null, "Other")] - [InlineData(null, null, "Other")] - [InlineData("", null, "Other")] + [InlineData("CS0103", null, nameof(ErrorCategory.Compiler))] + [InlineData("CS1002", "CS", nameof(ErrorCategory.Compiler))] + [InlineData("VBC30451", "VBC", nameof(ErrorCategory.Compiler))] + [InlineData("FS0039", null, nameof(ErrorCategory.Compiler))] + [InlineData("MSB4018", null, nameof(ErrorCategory.MSBuildGeneral))] + [InlineData("MSB4236", null, nameof(ErrorCategory.SDKResolvers))] + [InlineData("MSB3026", null, nameof(ErrorCategory.Tasks))] + [InlineData("NETSDK1045", null, nameof(ErrorCategory.NETSDK))] + [InlineData("NU1101", null, nameof(ErrorCategory.NuGet))] + [InlineData("BC0001", null, nameof(ErrorCategory.BuildCheck))] + [InlineData("CUSTOM001", null, nameof(ErrorCategory.Other))] + [InlineData(null, null, nameof(ErrorCategory.Other))] + [InlineData("", null, nameof(ErrorCategory.Other))] public void ErrorCategorizationWorksCorrectly(string errorCode, string subcategory, string expectedCategory) { // Create a LoggingService @@ -63,28 +64,28 @@ public void ErrorCategorizationWorksCorrectly(string errorCode, string subcatego // Verify the appropriate count is incremented switch (expectedCategory) { - case "Compiler": + case nameof(ErrorCategory.Compiler): buildTelemetry.ErrorCounts.Compiler.ShouldBe(1); break; - case "MSBuildEngine": - buildTelemetry.ErrorCounts.MsBuildEngine.ShouldBe(1); + case nameof(ErrorCategory.MSBuildGeneral): + buildTelemetry.ErrorCounts.MsBuildGeneral.ShouldBe(1); break; - case "Tasks": + case nameof(ErrorCategory.Tasks): buildTelemetry.ErrorCounts.Task.ShouldBe(1); break; - case "SDKResolvers": + case nameof(ErrorCategory.SDKResolvers): buildTelemetry.ErrorCounts.SdkResolvers.ShouldBe(1); break; - case "NETSDK": + case nameof(ErrorCategory.NETSDK): buildTelemetry.ErrorCounts.NetSdk.ShouldBe(1); break; - case "NuGet": + case nameof(ErrorCategory.NuGet): buildTelemetry.ErrorCounts.NuGet.ShouldBe(1); break; - case "BuildCheck": + case nameof(ErrorCategory.BuildCheck): buildTelemetry.ErrorCounts.BuildCheck.ShouldBe(1); break; - case "Other": + case nameof(ErrorCategory.Other): buildTelemetry.ErrorCounts.Other.ShouldBe(1); break; } @@ -125,13 +126,13 @@ public void MultipleErrorsAreCountedByCategory() // Verify counts buildTelemetry.ErrorCounts.Compiler.ShouldBe(2); - buildTelemetry.ErrorCounts.MsBuildEngine.ShouldBe(1); + buildTelemetry.ErrorCounts.MsBuildGeneral.ShouldBe(1); buildTelemetry.ErrorCounts.Task.ShouldBe(1); buildTelemetry.ErrorCounts.NuGet.ShouldBe(1); buildTelemetry.ErrorCounts.Other.ShouldBe(1); // Primary category should be Compiler (highest count) - buildTelemetry.FailureCategory.ShouldBe("Compiler"); + buildTelemetry.FailureCategory.ShouldBe(nameof(ErrorCategory.Compiler)); } finally { @@ -166,7 +167,7 @@ public void PrimaryCategoryIsSetToHighestErrorCount() loggingService.PopulateBuildTelemetryWithErrors(buildTelemetry); // Primary category should be Tasks (3 errors vs 1 compiler error) - buildTelemetry.FailureCategory.ShouldBe("Tasks"); + buildTelemetry.FailureCategory.ShouldBe(nameof(ErrorCategory.Tasks)); buildTelemetry.ErrorCounts.Task.ShouldBe(3); buildTelemetry.ErrorCounts.Compiler.ShouldBe(1); } @@ -204,7 +205,7 @@ public void SubcategoryIsUsedForCompilerErrors() loggingService.PopulateBuildTelemetryWithErrors(buildTelemetry); // Should be categorized as Compiler based on subcategory - buildTelemetry.FailureCategory.ShouldBe("Compiler"); + buildTelemetry.FailureCategory.ShouldBe(nameof(ErrorCategory.Compiler)); buildTelemetry.ErrorCounts.Compiler.ShouldBe(1); } finally diff --git a/src/Build.UnitTests/BackEnd/KnownTelemetry_Tests.cs b/src/Build.UnitTests/BackEnd/KnownTelemetry_Tests.cs index 63fa6f3ae06..41afe33d841 100644 --- a/src/Build.UnitTests/BackEnd/KnownTelemetry_Tests.cs +++ b/src/Build.UnitTests/BackEnd/KnownTelemetry_Tests.cs @@ -7,6 +7,7 @@ using Microsoft.Build.Framework.Telemetry; using Shouldly; using Xunit; +using static Microsoft.Build.BackEnd.Logging.BuildErrorTelemetryTracker; using static Microsoft.Build.Framework.Telemetry.BuildInsights; namespace Microsoft.Build.UnitTests.Telemetry; @@ -131,29 +132,37 @@ public void BuildTelemetryIncludesFailureCategoryProperties() BuildTelemetry buildTelemetry = new BuildTelemetry(); buildTelemetry.BuildSuccess = false; - buildTelemetry.FailureCategory = "Compiler"; + buildTelemetry.FailureCategory = nameof(ErrorCategory.Compiler); buildTelemetry.ErrorCounts = new ErrorCountsInfo( Compiler: 5, - MsBuildEngine: 2, + MsBuildGeneral: 2, + MsBuildEvaluation: null, + MsBuildExecution: null, + MsBuildGraph: null, Task: 1, SdkResolvers: null, NetSdk: null, NuGet: 3, BuildCheck: null, + NativeToolchain: null, + CodeAnalysis: null, + Razor: null, + Wpf: null, + AspNet: null, Other: 1); var properties = buildTelemetry.GetProperties(); properties["BuildSuccess"].ShouldBe("False"); - properties["FailureCategory"].ShouldBe("Compiler"); + properties["FailureCategory"].ShouldBe(nameof(ErrorCategory.Compiler)); properties.ContainsKey("ErrorCounts").ShouldBeTrue(); var activityProperties = buildTelemetry.GetActivityProperties(); - activityProperties["FailureCategory"].ShouldBe("Compiler"); + activityProperties["FailureCategory"].ShouldBe(nameof(ErrorCategory.Compiler)); var errorCounts = activityProperties["ErrorCounts"] as ErrorCountsInfo; errorCounts.ShouldNotBeNull(); errorCounts.Compiler.ShouldBe(5); - errorCounts.MsBuildEngine.ShouldBe(2); + errorCounts.MsBuildGeneral.ShouldBe(2); errorCounts.Task.ShouldBe(1); errorCounts.NuGet.ShouldBe(3); errorCounts.Other.ShouldBe(1); @@ -168,21 +177,29 @@ public void BuildTelemetryActivityPropertiesIncludesFailureData() BuildTelemetry buildTelemetry = new BuildTelemetry(); buildTelemetry.BuildSuccess = false; - buildTelemetry.FailureCategory = "Tasks"; + buildTelemetry.FailureCategory = nameof(ErrorCategory.Tasks); buildTelemetry.ErrorCounts = new ErrorCountsInfo( Compiler: null, - MsBuildEngine: null, + MsBuildGeneral: null, + MsBuildEvaluation: null, + MsBuildExecution: null, + MsBuildGraph: null, Task: 10, SdkResolvers: null, NetSdk: null, NuGet: null, BuildCheck: null, + NativeToolchain: null, + CodeAnalysis: null, + Razor: null, + Wpf: null, + AspNet: null, Other: null); var activityProperties = buildTelemetry.GetActivityProperties(); activityProperties["BuildSuccess"].ShouldBe(false); - activityProperties["FailureCategory"].ShouldBe("Tasks"); + activityProperties["FailureCategory"].ShouldBe(nameof(ErrorCategory.Tasks)); var errorCounts = activityProperties["ErrorCounts"] as ErrorCountsInfo; errorCounts.ShouldNotBeNull(); errorCounts.Task.ShouldBe(10); diff --git a/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs b/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs index de1f2c063d6..d81776f1d77 100644 --- a/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs +++ b/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs @@ -54,7 +54,7 @@ public void WorkerNodeTelemetryCollection_BasicTarget() workerNodeTelemetryData!.ShouldNotBeNull(); var buildTargetKey = new TaskOrTargetTelemetryKey("Build", true, false); workerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(buildTargetKey); - workerNodeTelemetryData.TargetsExecutionData[buildTargetKey].ShouldBeTrue(); + workerNodeTelemetryData.TargetsExecutionData[buildTargetKey].WasExecuted.ShouldBeTrue(); workerNodeTelemetryData.TargetsExecutionData.Keys.Count.ShouldBe(1); workerNodeTelemetryData.TasksExecutionData.Keys.Count.ShouldBeGreaterThan(2); @@ -124,11 +124,11 @@ public void WorkerNodeTelemetryCollection_CustomTargetsAndTasks() workerNodeData!.ShouldNotBeNull(); workerNodeData.TargetsExecutionData.ShouldContainKey(new TaskOrTargetTelemetryKey("Build", true, false)); - workerNodeData.TargetsExecutionData[new TaskOrTargetTelemetryKey("Build", true, false)].ShouldBeTrue(); + workerNodeData.TargetsExecutionData[new TaskOrTargetTelemetryKey("Build", true, false)].WasExecuted.ShouldBeTrue(); workerNodeData.TargetsExecutionData.ShouldContainKey(new TaskOrTargetTelemetryKey("BeforeBuild", true, false)); - workerNodeData.TargetsExecutionData[new TaskOrTargetTelemetryKey("BeforeBuild", true, false)].ShouldBeTrue(); + workerNodeData.TargetsExecutionData[new TaskOrTargetTelemetryKey("BeforeBuild", true, false)].WasExecuted.ShouldBeTrue(); workerNodeData.TargetsExecutionData.ShouldContainKey(new TaskOrTargetTelemetryKey("NotExecuted", true, false)); - workerNodeData.TargetsExecutionData[new TaskOrTargetTelemetryKey("NotExecuted", true, false)].ShouldBeFalse(); + workerNodeData.TargetsExecutionData[new TaskOrTargetTelemetryKey("NotExecuted", true, false)].WasExecuted.ShouldBeFalse(); workerNodeData.TargetsExecutionData.Keys.Count.ShouldBe(3); workerNodeData.TasksExecutionData.Keys.Count.ShouldBeGreaterThan(2); @@ -204,7 +204,7 @@ public void TelemetryDataUtils_HashesCustomFactoryName() { new TaskOrTargetTelemetryKey("BuiltInTask", false, false), new TaskExecutionStats(TimeSpan.FromMilliseconds(50), 2, 500, "AssemblyTaskFactory", null) }, { new TaskOrTargetTelemetryKey("InlineTask", true, false), new TaskExecutionStats(TimeSpan.FromMilliseconds(75), 1, 750, "RoslynCodeTaskFactory", "CLR4") } }; - var targetsData = new Dictionary(); + var targetsData = new Dictionary(); var telemetryData = new WorkerNodeTelemetryData(tasksData, targetsData); var activityData = telemetryData.AsActivityDataHolder(includeTasksDetails: true, includeTargetDetails: false); @@ -388,5 +388,87 @@ private sealed class ProjectFinishedCapturingLogger : ILogger public void Shutdown() { } } + + [Fact] + public void BuildIncrementalityInfo_AllTargetsExecuted_ClassifiedAsFull() + { + // Arrange: All targets were executed (none skipped) + var targetsData = new Dictionary + { + { new TaskOrTargetTelemetryKey("Build", false, false), TargetExecutionStats.Executed() }, + { new TaskOrTargetTelemetryKey("Compile", false, false), TargetExecutionStats.Executed() }, + { new TaskOrTargetTelemetryKey("Link", false, false), TargetExecutionStats.Executed() }, + { new TaskOrTargetTelemetryKey("Pack", false, false), TargetExecutionStats.Executed() }, + }; + var tasksData = new Dictionary(); + var telemetryData = new WorkerNodeTelemetryData(tasksData, targetsData); + + // Act + var activityData = telemetryData.AsActivityDataHolder(includeTasksDetails: false, includeTargetDetails: false); + var properties = activityData!.GetActivityProperties(); + + // Assert + properties.ShouldContainKey("Incrementality"); + var incrementality = properties["Incrementality"] as BuildInsights.BuildIncrementalityInfo; + incrementality.ShouldNotBeNull(); + incrementality!.Classification.ShouldBe(BuildInsights.BuildType.Full); + incrementality.TotalTargetsCount.ShouldBe(4); + incrementality.ExecutedTargetsCount.ShouldBe(4); + incrementality.SkippedTargetsCount.ShouldBe(0); + incrementality.IncrementalityRatio.ShouldBe(0.0); + } + + [Fact] + public void BuildIncrementalityInfo_MostTargetsSkipped_ClassifiedAsIncremental() + { + // Arrange: Most targets were skipped (>70%) + var targetsData = new Dictionary + { + { new TaskOrTargetTelemetryKey("Build", false, false), TargetExecutionStats.Skipped(TargetSkipReason.OutputsUpToDate) }, + { new TaskOrTargetTelemetryKey("Compile", false, false), TargetExecutionStats.Skipped(TargetSkipReason.OutputsUpToDate) }, + { new TaskOrTargetTelemetryKey("Link", false, false), TargetExecutionStats.Skipped(TargetSkipReason.ConditionWasFalse) }, + { new TaskOrTargetTelemetryKey("Pack", false, false), TargetExecutionStats.Executed() }, // Only one executed + }; + var tasksData = new Dictionary(); + var telemetryData = new WorkerNodeTelemetryData(tasksData, targetsData); + + // Act + var activityData = telemetryData.AsActivityDataHolder(includeTasksDetails: false, includeTargetDetails: false); + var properties = activityData!.GetActivityProperties(); + + // Assert + properties.ShouldContainKey("Incrementality"); + var incrementality = properties["Incrementality"] as BuildInsights.BuildIncrementalityInfo; + incrementality.ShouldNotBeNull(); + incrementality!.Classification.ShouldBe(BuildInsights.BuildType.Incremental); + incrementality.TotalTargetsCount.ShouldBe(4); + incrementality.ExecutedTargetsCount.ShouldBe(1); + incrementality.SkippedTargetsCount.ShouldBe(3); + incrementality.SkippedDueToUpToDateCount.ShouldBe(2); + incrementality.SkippedDueToConditionCount.ShouldBe(1); + incrementality.SkippedDueToPreviouslyBuiltCount.ShouldBe(0); + incrementality.IncrementalityRatio.ShouldBe(0.75); + } + + [Fact] + public void BuildIncrementalityInfo_NoTargets_ClassifiedAsUnknown() + { + // Arrange: No targets + var targetsData = new Dictionary(); + var tasksData = new Dictionary(); + var telemetryData = new WorkerNodeTelemetryData(tasksData, targetsData); + + // Act + var activityData = telemetryData.AsActivityDataHolder(includeTasksDetails: false, includeTargetDetails: false); + var properties = activityData!.GetActivityProperties(); + + // Assert + properties.ShouldContainKey("Incrementality"); + var incrementality = properties["Incrementality"] as BuildInsights.BuildIncrementalityInfo; + incrementality.ShouldNotBeNull(); + incrementality!.Classification.ShouldBe(BuildInsights.BuildType.Unknown); + incrementality.TotalTargetsCount.ShouldBe(0); + incrementality.IncrementalityRatio.ShouldBe(0.0); + } } } diff --git a/src/Build/BackEnd/Components/Logging/BuildErrorTelemetryTracker.cs b/src/Build/BackEnd/Components/Logging/BuildErrorTelemetryTracker.cs index a2d4dc179c0..dc5c5d0d9ed 100644 --- a/src/Build/BackEnd/Components/Logging/BuildErrorTelemetryTracker.cs +++ b/src/Build/BackEnd/Components/Logging/BuildErrorTelemetryTracker.cs @@ -16,15 +16,23 @@ namespace Microsoft.Build.BackEnd.Logging internal sealed class BuildErrorTelemetryTracker { // Use an enum internally for efficient tracking, convert to string only when needed - private enum ErrorCategory + internal enum ErrorCategory { Compiler, - MSBuildEngine, + MSBuildGeneral, + MSBuildEvaluation, + MSBuildExecution, + MSBuildGraph, Tasks, SDKResolvers, NETSDK, NuGet, BuildCheck, + NativeToolchain, + CodeAnalysis, + Razor, + WPF, + AspNet, Other, } @@ -76,36 +84,44 @@ public void TrackError(string? errorCode, string? subcategory) /// The BuildTelemetry object to populate with error data. public void PopulateBuildTelemetry(BuildTelemetry buildTelemetry) { - // Read counts atomically - int compilerErrorCount = System.Threading.Interlocked.CompareExchange(ref _errorCounts[(int)ErrorCategory.Compiler], 0, 0); - int msbuildEngineErrorCount = System.Threading.Interlocked.CompareExchange(ref _errorCounts[(int)ErrorCategory.MSBuildEngine], 0, 0); - int taskErrorCount = System.Threading.Interlocked.CompareExchange(ref _errorCounts[(int)ErrorCategory.Tasks], 0, 0); - int sdkResolversErrorCount = System.Threading.Interlocked.CompareExchange(ref _errorCounts[(int)ErrorCategory.SDKResolvers], 0, 0); - int netsdkErrorCount = System.Threading.Interlocked.CompareExchange(ref _errorCounts[(int)ErrorCategory.NETSDK], 0, 0); - int nugetErrorCount = System.Threading.Interlocked.CompareExchange(ref _errorCounts[(int)ErrorCategory.NuGet], 0, 0); - int buildCheckErrorCount = System.Threading.Interlocked.CompareExchange(ref _errorCounts[(int)ErrorCategory.BuildCheck], 0, 0); - int otherErrorCount = System.Threading.Interlocked.CompareExchange(ref _errorCounts[(int)ErrorCategory.Other], 0, 0); - buildTelemetry.ErrorCounts = new ErrorCountsInfo( - Compiler: compilerErrorCount > 0 ? compilerErrorCount : null, - MsBuildEngine: msbuildEngineErrorCount > 0 ? msbuildEngineErrorCount : null, - Task: taskErrorCount > 0 ? taskErrorCount : null, - SdkResolvers: sdkResolversErrorCount > 0 ? sdkResolversErrorCount : null, - NetSdk: netsdkErrorCount > 0 ? netsdkErrorCount : null, - NuGet: nugetErrorCount > 0 ? nugetErrorCount : null, - BuildCheck: buildCheckErrorCount > 0 ? buildCheckErrorCount : null, - Other: otherErrorCount > 0 ? otherErrorCount : null); + Compiler: GetCountOrNull(ErrorCategory.Compiler), + MsBuildGeneral: GetCountOrNull(ErrorCategory.MSBuildGeneral), + MsBuildEvaluation: GetCountOrNull(ErrorCategory.MSBuildEvaluation), + MsBuildExecution: GetCountOrNull(ErrorCategory.MSBuildExecution), + MsBuildGraph: GetCountOrNull(ErrorCategory.MSBuildGraph), + Task: GetCountOrNull(ErrorCategory.Tasks), + SdkResolvers: GetCountOrNull(ErrorCategory.SDKResolvers), + NetSdk: GetCountOrNull(ErrorCategory.NETSDK), + NuGet: GetCountOrNull(ErrorCategory.NuGet), + BuildCheck: GetCountOrNull(ErrorCategory.BuildCheck), + NativeToolchain: GetCountOrNull(ErrorCategory.NativeToolchain), + CodeAnalysis: GetCountOrNull(ErrorCategory.CodeAnalysis), + Razor: GetCountOrNull(ErrorCategory.Razor), + Wpf: GetCountOrNull(ErrorCategory.WPF), + AspNet: GetCountOrNull(ErrorCategory.AspNet), + Other: GetCountOrNull(ErrorCategory.Other)); // Set the primary failure category - int totalErrors = System.Threading.Interlocked.CompareExchange(ref _primaryCategoryCount, 0, 0); - if (totalErrors > 0) + if (_primaryCategoryCount > 0) { buildTelemetry.FailureCategory = _primaryCategory.ToString(); } } + /// + /// Gets the error count for a category, returning null if zero. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private int? GetCountOrNull(ErrorCategory category) + { + int count = System.Threading.Interlocked.CompareExchange(ref _errorCounts[(int)category], 0, 0); + return count > 0 ? count : null; + } + /// /// Categorizes an error based on its error code and subcategory. + /// Uses a two-level character switch for O(1) prefix matching. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] private static ErrorCategory CategorizeError(string? errorCode, string? subcategory) @@ -121,54 +137,82 @@ private static ErrorCategory CategorizeError(string? errorCode, string? subcateg return ErrorCategory.Compiler; } - // Check error code patterns - order by frequency for fast path + // Check error code patterns if (IsCompilerPrefix(errorCode!)) { return ErrorCategory.Compiler; } - // Use Span-based comparison to avoid allocations - ReadOnlySpan codeSpan = errorCode.AsSpan(); + if (errorCode!.Length < 2) + { + return ErrorCategory.Other; + } - if (codeSpan.Length >= 2) + // Two-level switch on first two characters for efficient prefix matching + char c0 = char.ToUpperInvariant(errorCode[0]); + char c1 = char.ToUpperInvariant(errorCode[1]); + + return (c0, c1) switch { - char c0 = char.ToUpperInvariant(codeSpan[0]); - char c1 = char.ToUpperInvariant(codeSpan[1]); + // A* + ('A', 'S') when StartsWithASP(errorCode) => ErrorCategory.AspNet, - // BC* -> BuildCheck - if (c0 == 'B' && c1 == 'C') - { - return ErrorCategory.BuildCheck; - } + // B* + ('B', 'C') => ErrorCategory.BuildCheck, + ('B', 'L') => ErrorCategory.AspNet, // Blazor - // NU* -> NuGet - if (c0 == 'N' && c1 == 'U') - { - return ErrorCategory.NuGet; - } + // C* (careful: CS is handled by IsCompilerPrefix above) + ('C', 'A') => ErrorCategory.CodeAnalysis, + ('C', 'L') => ErrorCategory.NativeToolchain, + ('C', 'V') when errorCode.Length >= 3 && char.ToUpperInvariant(errorCode[2]) == 'T' => ErrorCategory.NativeToolchain, // CVT* + ('C', >= '0' and <= '9') => ErrorCategory.NativeToolchain, // C1*, C2*, C4* (C/C++ compiler) - // MSB* -> categorize MSB errors - if (c0 == 'M' && c1 == 'S' && codeSpan.Length >= 3 && char.ToUpperInvariant(codeSpan[2]) == 'B') - { - return CategorizeMSBError(codeSpan); - } + // I* + ('I', 'D') when errorCode.Length >= 3 && char.ToUpperInvariant(errorCode[2]) == 'E' => ErrorCategory.CodeAnalysis, // IDE* - // NETSDK* -> .NET SDK diagnostics - if (c0 == 'N' && c1 == 'E' && codeSpan.Length >= 6 && - char.ToUpperInvariant(codeSpan[2]) == 'T' && - char.ToUpperInvariant(codeSpan[3]) == 'S' && - char.ToUpperInvariant(codeSpan[4]) == 'D' && - char.ToUpperInvariant(codeSpan[5]) == 'K') - { - return ErrorCategory.NETSDK; - } - } + // L* + ('L', 'N') when errorCode.Length >= 3 && char.ToUpperInvariant(errorCode[2]) == 'K' => ErrorCategory.NativeToolchain, // LNK* + + // M* + ('M', 'C') => ErrorCategory.WPF, // MC* (Markup Compiler) + ('M', 'S') when errorCode.Length >= 3 && char.ToUpperInvariant(errorCode[2]) == 'B' => CategorizeMSBError(errorCode.AsSpan()), + ('M', 'T') => ErrorCategory.NativeToolchain, // MT* (Manifest Tool) + + // N* + ('N', 'E') when StartsWithNETSDK(errorCode) => ErrorCategory.NETSDK, + ('N', 'U') => ErrorCategory.NuGet, - return ErrorCategory.Other; + // R* + ('R', 'C') => ErrorCategory.NativeToolchain, // RC* (Resource Compiler) + ('R', 'Z') => ErrorCategory.Razor, + + // X* + ('X', 'C') => ErrorCategory.WPF, // XC* (XAML Compiler) + + _ => ErrorCategory.Other + }; } /// - /// Checks if the string starts with a compiler error prefix (CS, VBC, FS). + /// Checks if the error code starts with "ASP" (case-insensitive). + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool StartsWithASP(string errorCode) + => errorCode.Length >= 3 && char.ToUpperInvariant(errorCode[2]) == 'P'; + + /// + /// Checks if the error code starts with "NETSDK" (case-insensitive). + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool StartsWithNETSDK(string errorCode) + => errorCode.Length >= 6 && + char.ToUpperInvariant(errorCode[2]) == 'T' && + char.ToUpperInvariant(errorCode[3]) == 'S' && + char.ToUpperInvariant(errorCode[4]) == 'D' && + char.ToUpperInvariant(errorCode[5]) == 'K'; + + /// + /// Checks if the string starts with a compiler error prefix (CS, FS, VBC). /// [MethodImpl(MethodImplOptions.AggressiveInlining)] private static bool IsCompilerPrefix(string value) @@ -181,83 +225,55 @@ private static bool IsCompilerPrefix(string value) char c0 = char.ToUpperInvariant(value[0]); char c1 = char.ToUpperInvariant(value[1]); - // CS* -> C# compiler - if (c0 == 'C' && c1 == 'S') - { - return true; - } - - // FS* -> F# compiler - if (c0 == 'F' && c1 == 'S') - { - return true; - } - - // VBC* -> VB compiler (need 3 chars) - if (c0 == 'V' && c1 == 'B' && value.Length >= 3 && char.ToUpperInvariant(value[2]) == 'C') + return (c0, c1) switch { - return true; - } - - return false; + ('C', 'S') => true, // CS* -> C# compiler + ('F', 'S') => true, // FS* -> F# compiler + ('V', 'B') => value.Length >= 3 && char.ToUpperInvariant(value[2]) == 'C', // VBC* -> VB compiler + _ => false + }; } /// - /// Categorizes MSB error codes into MSBuildEngine, Tasks, or SDKResolvers. + /// Categorizes MSB error codes into granular MSBuild categories. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] private static ErrorCategory CategorizeMSBError(ReadOnlySpan codeSpan) { - // MSB error codes consist of 3-letter prefix + 4-digit number (e.g., MSB3026) - const int MinimumMsbCodeLength = 7; - - if (codeSpan.Length < MinimumMsbCodeLength) - { - return ErrorCategory.Other; - } - - // Check for MSB4236 (SDKResolvers error) - fast path for exact match - if (codeSpan.Length == 7 && codeSpan[3] == '4' && codeSpan[4] == '2' && codeSpan[5] == '3' && codeSpan[6] == '6') - { - return ErrorCategory.SDKResolvers; - } - - if (!TryParseErrorNumber(codeSpan, out int errorNumber)) + // MSB error codes: 3-letter prefix + 4-digit number (e.g., MSB3026) + if (codeSpan.Length < 7 || !TryParseErrorNumber(codeSpan, out int errorNumber)) { return ErrorCategory.Other; } - // MSB4xxx (except MSB4236, handled above as SDKResolvers) -> MSBuildEngine (evaluation and execution errors) - if (errorNumber is >= 4001 and <= 4999) + return errorNumber switch { - return ErrorCategory.MSBuildEngine; - } - - // MSB3xxx -> Tasks - return errorNumber is >= 3001 and <= 3999 ? ErrorCategory.Tasks : ErrorCategory.Other; + >= 3001 and <= 3999 => ErrorCategory.Tasks, + >= 4001 and <= 4099 => ErrorCategory.MSBuildGeneral, + >= 4100 and <= 4199 => ErrorCategory.MSBuildEvaluation, + >= 4200 and <= 4299 => ErrorCategory.SDKResolvers, + >= 4300 and <= 4399 => ErrorCategory.MSBuildExecution, + >= 4400 and <= 4499 => ErrorCategory.MSBuildGraph, + >= 4500 and <= 4999 => ErrorCategory.MSBuildGeneral, + >= 5001 and <= 5999 => ErrorCategory.MSBuildExecution, + >= 6001 and <= 6999 => ErrorCategory.MSBuildExecution, + _ => ErrorCategory.Other + }; } /// - /// Parses the 4-digit error number from an MSB error code span (starting at index 3). + /// Parses the 4-digit error number from an MSB error code span (e.g., "MSB3026" -> 3026). /// [MethodImpl(MethodImplOptions.AggressiveInlining)] private static bool TryParseErrorNumber(ReadOnlySpan codeSpan, out int errorNumber) { - errorNumber = 0; - - // We need exactly 4 digits starting at position 3 - for (int i = 3; i < 7; i++) - { - char c = codeSpan[i]; - if (c < '0' || c > '9') - { - return false; - } - - errorNumber = (errorNumber * 10) + (c - '0'); - } - - return true; + // Extract digits after "MSB" prefix (positions 3-6) + ReadOnlySpan digits = codeSpan.Slice(3, 4); +#if NET + return int.TryParse(digits, out errorNumber); +#else + return int.TryParse(digits.ToString(), out errorNumber); +#endif } } } diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 24dae3c05f3..2709e004fcb 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1292,6 +1292,11 @@ private void UpdateStatisticsPostBuild() // E.g. _SourceLinkHasSingleProvider can be brought explicitly via nuget (Microsoft.SourceLink.GitHub) as well as sdk projectTargetInstance.Value.Location.Equals(targetResult.TargetLocation); + // Get skip reason from TargetResult - it's set when targets are skipped for various reasons: + // - ConditionWasFalse: target's condition evaluated to false + // - PreviouslyBuiltSuccessfully/Unsuccessfully: target was already built in this session + TargetSkipReason skipReason = targetResult?.SkipReason ?? TargetSkipReason.None; + bool isFromNuget, isMetaprojTarget, isCustom; if (IsMetaprojTargetPath(projectTargetInstance.Value.FullPath)) @@ -1316,7 +1321,8 @@ private void UpdateStatisticsPostBuild() wasExecuted, isCustom, isMetaprojTarget, - isFromNuget); + isFromNuget, + skipReason); } TaskRegistry taskReg = _requestEntry.RequestConfiguration.Project.TaskRegistry; diff --git a/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs index 65b6903876a..d1b8b636f25 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs @@ -566,6 +566,10 @@ private bool CheckSkipTarget(ref bool stopProcessingStack, TargetEntry currentTa // If we've already dealt with this target and it didn't skip, let's log appropriately // Otherwise we don't want anything more to do with it. bool success = targetResult.ResultCode == TargetResultCode.Success; + + // Update the skip reason on the existing result for telemetry purposes + targetResult.SkipReason = success ? TargetSkipReason.PreviouslyBuiltSuccessfully : TargetSkipReason.PreviouslyBuiltUnsuccessfully; + var skippedTargetEventArgs = new TargetSkippedEventArgs(message: null) { BuildEventContext = _projectLoggingContext.BuildEventContext, @@ -574,7 +578,7 @@ private bool CheckSkipTarget(ref bool stopProcessingStack, TargetEntry currentTa ParentTarget = currentTargetEntry.ParentEntry?.Target.Name, BuildReason = currentTargetEntry.BuildReason, OriginallySucceeded = success, - SkipReason = success ? TargetSkipReason.PreviouslyBuiltSuccessfully : TargetSkipReason.PreviouslyBuiltUnsuccessfully, + SkipReason = targetResult.SkipReason, OriginalBuildEventContext = targetResult.OriginalBuildEventContext }; diff --git a/src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs b/src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs index 405d5421172..ea2914858f9 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs @@ -364,7 +364,8 @@ internal List GetDependencies(ProjectLoggingContext project _targetResult = new TargetResult( Array.Empty(), new WorkUnitResult(WorkUnitResultCode.Skipped, WorkUnitActionCode.Continue, null), - projectLoggingContext.BuildEventContext); + projectLoggingContext.BuildEventContext, + TargetSkipReason.ConditionWasFalse); _state = TargetEntryState.Completed; if (projectLoggingContext.LoggingService.MinimumRequiredMessageImportance > MessageImportance.Low && diff --git a/src/Build/BackEnd/Shared/TargetResult.cs b/src/Build/BackEnd/Shared/TargetResult.cs index 14ce318f869..efb94758c54 100644 --- a/src/Build/BackEnd/Shared/TargetResult.cs +++ b/src/Build/BackEnd/Shared/TargetResult.cs @@ -56,6 +56,11 @@ public class TargetResult : ITargetResult, ITranslatable /// private BuildEventContext _originalBuildEventContext; + /// + /// The reason why the target was skipped, if applicable. + /// + private TargetSkipReason _skipReason; + /// /// Initializes the results with specified items and result. /// @@ -68,13 +73,15 @@ public class TargetResult : ITargetResult, ITranslatable /// * in when Cancellation was requested /// * in ProjectCache.CacheResult.ConstructBuildResult /// - internal TargetResult(TaskItem[] items, WorkUnitResult result, BuildEventContext originalBuildEventContext = null) + /// The reason why the target was skipped, if applicable. + internal TargetResult(TaskItem[] items, WorkUnitResult result, BuildEventContext originalBuildEventContext = null, TargetSkipReason skipReason = TargetSkipReason.None) { ErrorUtilities.VerifyThrowArgumentNull(items); ErrorUtilities.VerifyThrowArgumentNull(result); _items = items; _result = result; _originalBuildEventContext = originalBuildEventContext; + _skipReason = skipReason; } /// @@ -149,6 +156,19 @@ public TargetResultCode ResultCode } } + /// + /// Gets the reason why the target was skipped, if applicable. + /// + /// The reason for skipping, or if the target was not skipped or the reason is unknown. + internal TargetSkipReason SkipReason + { + [DebuggerStepThrough] + get => _skipReason; + + [DebuggerStepThrough] + set => _skipReason = value; + } + public string TargetResultCodeToString() { switch (ResultCode) @@ -323,6 +343,7 @@ private void InternalTranslate(ITranslator translator) translator.Translate(ref _targetFailureDoesntCauseBuildFailure); translator.Translate(ref _afterTargetsHaveFailed); translator.TranslateOptionalBuildEventContext(ref _originalBuildEventContext); + translator.TranslateEnum(ref _skipReason, (int)_skipReason); TranslateItems(translator); } diff --git a/src/Build/TelemetryInfra/ITelemetryForwarder.cs b/src/Build/TelemetryInfra/ITelemetryForwarder.cs index 98908828c52..916661a7aa2 100644 --- a/src/Build/TelemetryInfra/ITelemetryForwarder.cs +++ b/src/Build/TelemetryInfra/ITelemetryForwarder.cs @@ -3,6 +3,7 @@ using System; using Microsoft.Build.BackEnd.Logging; +using Microsoft.Build.Framework; namespace Microsoft.Build.TelemetryInfra; @@ -27,12 +28,13 @@ void AddTask( /// /// Add info about target execution to the telemetry. /// - /// - /// Means anytime, not necessarily from the last time target was added to telemetry - /// - /// - /// - void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache); + /// The target name. + /// Whether the target was executed (not skipped). + /// Whether this is a custom target. + /// Whether the target is from a meta project. + /// Whether the target is from a NuGet package. + /// The reason the target was skipped, if applicable. + void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache, TargetSkipReason skipReason = TargetSkipReason.None); void FinalizeProcessing(LoggingContext loggingContext); } diff --git a/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs b/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs index 3c832c5d515..92175ef4a71 100644 --- a/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs +++ b/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs @@ -4,6 +4,7 @@ using System; using Microsoft.Build.BackEnd; using Microsoft.Build.BackEnd.Logging; +using Microsoft.Build.Framework; using Microsoft.Build.Framework.Telemetry; using Microsoft.Build.Shared; @@ -61,10 +62,10 @@ public void AddTask(string name, TimeSpan cumulativeExecutionTime, short executi _workerNodeTelemetryData.AddTask(key, cumulativeExecutionTime, executionsCount, totalMemoryConsumed, taskFactoryName, taskHostRuntime); } - public void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache) + public void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache, TargetSkipReason skipReason = TargetSkipReason.None) { var key = GetKey(name, isCustom, isMetaproj, isFromNugetCache); - _workerNodeTelemetryData.AddTarget(key, wasExecuted); + _workerNodeTelemetryData.AddTarget(key, wasExecuted, skipReason); } private static TaskOrTargetTelemetryKey GetKey(string name, bool isCustom, bool isMetaproj, @@ -85,7 +86,7 @@ public class NullTelemetryForwarder : ITelemetryForwarder public void AddTask(string name, TimeSpan cumulativeExecutionTime, short executionsCount, long totalMemoryConsumed, bool isCustom, bool isFromNugetCache, string? taskFactoryName, string? taskHostRuntime) { } - public void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache) { } + public void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache, TargetSkipReason skipReason = TargetSkipReason.None) { } public void FinalizeProcessing(LoggingContext loggingContext) { } } diff --git a/src/Framework.UnitTests/WorkerNodeTelemetryEventArgs_Tests.cs b/src/Framework.UnitTests/WorkerNodeTelemetryEventArgs_Tests.cs index 651d0446b5d..88ffb519f0c 100644 --- a/src/Framework.UnitTests/WorkerNodeTelemetryEventArgs_Tests.cs +++ b/src/Framework.UnitTests/WorkerNodeTelemetryEventArgs_Tests.cs @@ -22,7 +22,12 @@ public void SerializationDeserializationTest() { (TaskOrTargetTelemetryKey)"task2", new TaskExecutionStats(TimeSpan.Zero, 0, 0, null, null) }, { (TaskOrTargetTelemetryKey)"task3", new TaskExecutionStats(TimeSpan.FromTicks(1234), 12, 987654321, "CodeTaskFactory", "NET") } }, - new Dictionary() { { (TaskOrTargetTelemetryKey)"target1", false }, { (TaskOrTargetTelemetryKey)"target2", true }, }); + new Dictionary() + { + { (TaskOrTargetTelemetryKey)"target1", TargetExecutionStats.Skipped(TargetSkipReason.OutputsUpToDate) }, + { (TaskOrTargetTelemetryKey)"target2", TargetExecutionStats.Executed() }, + { (TaskOrTargetTelemetryKey)"target3", TargetExecutionStats.Skipped(TargetSkipReason.ConditionWasFalse) }, + }); WorkerNodeTelemetryEventArgs args = new WorkerNodeTelemetryEventArgs(td); diff --git a/src/Framework/Telemetry/BuildInsights.cs b/src/Framework/Telemetry/BuildInsights.cs index b21898e74e0..c2effb5ce11 100644 --- a/src/Framework/Telemetry/BuildInsights.cs +++ b/src/Framework/Telemetry/BuildInsights.cs @@ -19,16 +19,23 @@ internal sealed class BuildInsights public TasksSummaryInfo TasksSummary { get; } + /// + /// Information about build incrementality classification. + /// + public BuildIncrementalityInfo? Incrementality { get; } + public BuildInsights( List tasks, List targets, TargetsSummaryInfo targetsSummary, - TasksSummaryInfo tasksSummary) + TasksSummaryInfo tasksSummary, + BuildIncrementalityInfo? incrementality = null) { Tasks = tasks; Targets = targets; TargetsSummary = targetsSummary; TasksSummary = tasksSummary; + Incrementality = incrementality; } internal record TasksSummaryInfo(TaskCategoryStats? Microsoft, TaskCategoryStats? Custom); @@ -39,11 +46,61 @@ internal record TaskStatsInfo(int ExecutionsCount, double TotalMilliseconds, lon internal record ErrorCountsInfo( int? Compiler, - int? MsBuildEngine, + int? MsBuildGeneral, + int? MsBuildEvaluation, + int? MsBuildExecution, + int? MsBuildGraph, int? Task, int? SdkResolvers, int? NetSdk, int? NuGet, int? BuildCheck, + int? NativeToolchain, + int? CodeAnalysis, + int? Razor, + int? Wpf, + int? AspNet, int? Other); + + /// + /// Represents the type of build based on incrementality analysis. + /// + internal enum BuildType + { + /// + /// Build type could not be determined. + /// + Unknown, + + /// + /// Full build where most targets were executed. + /// + Full, + + /// + /// Incremental build where most targets were skipped due to up-to-date checks. + /// + Incremental + } + + /// + /// Information about build incrementality classification. + /// + /// The determined build type (Full, Incremental, or Unknown). + /// Total number of targets in the build. + /// Number of targets that were actually executed. + /// Number of targets that were skipped. + /// Number of targets skipped because outputs were up-to-date. + /// Number of targets skipped due to false conditions. + /// Number of targets skipped because they were previously built. + /// Ratio of skipped targets to total targets (0.0 to 1.0). Higher values indicate more incremental builds. + internal record BuildIncrementalityInfo( + BuildType Classification, + int TotalTargetsCount, + int ExecutedTargetsCount, + int SkippedTargetsCount, + int SkippedDueToUpToDateCount, + int SkippedDueToConditionCount, + int SkippedDueToPreviouslyBuiltCount, + double IncrementalityRatio); } diff --git a/src/Framework/Telemetry/IWorkerNodeTelemetryData.cs b/src/Framework/Telemetry/IWorkerNodeTelemetryData.cs index b4ca028d57d..1aa5dea0025 100644 --- a/src/Framework/Telemetry/IWorkerNodeTelemetryData.cs +++ b/src/Framework/Telemetry/IWorkerNodeTelemetryData.cs @@ -9,5 +9,33 @@ internal interface IWorkerNodeTelemetryData { Dictionary TasksExecutionData { get; } - Dictionary TargetsExecutionData { get; } + Dictionary TargetsExecutionData { get; } +} + +/// +/// Represents the execution statistics of a target. +/// +/// Whether the target was executed (not skipped). +/// The reason the target was skipped, if applicable. +internal readonly struct TargetExecutionStats(bool wasExecuted, TargetSkipReason skipReason = TargetSkipReason.None) +{ + /// + /// Whether the target was executed (not skipped). + /// + public bool WasExecuted { get; } = wasExecuted; + + /// + /// The reason the target was skipped. Only meaningful when is false. + /// + public TargetSkipReason SkipReason { get; } = skipReason; + + /// + /// Creates stats for an executed target. + /// + public static TargetExecutionStats Executed() => new(wasExecuted: true); + + /// + /// Creates stats for a skipped target with the given reason. + /// + public static TargetExecutionStats Skipped(TargetSkipReason reason) => new(wasExecuted: false, skipReason: reason); } diff --git a/src/Framework/Telemetry/TelemetryDataUtils.cs b/src/Framework/Telemetry/TelemetryDataUtils.cs index 66a56c8f497..782561ddc52 100644 --- a/src/Framework/Telemetry/TelemetryDataUtils.cs +++ b/src/Framework/Telemetry/TelemetryDataUtils.cs @@ -14,7 +14,7 @@ internal static class TelemetryDataUtils /// /// Known Microsoft task factory type names that should not be hashed. /// - private static readonly HashSet KnownTaskFactoryNames = new(StringComparer.Ordinal) + private static readonly HashSet KnownTaskFactoryNames = new(StringComparer.OrdinalIgnoreCase) { "AssemblyTaskFactory", "TaskHostFactory", @@ -44,11 +44,14 @@ internal static class TelemetryDataUtils var tasksSummary = new TasksSummaryConverter(); tasksSummary.Process(telemetryData.TasksExecutionData); + var incrementality = ComputeIncrementalityInfo(telemetryData.TargetsExecutionData); + var buildInsights = new BuildInsights( includeTasksDetails ? GetTasksDetails(telemetryData.TasksExecutionData) : [], includeTargetDetails ? GetTargetsDetails(telemetryData.TargetsExecutionData) : [], GetTargetsSummary(targetsSummary), - GetTasksSummary(tasksSummary)); + GetTasksSummary(tasksSummary), + incrementality); return new NodeTelemetry(buildInsights); } @@ -56,20 +59,21 @@ internal static class TelemetryDataUtils /// /// Converts targets details to a list of custom objects for telemetry. /// - private static List GetTargetsDetails(Dictionary targetsDetails) + private static List GetTargetsDetails(Dictionary targetsDetails) { var result = new List(); - foreach (KeyValuePair valuePair in targetsDetails) + foreach (KeyValuePair valuePair in targetsDetails) { string targetName = ShouldHashKey(valuePair.Key) ? GetHashed(valuePair.Key.Name) : valuePair.Key.Name; result.Add(new TargetDetailInfo( targetName, - valuePair.Value, + valuePair.Value.WasExecuted, valuePair.Key.IsCustom, valuePair.Key.IsNuget, - valuePair.Key.IsMetaProj)); + valuePair.Key.IsMetaProj, + valuePair.Value.SkipReason)); } return result; @@ -77,7 +81,7 @@ private static List GetTargetsDetails(Dictionary key.IsCustom || key.IsMetaProj; } - internal record TargetDetailInfo(string Name, bool WasExecuted, bool IsCustom, bool IsNuget, bool IsMetaProj); + internal record TargetDetailInfo(string Name, bool WasExecuted, bool IsCustom, bool IsNuget, bool IsMetaProj, TargetSkipReason SkipReason); /// /// Converts tasks details to a list of custom objects for telemetry. @@ -238,14 +242,14 @@ private class TargetsSummaryConverter /// /// Processes target execution data to compile summary statistics for both built-in and custom targets. /// - public void Process(Dictionary targetsExecutionData) + public void Process(Dictionary targetsExecutionData) { foreach (var kv in targetsExecutionData) { GetTargetInfo(kv.Key, isExecuted: false).Increment(kv.Key); // Update executed targets statistics (only if executed) - if (kv.Value) + if (kv.Value.WasExecuted) { GetTargetInfo(kv.Key, isExecuted: true).Increment(kv.Key); } @@ -316,27 +320,95 @@ internal class TasksInfo } } + /// + /// Threshold ratio above which a build is classified as incremental. + /// A build with more than 70% skipped targets is considered incremental. + /// + private const double IncrementalBuildThreshold = 0.70; + + /// + /// Computes build incrementality information from target execution data. + /// + private static BuildInsights.BuildIncrementalityInfo ComputeIncrementalityInfo( + Dictionary targetsExecutionData) + { + int totalTargets = targetsExecutionData.Count; + int executedTargets = 0; + int skippedTargets = 0; + int skippedDueToUpToDate = 0; + int skippedDueToCondition = 0; + int skippedDueToPreviouslyBuilt = 0; + + foreach (var kv in targetsExecutionData) + { + if (kv.Value.WasExecuted) + { + executedTargets++; + } + else + { + skippedTargets++; + _ = kv.Value.SkipReason switch + { + TargetSkipReason.OutputsUpToDate => skippedDueToUpToDate++, + TargetSkipReason.ConditionWasFalse => skippedDueToCondition++, + TargetSkipReason.PreviouslyBuiltSuccessfully or TargetSkipReason.PreviouslyBuiltUnsuccessfully => skippedDueToPreviouslyBuilt++, + _ => 0 + }; + } + } + + // Calculate incrementality ratio (0.0 = full build, 1.0 = fully incremental) + double incrementalityRatio = totalTargets > 0 ? (double)skippedTargets / totalTargets : 0.0; + + var classification = totalTargets == 0 + ? BuildInsights.BuildType.Unknown + : incrementalityRatio >= IncrementalBuildThreshold + ? BuildInsights.BuildType.Incremental + : BuildInsights.BuildType.Full; + + return new BuildInsights.BuildIncrementalityInfo( + Classification: classification, + TotalTargetsCount: totalTargets, + ExecutedTargetsCount: executedTargets, + SkippedTargetsCount: skippedTargets, + SkippedDueToUpToDateCount: skippedDueToUpToDate, + SkippedDueToConditionCount: skippedDueToCondition, + SkippedDueToPreviouslyBuiltCount: skippedDueToPreviouslyBuilt, + IncrementalityRatio: incrementalityRatio); + } + private sealed class NodeTelemetry(BuildInsights insights) : IActivityTelemetryDataHolder { Dictionary IActivityTelemetryDataHolder.GetActivityProperties() { - Dictionary properties = new() + Dictionary properties = new(5) { [nameof(BuildInsights.TargetsSummary)] = insights.TargetsSummary, [nameof(BuildInsights.TasksSummary)] = insights.TasksSummary, }; - if (insights.Targets.Count > 0) + AddIfNotEmpty(nameof(BuildInsights.Targets), insights.Targets); + AddIfNotEmpty(nameof(BuildInsights.Tasks), insights.Tasks); + AddIfNotNull(nameof(BuildInsights.Incrementality), insights.Incrementality); + + return properties; + + void AddIfNotEmpty(string key, List list) { - properties[nameof(BuildInsights.Targets)] = insights.Targets; + if (list.Count > 0) + { + properties[key] = list; + } } - if (insights.Tasks.Count > 0) + void AddIfNotNull(string key, object? value) { - properties[nameof(BuildInsights.Tasks)] = insights.Tasks; + if (value != null) + { + properties[key] = value; + } } - - return properties; } } } diff --git a/src/Framework/Telemetry/WorkerNodeTelemetryData.cs b/src/Framework/Telemetry/WorkerNodeTelemetryData.cs index ae143f52ed4..d127a280d54 100644 --- a/src/Framework/Telemetry/WorkerNodeTelemetryData.cs +++ b/src/Framework/Telemetry/WorkerNodeTelemetryData.cs @@ -8,7 +8,7 @@ namespace Microsoft.Build.Framework.Telemetry; internal class WorkerNodeTelemetryData : IWorkerNodeTelemetryData { - public WorkerNodeTelemetryData(Dictionary tasksExecutionData, Dictionary targetsExecutionData) + public WorkerNodeTelemetryData(Dictionary tasksExecutionData, Dictionary targetsExecutionData) { TasksExecutionData = tasksExecutionData; TargetsExecutionData = targetsExecutionData; @@ -23,7 +23,7 @@ public void Add(IWorkerNodeTelemetryData other) foreach (var target in other.TargetsExecutionData) { - AddTarget(target.Key, target.Value); + AddTarget(target.Key, target.Value.WasExecuted, target.Value.SkipReason); } } @@ -45,16 +45,33 @@ public void AddTask(TaskOrTargetTelemetryKey task, TimeSpan cumulativeExecutionT } } - public void AddTarget(TaskOrTargetTelemetryKey target, bool wasExecuted) + public void AddTarget(TaskOrTargetTelemetryKey target, bool wasExecuted, TargetSkipReason skipReason = TargetSkipReason.None) { - TargetsExecutionData[target] = - // we just need to store if it was ever executed - wasExecuted || (TargetsExecutionData.TryGetValue(target, out bool wasAlreadyExecuted) && wasAlreadyExecuted); + if (TargetsExecutionData.TryGetValue(target, out var existingStats)) + { + // If the target was ever executed, mark it as executed + // Otherwise, keep the most informative skip reason (non-None preferred) + if (wasExecuted || existingStats.WasExecuted) + { + TargetsExecutionData[target] = TargetExecutionStats.Executed(); + } + else if (skipReason != TargetSkipReason.None) + { + TargetsExecutionData[target] = TargetExecutionStats.Skipped(skipReason); + } + // else keep existing stats + } + else + { + TargetsExecutionData[target] = wasExecuted + ? TargetExecutionStats.Executed() + : TargetExecutionStats.Skipped(skipReason); + } } public WorkerNodeTelemetryData() : this([], []) { } public Dictionary TasksExecutionData { get; } - public Dictionary TargetsExecutionData { get; } + public Dictionary TargetsExecutionData { get; } } diff --git a/src/Framework/Telemetry/WorkerNodeTelemetryEventArgs.cs b/src/Framework/Telemetry/WorkerNodeTelemetryEventArgs.cs index d2f35e5eb4f..38c2bb43910 100644 --- a/src/Framework/Telemetry/WorkerNodeTelemetryEventArgs.cs +++ b/src/Framework/Telemetry/WorkerNodeTelemetryEventArgs.cs @@ -32,10 +32,11 @@ internal override void WriteToStream(BinaryWriter writer) } writer.Write7BitEncodedInt(WorkerNodeTelemetryData.TargetsExecutionData.Count); - foreach (KeyValuePair entry in WorkerNodeTelemetryData.TargetsExecutionData) + foreach (KeyValuePair entry in WorkerNodeTelemetryData.TargetsExecutionData) { WriteToStream(writer, entry.Key); - writer.Write(entry.Value); + writer.Write(entry.Value.WasExecuted); + writer.Write((int)entry.Value.SkipReason); } } @@ -63,10 +64,13 @@ internal override void CreateFromStream(BinaryReader reader, int version) } count = reader.Read7BitEncodedInt(); - Dictionary targetsExecutionData = new(); + Dictionary targetsExecutionData = new(); for (int i = 0; i < count; i++) { - targetsExecutionData.Add(ReadFromStream(reader), reader.ReadBoolean()); + var key = ReadFromStream(reader); + var wasExecuted = reader.ReadBoolean(); + var skipReason = (TargetSkipReason)reader.ReadInt32(); + targetsExecutionData.Add(key, new TargetExecutionStats(wasExecuted, skipReason)); } WorkerNodeTelemetryData = new WorkerNodeTelemetryData(tasksExecutionData, targetsExecutionData);