diff --git a/src/Build.UnitTests/BackEnd/NodePackets_Tests.cs b/src/Build.UnitTests/BackEnd/NodePackets_Tests.cs index c63606144c..f609a693cb 100644 --- a/src/Build.UnitTests/BackEnd/NodePackets_Tests.cs +++ b/src/Build.UnitTests/BackEnd/NodePackets_Tests.cs @@ -8,6 +8,7 @@ using System.Linq; using FluentAssertions; using Microsoft.Build.BackEnd; using Microsoft.Build.Execution; +using Microsoft.Build.Experimental.BuildCheck; using Microsoft.Build.Framework; using Microsoft.Build.Shared; using Xunit; @@ -77,6 +78,7 @@ namespace Microsoft.Build.UnitTests.BackEnd EnvironmentVariableReadEventArgs environmentVariableRead = new("env", "message", "file", 0, 0); GeneratedFileUsedEventArgs generatedFileUsed = new GeneratedFileUsedEventArgs("path", "some content"); BuildSubmissionStartedEventArgs buildSubmissionStarted = new(new Dictionary { { "Value1", "Value2" } }, ["Path1"], ["TargetName"], BuildRequestDataFlags.ReplaceExistingProjectInstance, 123); + BuildCheckTracingEventArgs buildCheckTracing = new(); VerifyLoggingPacket(buildFinished, LoggingEventType.BuildFinishedEvent); VerifyLoggingPacket(buildStarted, LoggingEventType.BuildStartedEvent); @@ -111,6 +113,7 @@ namespace Microsoft.Build.UnitTests.BackEnd VerifyLoggingPacket(environmentVariableRead, LoggingEventType.EnvironmentVariableReadEvent); VerifyLoggingPacket(generatedFileUsed, LoggingEventType.GeneratedFileUsedEvent); VerifyLoggingPacket(buildSubmissionStarted, LoggingEventType.BuildSubmissionStartedEvent); + VerifyLoggingPacket(buildCheckTracing, LoggingEventType.BuildCheckTracingEvent); } private static BuildEventContext CreateBuildEventContext() diff --git a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs index 9df91a2a93..bf2af7ca1f 100644 --- a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs +++ b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs @@ -4,11 +4,13 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Globalization; using System.IO; using System.Linq; using System.Text; using FluentAssertions; using Microsoft.Build.BackEnd; +using Microsoft.Build.Experimental.BuildCheck; using Microsoft.Build.Framework; using Microsoft.Build.Framework.Profiler; using Microsoft.Build.Logging; @@ -530,6 +532,27 @@ namespace Microsoft.Build.UnitTests e => string.Join(", ", e.RawArguments ?? Array.Empty())); } + [Fact] + public void RoundtripBuildCheckTracingEventArgs() + { + string key1 = "AA"; + TimeSpan span1 = TimeSpan.FromSeconds(5); + string key2 = "b"; + TimeSpan span2 = TimeSpan.FromSeconds(15); + string key3 = "cCc"; + TimeSpan span3 = TimeSpan.FromSeconds(50); + + Dictionary stats = new() { { key1, span1 }, { key2, span2 }, { key3, span3 } }; + + BuildCheckTracingEventArgs args = new BuildCheckTracingEventArgs(stats); + + Roundtrip(args, + e => e.TracingData.InfrastructureTracingData.Keys.Count.ToString(), + e => e.TracingData.InfrastructureTracingData.Keys.ToCsvString(false), + e => e.TracingData.InfrastructureTracingData.Values + .Select(v => v.TotalSeconds.ToString(CultureInfo.InvariantCulture)).ToCsvString(false)); + } + [Theory] [InlineData(true)] [InlineData(false)] diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index ffd2b33a2e..96cada2d55 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -1070,6 +1070,11 @@ namespace Microsoft.Build.Execution } _buildTelemetry.Host = host; + _buildTelemetry.BuildCheckEnabled = _buildParameters!.IsBuildCheckEnabled; + var sacState = NativeMethodsShared.GetSACState(); + // The Enforcement would lead to build crash - but let's have the check for completeness sake. + _buildTelemetry.SACEnabled = sacState == NativeMethodsShared.SAC_State.Evaluation || sacState == NativeMethodsShared.SAC_State.Enforcement; + loggingService.LogTelemetry(buildEventContext: null, _buildTelemetry.EventName, _buildTelemetry.GetProperties()); // Clean telemetry to make it ready for next build submission. _buildTelemetry = null; diff --git a/src/Build/BuildCheck/API/Check.cs b/src/Build/BuildCheck/API/Check.cs index 4eeeb8599f..55c1d87a95 100644 --- a/src/Build/BuildCheck/API/Check.cs +++ b/src/Build/BuildCheck/API/Check.cs @@ -43,6 +43,8 @@ public abstract class Check : IDisposable /// public abstract void RegisterActions(IBuildCheckRegistrationContext registrationContext); + internal virtual bool IsBuiltIn => false; + public virtual void Dispose() { } } diff --git a/src/Build/BuildCheck/API/InternalCheck.cs b/src/Build/BuildCheck/API/InternalCheck.cs index 728a01c053..242c513e65 100644 --- a/src/Build/BuildCheck/API/InternalCheck.cs +++ b/src/Build/BuildCheck/API/InternalCheck.cs @@ -28,4 +28,6 @@ internal abstract class InternalCheck : Check this.RegisterInternalActions(internalRegistrationContext); } + + internal override bool IsBuiltIn => true; } diff --git a/src/Build/BuildCheck/Acquisition/BuildCheckAcquisitionModule.cs b/src/Build/BuildCheck/Acquisition/BuildCheckAcquisitionModule.cs index b0899f2213..7083d50aa8 100644 --- a/src/Build/BuildCheck/Acquisition/BuildCheckAcquisitionModule.cs +++ b/src/Build/BuildCheck/Acquisition/BuildCheckAcquisitionModule.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Reflection; using Microsoft.Build.Experimental.BuildCheck.Infrastructure; using Microsoft.Build.Framework; +using Microsoft.Build.Framework.Telemetry; using Microsoft.Build.Shared; namespace Microsoft.Build.Experimental.BuildCheck.Acquisition; @@ -53,21 +54,24 @@ internal class BuildCheckAcquisitionModule : IBuildCheckAcquisitionModule .ForEach(t => checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckBaseTypeNotAssignable", t.Name, t.Assembly)); } } - catch (ReflectionTypeLoadException ex) + catch (ReflectionTypeLoadException ex) when (ex.LoaderExceptions.Length != 0) { - if (ex.LoaderExceptions.Length != 0) + foreach (Exception? unrolledEx in ex.LoaderExceptions.Where(e => e != null).Prepend(ex)) { - foreach (Exception? loaderException in ex.LoaderExceptions) - { - checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckFailedRuleLoading", loaderException?.Message); - } + ReportLoadingError(unrolledEx!); } } catch (Exception ex) { - checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckFailedRuleLoading", ex?.Message); + ReportLoadingError(ex); } return checksFactories; + + void ReportLoadingError(Exception ex) + { + checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckFailedRuleLoading", ex.Message); + checkContext.DispatchFailedAcquisitionTelemetry(System.IO.Path.GetFileName(checkAcquisitionData.AssemblyPath), ex); + } } } diff --git a/src/Build/BuildCheck/Checks/DoubleWritesCheck.cs b/src/Build/BuildCheck/Checks/DoubleWritesCheck.cs index 83f1296113..25d99b2bb9 100644 --- a/src/Build/BuildCheck/Checks/DoubleWritesCheck.cs +++ b/src/Build/BuildCheck/Checks/DoubleWritesCheck.cs @@ -42,6 +42,8 @@ internal sealed class DoubleWritesCheck : Check registrationContext.RegisterTaskInvocationAction(TaskInvocationAction); } + internal override bool IsBuiltIn => true; + /// /// Contains the first project file + task that wrote the given file during the build. /// @@ -126,5 +128,5 @@ internal sealed class DoubleWritesCheck : Check _filesWritten.Add(fileBeingWritten, (context.Data.ProjectFilePath, context.Data.TaskName)); } } - } + } } diff --git a/src/Build/BuildCheck/Checks/NoEnvironmentVariablePropertyCheck.cs b/src/Build/BuildCheck/Checks/NoEnvironmentVariablePropertyCheck.cs index 86d5b167a1..30049c3f7c 100644 --- a/src/Build/BuildCheck/Checks/NoEnvironmentVariablePropertyCheck.cs +++ b/src/Build/BuildCheck/Checks/NoEnvironmentVariablePropertyCheck.cs @@ -49,6 +49,8 @@ internal sealed class NoEnvironmentVariablePropertyCheck : Check public override void RegisterActions(IBuildCheckRegistrationContext registrationContext) => registrationContext.RegisterEnvironmentVariableReadAction(ProcessEnvironmentVariableReadAction); + internal override bool IsBuiltIn => true; + private void ProcessEnvironmentVariableReadAction(BuildCheckDataContext context) { EnvironmentVariableIdentityKey identityKey = new(context.Data.EnvironmentVariableName, context.Data.EnvironmentVariableLocation); diff --git a/src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs b/src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs index 42a1ca50d0..e2e01caf5e 100644 --- a/src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs +++ b/src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs @@ -118,6 +118,8 @@ internal class PropertiesUsageCheck : InternalCheck } } + internal override bool IsBuiltIn => true; + private Dictionary _writenProperties = new(MSBuildNameIgnoreCaseComparer.Default); private HashSet _readProperties = new(MSBuildNameIgnoreCaseComparer.Default); // For the 'Property Initialized after used' check - we are interested in cases where: diff --git a/src/Build/BuildCheck/Checks/SharedOutputPathCheck.cs b/src/Build/BuildCheck/Checks/SharedOutputPathCheck.cs index 260d76b7eb..e01f62ab78 100644 --- a/src/Build/BuildCheck/Checks/SharedOutputPathCheck.cs +++ b/src/Build/BuildCheck/Checks/SharedOutputPathCheck.cs @@ -35,6 +35,8 @@ internal sealed class SharedOutputPathCheck : Check registrationContext.RegisterEvaluatedPropertiesAction(EvaluatedPropertiesAction); } + internal override bool IsBuiltIn => true; + private readonly Dictionary _projectsPerOutputPath = new(StringComparer.CurrentCultureIgnoreCase); private readonly HashSet _projects = new(StringComparer.CurrentCultureIgnoreCase); diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs index 419ca2c9f2..84cdd25ad6 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs @@ -107,7 +107,7 @@ internal class BuildCheckBuildEventHandler { if (!eventArgs.IsAggregatedGlobalReport) { - _stats.Merge(eventArgs.TracingData, (span1, span2) => span1 + span2); + _tracingData.MergeIn(eventArgs.TracingData); } } @@ -138,36 +138,25 @@ internal class BuildCheckBuildEventHandler private bool IsMetaProjFile(string? projectFile) => projectFile?.EndsWith(".metaproj", StringComparison.OrdinalIgnoreCase) == true; - private readonly Dictionary _stats = new Dictionary(); + private readonly BuildCheckTracingData _tracingData = new BuildCheckTracingData(); private void HandleBuildFinishedEvent(BuildFinishedEventArgs eventArgs) { _buildCheckManager.ProcessBuildFinished(_checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!)); - _stats.Merge(_buildCheckManager.CreateCheckTracingStats(), (span1, span2) => span1 + span2); + _tracingData.MergeIn(_buildCheckManager.CreateCheckTracingStats()); LogCheckStats(_checkContextFactory.CreateCheckContext(GetBuildEventContext(eventArgs))); } private void LogCheckStats(ICheckContext checkContext) { - Dictionary infraStats = new Dictionary(); - Dictionary checkStats = new Dictionary(); + Dictionary infraStats = _tracingData.InfrastructureTracingData; + // Stats are per rule, while runtime is per check - and check can have multiple rules. + // In case of multi-rule check, the runtime stats are duplicated for each rule. + Dictionary checkStats = _tracingData.ExtractCheckStats(); - foreach (var stat in _stats) - { - if (stat.Key.StartsWith(BuildCheckConstants.infraStatPrefix)) - { - string newKey = stat.Key.Substring(BuildCheckConstants.infraStatPrefix.Length); - infraStats[newKey] = stat.Value; - } - else - { - checkStats[stat.Key] = stat.Value; - } - } - - BuildCheckTracingEventArgs statEvent = new BuildCheckTracingEventArgs(_stats, true) + BuildCheckTracingEventArgs statEvent = new BuildCheckTracingEventArgs(_tracingData, true) { BuildEventContext = checkContext.BuildEventContext }; checkContext.DispatchBuildEvent(statEvent); @@ -177,6 +166,7 @@ internal class BuildCheckBuildEventHandler checkContext.DispatchAsCommentFromText(MessageImportance.Low, infraData); string checkData = BuildCsvString("Checks run times", checkStats); checkContext.DispatchAsCommentFromText(MessageImportance.Low, checkData); + checkContext.DispatchTelemetry(_tracingData); } private string BuildCsvString(string title, Dictionary rowData) diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs index 959f39754d..7185030e52 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs @@ -229,10 +229,10 @@ internal sealed class BuildCheckManagerProvider : IBuildCheckManagerProvider CheckConfigurationEffective[] configurations; if (checkFactoryContext.MaterializedCheck == null) { - CheckConfiguration[] userConfigs = + CheckConfiguration[] userEditorConfigs = _configurationProvider.GetUserConfigurations(projectFullPath, checkFactoryContext.RuleIds); - if (userConfigs.All(c => !(c.IsEnabled ?? checkFactoryContext.IsEnabledByDefault))) + if (userEditorConfigs.All(c => !(c.IsEnabled ?? checkFactoryContext.IsEnabledByDefault))) { // the check was not yet instantiated nor mounted - so nothing to do here now. return; @@ -242,7 +242,7 @@ internal sealed class BuildCheckManagerProvider : IBuildCheckManagerProvider _configurationProvider.GetCustomConfigurations(projectFullPath, checkFactoryContext.RuleIds); Check uninitializedCheck = checkFactoryContext.Factory(); - configurations = _configurationProvider.GetMergedConfigurations(userConfigs, uninitializedCheck); + configurations = _configurationProvider.GetMergedConfigurations(userEditorConfigs, uninitializedCheck); ConfigurationContext configurationContext = ConfigurationContext.FromDataEnumeration(customConfigData, configurations); @@ -271,7 +271,7 @@ internal sealed class BuildCheckManagerProvider : IBuildCheckManagerProvider // price to be paid in that case is slight performance cost. // Create the wrapper and register to central context - wrapper.StartNewProject(projectFullPath, configurations); + wrapper.StartNewProject(projectFullPath, configurations, userEditorConfigs); var wrappedContext = new CheckRegistrationContext(wrapper, _buildCheckCentralContext); check.RegisterActions(wrappedContext); } @@ -279,13 +279,15 @@ internal sealed class BuildCheckManagerProvider : IBuildCheckManagerProvider { wrapper = checkFactoryContext.MaterializedCheck; - configurations = _configurationProvider.GetMergedConfigurations(projectFullPath, wrapper.Check); + CheckConfiguration[] userEditorConfigs = + _configurationProvider.GetUserConfigurations(projectFullPath, checkFactoryContext.RuleIds); + configurations = _configurationProvider.GetMergedConfigurations(userEditorConfigs, wrapper.Check); _configurationProvider.CheckCustomConfigurationDataValidity(projectFullPath, checkFactoryContext.RuleIds[0]); // Update the wrapper - wrapper.StartNewProject(projectFullPath, configurations); + wrapper.StartNewProject(projectFullPath, configurations, userEditorConfigs); } } @@ -346,7 +348,7 @@ internal sealed class BuildCheckManagerProvider : IBuildCheckManagerProvider if (checkToRemove.MaterializedCheck is not null) { _buildCheckCentralContext.DeregisterCheck(checkToRemove.MaterializedCheck); - _tracingReporter.AddCheckStats(checkToRemove.MaterializedCheck.Check.FriendlyName, checkToRemove.MaterializedCheck.Elapsed); + _ruleTelemetryData.AddRange(checkToRemove.MaterializedCheck.GetRuleTelemetryData()); checkToRemove.MaterializedCheck.Check.Dispose(); } } @@ -411,19 +413,18 @@ internal sealed class BuildCheckManagerProvider : IBuildCheckManagerProvider => _buildEventsProcessor .ProcessTaskParameterEventArgs(checkContext, taskParameterEventArgs); - public Dictionary CreateCheckTracingStats() + private readonly List _ruleTelemetryData = []; + public BuildCheckTracingData CreateCheckTracingStats() { foreach (CheckFactoryContext checkFactoryContext in _checkRegistry) { if (checkFactoryContext.MaterializedCheck != null) { - _tracingReporter.AddCheckStats(checkFactoryContext.FriendlyName, checkFactoryContext.MaterializedCheck.Elapsed); - checkFactoryContext.MaterializedCheck.ClearStats(); + _ruleTelemetryData.AddRange(checkFactoryContext.MaterializedCheck.GetRuleTelemetryData()); } } - _tracingReporter.AddCheckInfraStats(); - return _tracingReporter.TracingStats; + return new BuildCheckTracingData(_ruleTelemetryData, _tracingReporter.GetInfrastructureTracingStats()); } public void FinalizeProcessing(LoggingContext loggingContext) diff --git a/src/Build/BuildCheck/Infrastructure/CheckContext/CheckDispatchingContext.cs b/src/Build/BuildCheck/Infrastructure/CheckContext/CheckDispatchingContext.cs index 584bedca51..06b7d5ccec 100644 --- a/src/Build/BuildCheck/Infrastructure/CheckContext/CheckDispatchingContext.cs +++ b/src/Build/BuildCheck/Infrastructure/CheckContext/CheckDispatchingContext.cs @@ -69,4 +69,12 @@ internal class CheckDispatchingContext : ICheckContext _eventDispatcher.Dispatch(buildEvent); } + + public void DispatchFailedAcquisitionTelemetry(string assemblyName, Exception exception) + // This is it - no action for replay mode. + { } + + public void DispatchTelemetry(BuildCheckTracingData data) + // This is it - no action for replay mode. + { } } diff --git a/src/Build/BuildCheck/Infrastructure/CheckContext/CheckLoggingContext.cs b/src/Build/BuildCheck/Infrastructure/CheckContext/CheckLoggingContext.cs index 8c15478a70..13f308d228 100644 --- a/src/Build/BuildCheck/Infrastructure/CheckContext/CheckLoggingContext.cs +++ b/src/Build/BuildCheck/Infrastructure/CheckContext/CheckLoggingContext.cs @@ -8,6 +8,7 @@ using System.Text; using System.Threading.Tasks; using Microsoft.Build.BackEnd.Logging; using Microsoft.Build.Framework; +using Microsoft.Build.Framework.Telemetry; using Microsoft.Build.Shared; namespace Microsoft.Build.Experimental.BuildCheck; @@ -43,4 +44,18 @@ internal readonly struct CheckLoggingContext(ILoggingService loggingService, Bui public void DispatchAsWarningFromText(string? subcategoryResourceName, string? errorCode, string? helpKeyword, BuildEventFileInfo file, string message) => loggingService .LogWarningFromText(eventContext, subcategoryResourceName, errorCode, helpKeyword, file, message); + + public void DispatchFailedAcquisitionTelemetry(string assemblyName, Exception exception) + { + var telemetryTransportData = KnownTelemetry.BuildCheckTelemetry.ProcessCustomCheckLoadingFailure(assemblyName, exception); + loggingService.LogTelemetry(eventContext, telemetryTransportData.Item1, telemetryTransportData.Item2); + } + + public void DispatchTelemetry(BuildCheckTracingData data) + { + foreach ((string, IDictionary) telemetryTransportData in KnownTelemetry.BuildCheckTelemetry.ProcessBuildCheckTracingData(data)) + { + loggingService.LogTelemetry(eventContext, telemetryTransportData.Item1, telemetryTransportData.Item2); + } + } } diff --git a/src/Build/BuildCheck/Infrastructure/CheckContext/ICheckContext.cs b/src/Build/BuildCheck/Infrastructure/CheckContext/ICheckContext.cs index 0f592d129d..8cfa8b5b82 100644 --- a/src/Build/BuildCheck/Infrastructure/CheckContext/ICheckContext.cs +++ b/src/Build/BuildCheck/Infrastructure/CheckContext/ICheckContext.cs @@ -45,4 +45,14 @@ internal interface ICheckContext /// Dispatch the instance of as a warning message. /// void DispatchAsWarningFromText(string? subcategoryResourceName, string? errorCode, string? helpKeyword, BuildEventFileInfo file, string message); + + /// + /// Dispatch the telemetry data for a failed acquisition. + /// + void DispatchFailedAcquisitionTelemetry(string assemblyName, Exception exception); + + /// + /// If supported - dispatches the telemetry data. + /// + void DispatchTelemetry(BuildCheckTracingData data); } diff --git a/src/Build/BuildCheck/Infrastructure/CheckWrapper.cs b/src/Build/BuildCheck/Infrastructure/CheckWrapper.cs index fb736e2d13..9568709534 100644 --- a/src/Build/BuildCheck/Infrastructure/CheckWrapper.cs +++ b/src/Build/BuildCheck/Infrastructure/CheckWrapper.cs @@ -17,6 +17,7 @@ namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure; internal sealed class CheckWrapper { private readonly Stopwatch _stopwatch = new Stopwatch(); + private readonly BuildCheckRuleTelemetryData[] _ruleTelemetryData; /// /// Maximum amount of messages that could be sent per check rule. @@ -41,6 +42,23 @@ internal sealed class CheckWrapper public CheckWrapper(Check check) { Check = check; + _ruleTelemetryData = new BuildCheckRuleTelemetryData[check.SupportedRules.Count]; + + InitializeTelemetryData(_ruleTelemetryData, check); + } + + private static void InitializeTelemetryData(BuildCheckRuleTelemetryData[] ruleTelemetryData, Check check) + { + int idx = 0; + foreach (CheckRule checkRule in check.SupportedRules) + { + ruleTelemetryData[idx++] = new BuildCheckRuleTelemetryData( + ruleId: checkRule.Id, + checkFriendlyName: check.FriendlyName, + isBuiltIn: check.IsBuiltIn, + defaultSeverity: (checkRule.DefaultConfiguration.Severity ?? + CheckConfigurationEffective.Default.Severity).ToDiagnosticSeverity()); + } } internal Check Check { get; } @@ -51,29 +69,87 @@ internal sealed class CheckWrapper // In such case - configuration will be same for all projects. So we do not need to store it per project in a collection. internal CheckConfigurationEffective? CommonConfig { get; private set; } - // start new project + /// + /// Ensures the check being configured for a new project (as each project can have different settings) + /// + /// + /// Resulting merged configurations per rule (merged from check default and explicit user editorconfig). + /// Configurations from editorconfig per rule. internal void StartNewProject( string fullProjectPath, - IReadOnlyList userConfigs) + IReadOnlyList effectiveConfigs, + IReadOnlyList editorConfigs) { + // Let's first update the telemetry data for the rules. + int idx = 0; + foreach (BuildCheckRuleTelemetryData ruleTelemetryData in _ruleTelemetryData) + { + CheckConfigurationEffective effectiveConfig = effectiveConfigs[Math.Max(idx, effectiveConfigs.Count - 1)]; + if (editorConfigs[idx].Severity != null) + { + ruleTelemetryData.ExplicitSeverities.Add(editorConfigs[idx].Severity!.Value.ToDiagnosticSeverity()); + } + + if (effectiveConfig.IsEnabled) + { + ruleTelemetryData.ProjectNamesWhereEnabled.Add(fullProjectPath); + } + + idx++; + } + if (!_areStatsInitialized) { _areStatsInitialized = true; - CommonConfig = userConfigs[0]; + CommonConfig = effectiveConfigs[0]; - if (userConfigs.Count == 1) + if (effectiveConfigs.Count == 1) { return; } } // The Common configuration is not common anymore - let's nullify it and we will need to fetch configuration per project. - if (CommonConfig == null || !userConfigs.All(t => t.IsSameConfigurationAs(CommonConfig))) + if (CommonConfig == null || !effectiveConfigs.All(t => t.IsSameConfigurationAs(CommonConfig))) { CommonConfig = null; } } + private void AddDiagnostic(CheckConfigurationEffective configurationEffective) + { + BuildCheckRuleTelemetryData? telemetryData = + _ruleTelemetryData.FirstOrDefault(td => td.RuleId.Equals(configurationEffective.RuleId)); + + if (telemetryData == null) + { + return; + } + + switch (configurationEffective.Severity) + { + + case CheckResultSeverity.Suggestion: + telemetryData.IncrementMessagesCount(); + break; + case CheckResultSeverity.Warning: + telemetryData.IncrementWarningsCount(); + break; + case CheckResultSeverity.Error: + telemetryData.IncrementErrorsCount(); + break; + case CheckResultSeverity.Default: + case CheckResultSeverity.None: + default: + break; + } + + if (IsThrottled) + { + telemetryData.SetThrottled(); + } + } + internal void ReportResult(BuildCheckResult result, ICheckContext checkContext, CheckConfigurationEffective config) { if (!IsThrottled) @@ -93,6 +169,9 @@ internal sealed class CheckWrapper IsThrottled = true; } } + + // Add the diagnostic to the check wrapper for telemetry purposes. + AddDiagnostic(config); } } @@ -102,9 +181,15 @@ internal sealed class CheckWrapper _areStatsInitialized = false; } - internal TimeSpan Elapsed => _stopwatch.Elapsed; + internal IReadOnlyList GetRuleTelemetryData() + { + foreach (BuildCheckRuleTelemetryData ruleTelemetryData in _ruleTelemetryData) + { + ruleTelemetryData.TotalRuntime = _stopwatch.Elapsed; + } - internal void ClearStats() => _stopwatch.Reset(); + return _ruleTelemetryData; + } internal CleanupScope StartSpan() { diff --git a/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs b/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs index 88c644954e..8f125db455 100644 --- a/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs +++ b/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs @@ -61,7 +61,7 @@ internal interface IBuildCheckManager void ProcessCheckAcquisition(CheckAcquisitionData acquisitionData, ICheckContext checksContext); - Dictionary CreateCheckTracingStats(); + BuildCheckTracingData CreateCheckTracingStats(); void FinalizeProcessing(LoggingContext loggingContext); diff --git a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs index a5bf0b968a..5f6e5c19b6 100644 --- a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs +++ b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs @@ -79,7 +79,7 @@ internal class NullBuildCheckManager : IBuildCheckManager, IBuildEngineDataRoute { } - public Dictionary CreateCheckTracingStats() => new Dictionary(); + public BuildCheckTracingData CreateCheckTracingStats() => new BuildCheckTracingData(); public void ProcessPropertyRead(PropertyReadInfo propertyReadInfo, CheckLoggingContext buildEventContext) { } diff --git a/src/Build/BuildCheck/Infrastructure/TracingReporter.cs b/src/Build/BuildCheck/Infrastructure/TracingReporter.cs index d7f7592296..05bb2f33cd 100644 --- a/src/Build/BuildCheck/Infrastructure/TracingReporter.cs +++ b/src/Build/BuildCheck/Infrastructure/TracingReporter.cs @@ -13,26 +13,10 @@ namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure; internal class TracingReporter { - internal Dictionary TracingStats { get; } = new(); - - // Infrastructure time keepers - // TODO: add more timers throughout BuildCheck run private TimeSpan checkAcquisitionTime; private TimeSpan checkSetDataSourceTime; private TimeSpan newProjectChecksTime; - public void AddCheckStats(string name, TimeSpan subtotal) - { - if (TracingStats.TryGetValue(name, out TimeSpan existing)) - { - TracingStats[name] = existing + subtotal; - } - else - { - TracingStats[name] = subtotal; - } - } - public void AddAcquisitionStats(TimeSpan subtotal) { checkAcquisitionTime += subtotal; @@ -48,14 +32,11 @@ internal class TracingReporter newProjectChecksTime += subtotal; } - public void AddCheckInfraStats() - { - var infraStats = new Dictionary() { - { $"{BuildCheckConstants.infraStatPrefix}checkAcquisitionTime", checkAcquisitionTime }, - { $"{BuildCheckConstants.infraStatPrefix}checkSetDataSourceTime", checkSetDataSourceTime }, - { $"{BuildCheckConstants.infraStatPrefix}newProjectChecksTime", newProjectChecksTime } - }; - - TracingStats.Merge(infraStats, (span1, span2) => span1 + span2); - } + public Dictionary GetInfrastructureTracingStats() + => new Dictionary() + { + { $"{BuildCheckConstants.infraStatPrefix}checkAcquisitionTime", checkAcquisitionTime }, + { $"{BuildCheckConstants.infraStatPrefix}checkSetDataSourceTime", checkSetDataSourceTime }, + { $"{BuildCheckConstants.infraStatPrefix}newProjectChecksTime", newProjectChecksTime } + }; } diff --git a/src/Build/BuildCheck/Utilities/CheckResultSeverityExtensions.cs b/src/Build/BuildCheck/Utilities/CheckResultSeverityExtensions.cs new file mode 100644 index 0000000000..04d6077fc2 --- /dev/null +++ b/src/Build/BuildCheck/Utilities/CheckResultSeverityExtensions.cs @@ -0,0 +1,31 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.Build.Experimental.BuildCheck; +internal static class CheckResultSeverityExtensions +{ + public static DiagnosticSeverity? ToDiagnosticSeverity(this CheckResultSeverity? severity) + { + if (severity == null) + { + return null; + } + + return ToDiagnosticSeverity(severity.Value); + } + + public static DiagnosticSeverity ToDiagnosticSeverity(this CheckResultSeverity severity) + { + return severity switch + { + CheckResultSeverity.Default => DiagnosticSeverity.Default, + CheckResultSeverity.None => DiagnosticSeverity.None, + CheckResultSeverity.Suggestion => DiagnosticSeverity.Suggestion, + CheckResultSeverity.Warning => DiagnosticSeverity.Warning, + CheckResultSeverity.Error => DiagnosticSeverity.Error, + _ => throw new ArgumentOutOfRangeException(nameof(severity), severity, null) + }; + } +} diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs index 294d96bae2..f140698b70 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs @@ -338,7 +338,11 @@ namespace Microsoft.Build.Logging private BinaryLogRecordKind Write(BuildCheckTracingEventArgs e) { WriteBuildEventArgsFields(e, writeMessage: false); - WriteProperties(e.TracingData); + + Dictionary stats = e.TracingData.ExtractCheckStats(); + stats.Merge(e.TracingData.InfrastructureTracingData, (span1, span2) => span1 + span2); + + WriteProperties(stats); return BinaryLogRecordKind.BuildCheckTracing; } diff --git a/src/Framework.UnitTests/BuildCheckTracingEventArgs_Tests.cs b/src/Framework.UnitTests/BuildCheckTracingEventArgs_Tests.cs new file mode 100644 index 0000000000..c96fa7b311 --- /dev/null +++ b/src/Framework.UnitTests/BuildCheckTracingEventArgs_Tests.cs @@ -0,0 +1,58 @@ +// 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 System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.Build.Experimental.BuildCheck; +using Shouldly; +using Xunit; + +namespace Microsoft.Build.Framework.UnitTests +{ + public class BuildCheckTracingEventArgs_Tests + { + [Fact] + public void SerializationDeserializationTest() + { + string key1 = "AA"; + TimeSpan span1 = TimeSpan.FromSeconds(5); + string key2 = "b"; + TimeSpan span2 = TimeSpan.FromSeconds(15); + string key3 = "cCc"; + TimeSpan span3 = TimeSpan.FromSeconds(50); + + Dictionary stats = new() { { key1, span1 }, { key2, span2 }, { key3, span3 } }; + + BuildCheckRuleTelemetryData ruleData1 = new("id1", "name1", true, DiagnosticSeverity.Suggestion, + new HashSet() { DiagnosticSeverity.Default, DiagnosticSeverity.Suggestion }, + new HashSet() { "aa", "b" }, 5, 2, 8, true, TimeSpan.FromSeconds(123)); + + BuildCheckRuleTelemetryData ruleData2 = new("id2", "name2", false, DiagnosticSeverity.Error, + new HashSet(), + new HashSet(), 0, 0, 500, false, TimeSpan.FromSeconds(1234)); + + BuildCheckTracingData data = new(new [] {ruleData1, ruleData2}, stats); + BuildCheckTracingEventArgs arg = new(data); + + using MemoryStream stream = new MemoryStream(); + using BinaryWriter bw = new BinaryWriter(stream); + arg.WriteToStream(bw); + + stream.Position = 0; + using BinaryReader br = new BinaryReader(stream); + BuildCheckTracingEventArgs argDeserialized = new(); + int packetVersion = (Environment.Version.Major * 10) + Environment.Version.Minor; + argDeserialized.CreateFromStream(br, packetVersion); + + argDeserialized.TracingData.InfrastructureTracingData.ShouldBeEquivalentTo(arg.TracingData.InfrastructureTracingData); + argDeserialized.TracingData.TelemetryData.Keys.ShouldBeEquivalentTo(arg.TracingData.TelemetryData.Keys); + + argDeserialized.TracingData.TelemetryData["id1"].ShouldBeEquivalentTo(arg.TracingData.TelemetryData["id1"]); + argDeserialized.TracingData.TelemetryData["id2"].ShouldBeEquivalentTo(arg.TracingData.TelemetryData["id2"]); + } + } +} diff --git a/src/Framework/BuildCheck/BuildCheckEventArgs.cs b/src/Framework/BuildCheck/BuildCheckEventArgs.cs index b490195a03..b017a05d3f 100644 --- a/src/Framework/BuildCheck/BuildCheckEventArgs.cs +++ b/src/Framework/BuildCheck/BuildCheckEventArgs.cs @@ -19,15 +19,20 @@ internal abstract class BuildCheckEventArgs : BuildEventArgs /// /// Transport mean for the BuildCheck tracing data from additional nodes. /// -/// -internal sealed class BuildCheckTracingEventArgs(Dictionary tracingData) : BuildCheckEventArgs +internal sealed class BuildCheckTracingEventArgs( + BuildCheckTracingData tracingData) : BuildCheckEventArgs { internal BuildCheckTracingEventArgs() - : this([]) - { - } + : this(new BuildCheckTracingData()) + { } - internal BuildCheckTracingEventArgs(Dictionary data, bool isAggregatedGlobalReport) : this(data) => IsAggregatedGlobalReport = isAggregatedGlobalReport; + internal BuildCheckTracingEventArgs(Dictionary executionData) + : this(new BuildCheckTracingData(executionData)) + { } + + internal BuildCheckTracingEventArgs( + BuildCheckTracingData tracingData, + bool isAggregatedGlobalReport) : this(tracingData) => IsAggregatedGlobalReport = isAggregatedGlobalReport; /// /// When true, the tracing information is from the whole build for logging purposes @@ -35,18 +40,42 @@ internal sealed class BuildCheckTracingEventArgs(Dictionary tr /// public bool IsAggregatedGlobalReport { get; private set; } = false; - public Dictionary TracingData { get; private set; } = tracingData; + public BuildCheckTracingData TracingData { get; private set; } = tracingData; internal override void WriteToStream(BinaryWriter writer) { base.WriteToStream(writer); - writer.Write7BitEncodedInt(TracingData.Count); - foreach (KeyValuePair kvp in TracingData) + writer.Write7BitEncodedInt(TracingData.InfrastructureTracingData.Count); + foreach (KeyValuePair kvp in TracingData.InfrastructureTracingData) { writer.Write(kvp.Key); writer.Write(kvp.Value.Ticks); } + + writer.Write7BitEncodedInt(TracingData.TelemetryData.Count); + foreach (BuildCheckRuleTelemetryData data in TracingData.TelemetryData.Values) + { + writer.Write(data.RuleId); + writer.Write(data.CheckFriendlyName); + writer.Write(data.IsBuiltIn); + writer.Write7BitEncodedInt((int)data.DefaultSeverity); + writer.Write7BitEncodedInt(data.ExplicitSeverities.Count); + foreach (DiagnosticSeverity severity in data.ExplicitSeverities) + { + writer.Write7BitEncodedInt((int)severity); + } + writer.Write7BitEncodedInt(data.ProjectNamesWhereEnabled.Count); + foreach (string projectName in data.ProjectNamesWhereEnabled) + { + writer.Write(projectName); + } + writer.Write7BitEncodedInt(data.ViolationMessagesCount); + writer.Write7BitEncodedInt(data.ViolationWarningsCount); + writer.Write7BitEncodedInt(data.ViolationErrorsCount); + writer.Write(data.IsThrottled); + writer.Write(data.TotalRuntime.Ticks); + } } internal override void CreateFromStream(BinaryReader reader, int version) @@ -54,14 +83,59 @@ internal sealed class BuildCheckTracingEventArgs(Dictionary tr base.CreateFromStream(reader, version); int count = reader.Read7BitEncodedInt(); - TracingData = new Dictionary(count); + var infrastructureTracingData = new Dictionary(count); for (int i = 0; i < count; i++) { string key = reader.ReadString(); TimeSpan value = TimeSpan.FromTicks(reader.ReadInt64()); - TracingData.Add(key, value); + infrastructureTracingData.Add(key, value); } + + count = reader.Read7BitEncodedInt(); + List tracingData = new List(count); + for (int i = 0; i < count; i++) + { + string ruleId = reader.ReadString(); + string checkFriendlyName = reader.ReadString(); + bool isBuiltIn = reader.ReadBoolean(); + DiagnosticSeverity defaultSeverity = (DiagnosticSeverity)reader.Read7BitEncodedInt(); + int explicitSeveritiesCount = reader.Read7BitEncodedInt(); + HashSet explicitSeverities = +#if NETSTANDARD2_0 + new HashSet(); +#else + new HashSet(explicitSeveritiesCount); +#endif + for (int j = 0; j < explicitSeveritiesCount; j++) + { + explicitSeverities.Add((DiagnosticSeverity)reader.Read7BitEncodedInt()); + } + int projectNamesWhereEnabledCount = reader.Read7BitEncodedInt(); + HashSet projectNamesWhereEnabled = +#if NETSTANDARD2_0 + new HashSet(); +#else + new HashSet(projectNamesWhereEnabledCount); +#endif + for (int j = 0; j < projectNamesWhereEnabledCount; j++) + { + projectNamesWhereEnabled.Add(reader.ReadString()); + } + int violationMessagesCount = reader.Read7BitEncodedInt(); + int violationWarningsCount = reader.Read7BitEncodedInt(); + int violationErrorsCount = reader.Read7BitEncodedInt(); + bool isThrottled = reader.ReadBoolean(); + TimeSpan totalRuntime = TimeSpan.FromTicks(reader.ReadInt64()); + + BuildCheckRuleTelemetryData data = new BuildCheckRuleTelemetryData( + ruleId, checkFriendlyName, isBuiltIn, defaultSeverity, explicitSeverities, projectNamesWhereEnabled, + violationMessagesCount, violationWarningsCount, violationErrorsCount, isThrottled, totalRuntime); + + tracingData.Add(data); + } + + TracingData = new BuildCheckTracingData(tracingData, infrastructureTracingData); } } diff --git a/src/Framework/BuildCheck/BuildCheckRuleTelemetryData.cs b/src/Framework/BuildCheck/BuildCheckRuleTelemetryData.cs new file mode 100644 index 0000000000..451b8ca55d --- /dev/null +++ b/src/Framework/BuildCheck/BuildCheckRuleTelemetryData.cs @@ -0,0 +1,90 @@ +// 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 System.Collections.Generic; +using System.Linq; + +namespace Microsoft.Build.Experimental.BuildCheck; + +/// +/// Telemetry data for a single build check rule. +/// +/// +/// +/// +/// +internal sealed class BuildCheckRuleTelemetryData( + string ruleId, + string checkFriendlyName, + bool isBuiltIn, + DiagnosticSeverity defaultSeverity) +{ + public BuildCheckRuleTelemetryData( + string ruleId, + string checkFriendlyName, + bool isBuiltIn, + DiagnosticSeverity defaultSeverity, + HashSet explicitSeverities, + HashSet projectNamesWhereEnabled, + int violationMessagesCount, + int violationWarningsCount, + int violationErrorsCount, + bool isThrottled, + TimeSpan totalRuntime) : this(ruleId, checkFriendlyName, isBuiltIn, + defaultSeverity) + { + ExplicitSeverities = explicitSeverities; + ProjectNamesWhereEnabled = projectNamesWhereEnabled; + ViolationMessagesCount = violationMessagesCount; + ViolationWarningsCount = violationWarningsCount; + ViolationErrorsCount = violationErrorsCount; + IsThrottled = isThrottled; + TotalRuntime = totalRuntime; + } + + public static BuildCheckRuleTelemetryData Merge( + BuildCheckRuleTelemetryData data1, + BuildCheckRuleTelemetryData data2) + { + if (data1.RuleId != data2.RuleId) + { + throw new InvalidOperationException("Cannot merge telemetry data for different rules."); + } + return new BuildCheckRuleTelemetryData( + data1.RuleId, + data1.CheckFriendlyName, + data1.IsBuiltIn, + data1.DefaultSeverity, + new HashSet(data1.ExplicitSeverities.Union(data2.ExplicitSeverities)), + new HashSet(data1.ProjectNamesWhereEnabled.Union(data2.ProjectNamesWhereEnabled)), + data1.ViolationMessagesCount + data2.ViolationMessagesCount, + data1.ViolationWarningsCount + data2.ViolationWarningsCount, + data1.ViolationErrorsCount + data2.ViolationErrorsCount, + data1.IsThrottled || data2.IsThrottled, + data1.TotalRuntime + data2.TotalRuntime); + } + + public string RuleId { get; init; } = ruleId; + public string CheckFriendlyName { get; init; } = checkFriendlyName; + public bool IsBuiltIn { get; init; } = isBuiltIn; + public DiagnosticSeverity DefaultSeverity { get; init; } = defaultSeverity; + + /// + /// A set of explicitly set severities (through editorconfig(s)) for the rule. There can be multiple - as different projects can have different settings. + /// + public HashSet ExplicitSeverities { get; init; } = []; + public HashSet ProjectNamesWhereEnabled { get; init; } = []; + public int ViolationMessagesCount { get; private set; } + public int ViolationWarningsCount { get; private set; } + public int ViolationErrorsCount { get; private set; } + public int ViolationsCount => ViolationMessagesCount + ViolationWarningsCount + ViolationErrorsCount; + public bool IsThrottled { get; private set; } + public TimeSpan TotalRuntime { get; set; } + + public void IncrementMessagesCount() => ViolationMessagesCount++; + public void IncrementWarningsCount() => ViolationWarningsCount++; + public void IncrementErrorsCount() => ViolationErrorsCount++; + public void SetThrottled() => IsThrottled = true; +} + diff --git a/src/Framework/BuildCheck/BuildCheckTracingData.cs b/src/Framework/BuildCheck/BuildCheckTracingData.cs new file mode 100644 index 0000000000..dc3ef3cb22 --- /dev/null +++ b/src/Framework/BuildCheck/BuildCheckTracingData.cs @@ -0,0 +1,51 @@ +// 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 System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Microsoft.Build.Experimental.BuildCheck; + +/// +/// Wrapper for the tracing data to be transferred from the worker nodes to the central node. +/// +/// +/// +internal sealed class BuildCheckTracingData( + Dictionary telemetryData, + Dictionary infrastructureTracingData) +{ + public BuildCheckTracingData(IReadOnlyList telemetryData, Dictionary infrastructureTracingData) + : this(telemetryData.ToDictionary(data => data.RuleId), infrastructureTracingData) + { } + + public BuildCheckTracingData() + : this(new Dictionary(), []) + { } + + internal BuildCheckTracingData(Dictionary executionData) + : this(new Dictionary(), executionData) + { } + + public Dictionary TelemetryData { get; private set; } = telemetryData; + public Dictionary InfrastructureTracingData { get; private set; } = infrastructureTracingData; + + /// + /// Gets the runtime stats per individual checks friendly names + /// + public Dictionary ExtractCheckStats() => + // Stats are per rule, while runtime is per check - and check can have multiple rules. + // In case of multi-rule check, the runtime stats are duplicated for each rule. + TelemetryData + .GroupBy(d => d.Value.CheckFriendlyName) + .ToDictionary(g => g.Key, g => g.First().Value.TotalRuntime); + + public void MergeIn(BuildCheckTracingData other) + { + InfrastructureTracingData.Merge(other.InfrastructureTracingData, (span1, span2) => span1 + span2); + TelemetryData.Merge(other.TelemetryData, BuildCheckRuleTelemetryData.Merge); + } +} diff --git a/src/Framework/BuildCheck/DiagnosticSeverity.cs b/src/Framework/BuildCheck/DiagnosticSeverity.cs new file mode 100644 index 0000000000..025b83faa1 --- /dev/null +++ b/src/Framework/BuildCheck/DiagnosticSeverity.cs @@ -0,0 +1,32 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Build.Experimental.BuildCheck; + +internal enum DiagnosticSeverity +{ + /// + /// When set, the default value of the BuildCheck rule will be used. + /// + Default, + + /// + /// When set to None the rule will not run. + /// + None, + + /// + /// Information level message. + /// + Suggestion, + + /// + /// Results a warning in build if the BuildCheck rule applied. + /// + Warning, + + /// + /// Results an error in build if the BuildCheck rule applied. + /// + Error +} diff --git a/src/Build/BuildCheck/Utilities/EnumerableExtensions.cs b/src/Framework/BuildCheck/EnumerableExtensions.cs similarity index 69% rename from src/Build/BuildCheck/Utilities/EnumerableExtensions.cs rename to src/Framework/BuildCheck/EnumerableExtensions.cs index f89f72b0e2..2ec4f958fc 100644 --- a/src/Build/BuildCheck/Utilities/EnumerableExtensions.cs +++ b/src/Framework/BuildCheck/EnumerableExtensions.cs @@ -66,4 +66,33 @@ internal static class EnumerableExtensions } } } + + /// + /// Adds a content of given list to current dictionary. + /// + /// + /// + /// Dictionary to receive another values. + /// List to be merged into current. + /// Way of getting a key of an incoming value. + /// Way of resolving keys conflicts. + public static void Merge( + this IDictionary dict, + IReadOnlyList another, + Func extractKey, + Func mergeValues) + { + foreach (var mergeValue in another) + { + TKey key = extractKey(mergeValue); + if (!dict.TryGetValue(key, out TValue? value)) + { + dict[key] = mergeValue; + } + else + { + dict[key] = mergeValues(value, mergeValue); + } + } + } } diff --git a/src/Framework/BuildException/BuildExceptionBase.cs b/src/Framework/BuildException/BuildExceptionBase.cs index 07db4994b9..426ad35526 100644 --- a/src/Framework/BuildException/BuildExceptionBase.cs +++ b/src/Framework/BuildException/BuildExceptionBase.cs @@ -127,7 +127,7 @@ public abstract class BuildExceptionBase : Exception string? deserializedStackTrace = reader.ReadOptionalString(); string? source = reader.ReadOptionalString(); string? helpLink = reader.ReadOptionalString(); - int hResult = reader.ReadOptionalInt32(); + int hResult = reader.ReadOptionalInt32() ?? 0; IDictionary? customKeyedSerializedData = null; if (reader.ReadByte() == 1) diff --git a/src/Framework/NativeMethods.cs b/src/Framework/NativeMethods.cs index 8df49a7e05..b907742edd 100644 --- a/src/Framework/NativeMethods.cs +++ b/src/Framework/NativeMethods.cs @@ -664,11 +664,20 @@ internal static class NativeMethods } } + private static SAC_State? s_sacState; + /// /// Get from registry state of the Smart App Control (SAC) on the system. /// /// State of SAC internal static SAC_State GetSACState() + { + s_sacState ??= GetSACStateInternal(); + + return s_sacState.Value; + } + + internal static SAC_State GetSACStateInternal() { if (IsWindows) { diff --git a/src/Framework/Telemetry/BuildCheckTelemetry.cs b/src/Framework/Telemetry/BuildCheckTelemetry.cs new file mode 100644 index 0000000000..3b8507203c --- /dev/null +++ b/src/Framework/Telemetry/BuildCheckTelemetry.cs @@ -0,0 +1,96 @@ +// 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 System.Collections.Generic; +using System.Globalization; +using System.Linq; +using Microsoft.Build.Experimental.BuildCheck; + +namespace Microsoft.Build.Framework.Telemetry; + +internal class BuildCheckTelemetry +{ + private const string FailedAcquisitionEventName = "buildcheck/acquisitionfailure"; + private const string RunEventName = "buildcheck/run"; + private const string RuleStatsEventName = "buildcheck/rule"; + private Guid _submissionId = Guid.NewGuid(); + + /// + /// Translates failed acquisition event to telemetry transport data. + /// + internal (string, IDictionary) ProcessCustomCheckLoadingFailure(string assemblyName, + Exception exception) + { + var properties = new Dictionary(); + properties["SubmissionId"] = _submissionId.ToString(); + properties["AssemblyName"] = assemblyName; + string? exceptionType = exception.GetType().FullName; + if (exceptionType != null) + { + properties["ExceptionType"] = exceptionType; + } + if (exception.Message != null) + { + properties["ExceptionMessage"] = exception.Message; + } + + return (FailedAcquisitionEventName, properties); + } + + /// + /// Translates BuildCheck tracing data to telemetry transport data. + /// + internal IEnumerable<(string, IDictionary)> ProcessBuildCheckTracingData(BuildCheckTracingData data) + { + int rulesCount = data.TelemetryData.Count; + int customRulesCount = data.TelemetryData.Count(t => !t.Value.IsBuiltIn); + int violationsCount = data.TelemetryData.Sum(t => t.Value.ViolationsCount); + long runtimeTicks = data.ExtractCheckStats().Sum(v => v.Value.Ticks); + runtimeTicks += data.InfrastructureTracingData.Sum(v => v.Value.Ticks); + TimeSpan totalRuntime = new TimeSpan(runtimeTicks); + + var properties = new Dictionary(); + properties["SubmissionId"] = _submissionId.ToString(); + properties["RulesCount"] = rulesCount.ToString(CultureInfo.InvariantCulture); + properties["CustomRulesCount"] = customRulesCount.ToString(CultureInfo.InvariantCulture); + properties["ViolationsCount"] = violationsCount.ToString(CultureInfo.InvariantCulture); + properties["TotalRuntimeInMilliseconds"] = totalRuntime.TotalMilliseconds.ToString(CultureInfo.InvariantCulture); + + yield return (RunEventName, properties); + + foreach (BuildCheckRuleTelemetryData buildCheckRuleTelemetryData in data.TelemetryData.Values) + { + properties = new Dictionary(); + properties["SubmissionId"] = _submissionId.ToString(); + properties["RuleId"] = buildCheckRuleTelemetryData.RuleId; + properties["CheckFriendlyName"] = buildCheckRuleTelemetryData.CheckFriendlyName; + properties["IsBuiltIn"] = buildCheckRuleTelemetryData.IsBuiltIn.ToString(CultureInfo.InvariantCulture); + properties["DefaultSeverityId"] = ((int)buildCheckRuleTelemetryData.DefaultSeverity).ToString(CultureInfo.InvariantCulture); + properties["DefaultSeverity"] = buildCheckRuleTelemetryData.DefaultSeverity.ToString(); + properties["EnabledProjectsCount"] = buildCheckRuleTelemetryData.ProjectNamesWhereEnabled.Count.ToString(CultureInfo.InvariantCulture); + + if (buildCheckRuleTelemetryData.ExplicitSeverities.Any()) + { + properties["ExplicitSeverities"] = buildCheckRuleTelemetryData.ExplicitSeverities + .Select(s => s.ToString()).ToCsvString(false); + properties["ExplicitSeveritiesIds"] = buildCheckRuleTelemetryData.ExplicitSeverities + .Select(s => ((int)s).ToString(CultureInfo.InvariantCulture)).ToCsvString(false); + } + + properties["ViolationMessagesCount"] = buildCheckRuleTelemetryData.ViolationMessagesCount.ToString(CultureInfo.InvariantCulture); + properties["ViolationWarningsCount"] = buildCheckRuleTelemetryData.ViolationWarningsCount.ToString(CultureInfo.InvariantCulture); + properties["ViolationErrorsCount"] = buildCheckRuleTelemetryData.ViolationErrorsCount.ToString(CultureInfo.InvariantCulture); + properties["IsThrottled"] = buildCheckRuleTelemetryData.IsThrottled.ToString(CultureInfo.InvariantCulture); + properties["TotalRuntimeInMilliseconds"] = buildCheckRuleTelemetryData.TotalRuntime.TotalMilliseconds.ToString(CultureInfo.InvariantCulture); + + yield return (RuleStatsEventName, properties); + } + + + // set for the new submission in case of build server + _submissionId = Guid.NewGuid(); + } +} + + diff --git a/src/Framework/Telemetry/BuildTelemetry.cs b/src/Framework/Telemetry/BuildTelemetry.cs index 7e2e0c6b51..c23d9269c9 100644 --- a/src/Framework/Telemetry/BuildTelemetry.cs +++ b/src/Framework/Telemetry/BuildTelemetry.cs @@ -74,6 +74,16 @@ namespace Microsoft.Build.Framework.Telemetry /// public string? Host { get; set; } + /// + /// True if buildcheck was used. + /// + public bool? BuildCheckEnabled { get; set; } + + /// + /// True if Smart Application Control was enabled. + /// + public bool? SACEnabled { get; set; } + /// /// State of MSBuild server process before this build. /// One of 'cold', 'hot', null (if not run as server) @@ -145,6 +155,16 @@ namespace Microsoft.Build.Framework.Telemetry properties["BuildEngineVersion"] = Version.ToString(); } + if (BuildCheckEnabled != null) + { + properties["BuildCheckEnabled"] = BuildCheckEnabled.Value.ToString(CultureInfo.InvariantCulture); + } + + if (SACEnabled != null) + { + properties["SACEnabled"] = SACEnabled.Value.ToString(CultureInfo.InvariantCulture); + } + return properties; } } diff --git a/src/Framework/Telemetry/KnownTelemetry.cs b/src/Framework/Telemetry/KnownTelemetry.cs index 7685bdda53..e775bce1fe 100644 --- a/src/Framework/Telemetry/KnownTelemetry.cs +++ b/src/Framework/Telemetry/KnownTelemetry.cs @@ -20,4 +20,9 @@ internal static class KnownTelemetry /// Describes how logging was configured. /// public static LoggingConfigurationTelemetry LoggingConfigurationTelemetry { get; } = new LoggingConfigurationTelemetry(); + + /// + /// Describes if and how BuildCheck was used. + /// + public static BuildCheckTelemetry BuildCheckTelemetry { get; } = new BuildCheckTelemetry(); } diff --git a/src/Shared/BinaryReaderExtensions.cs b/src/Shared/BinaryReaderExtensions.cs index 822af5c1a9..7990d261f9 100644 --- a/src/Shared/BinaryReaderExtensions.cs +++ b/src/Shared/BinaryReaderExtensions.cs @@ -22,9 +22,9 @@ namespace Microsoft.Build.Shared #if !TASKHOST [MethodImpl(MethodImplOptions.AggressiveInlining)] #endif - public static int ReadOptionalInt32(this BinaryReader reader) + public static int? ReadOptionalInt32(this BinaryReader reader) { - return reader.ReadByte() == 0 ? 0 : reader.ReadInt32(); + return reader.ReadByte() == 0 ? null : reader.ReadInt32(); } #if !TASKHOST