Skip to content

Commit aeabd8b

Browse files
JanKrivanekJanKrivanek
authored
Add BuildCheck basic telemetry (#10652)
* Add initial version of telemetry data extracting * Initial tracing data transport * Move buildcheck enabled telemetry to buildtelemetry * Add unittests * Fix typos * Add CheckFriendlyName to telemetry * Add SAC telemetry * Adjust after merging
1 parent e4797f3 commit aeabd8b

33 files changed

+725
-89
lines changed

‎src/Build.UnitTests/BackEnd/NodePackets_Tests.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using FluentAssertions;
99
using Microsoft.Build.BackEnd;
1010
using Microsoft.Build.Execution;
11+
using Microsoft.Build.Experimental.BuildCheck;
1112
using Microsoft.Build.Framework;
1213
using Microsoft.Build.Shared;
1314
using Xunit;
@@ -77,6 +78,7 @@ public void VerifyEventType()
7778
EnvironmentVariableReadEventArgs environmentVariableRead = new("env", "message", "file", 0, 0);
7879
GeneratedFileUsedEventArgs generatedFileUsed = new GeneratedFileUsedEventArgs("path", "some content");
7980
BuildSubmissionStartedEventArgs buildSubmissionStarted = new(new Dictionary<string, string> { { "Value1", "Value2" } }, ["Path1"], ["TargetName"], BuildRequestDataFlags.ReplaceExistingProjectInstance, 123);
81+
BuildCheckTracingEventArgs buildCheckTracing = new();
8082

8183
VerifyLoggingPacket(buildFinished, LoggingEventType.BuildFinishedEvent);
8284
VerifyLoggingPacket(buildStarted, LoggingEventType.BuildStartedEvent);
@@ -111,6 +113,7 @@ public void VerifyEventType()
111113
VerifyLoggingPacket(environmentVariableRead, LoggingEventType.EnvironmentVariableReadEvent);
112114
VerifyLoggingPacket(generatedFileUsed, LoggingEventType.GeneratedFileUsedEvent);
113115
VerifyLoggingPacket(buildSubmissionStarted, LoggingEventType.BuildSubmissionStartedEvent);
116+
VerifyLoggingPacket(buildCheckTracing, LoggingEventType.BuildCheckTracingEvent);
114117
}
115118

116119
private static BuildEventContext CreateBuildEventContext()

‎src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
44
using System;
55
using System.Collections;
66
using System.Collections.Generic;
7+
using System.Globalization;
78
using System.IO;
89
using System.Linq;
910
using System.Text;
1011
using FluentAssertions;
1112
using Microsoft.Build.BackEnd;
13+
using Microsoft.Build.Experimental.BuildCheck;
1214
using Microsoft.Build.Framework;
1315
using Microsoft.Build.Framework.Profiler;
1416
using Microsoft.Build.Logging;
@@ -530,6 +532,27 @@ public void RoundtripAssemblyLoadBuild()
530532
e => string.Join(", ", e.RawArguments ?? Array.Empty<object>()));
531533
}
532534

535+
[Fact]
536+
public void RoundtripBuildCheckTracingEventArgs()
537+
{
538+
string key1 = "AA";
539+
TimeSpan span1 = TimeSpan.FromSeconds(5);
540+
string key2 = "b";
541+
TimeSpan span2 = TimeSpan.FromSeconds(15);
542+
string key3 = "cCc";
543+
TimeSpan span3 = TimeSpan.FromSeconds(50);
544+
545+
Dictionary<string, TimeSpan> stats = new() { { key1, span1 }, { key2, span2 }, { key3, span3 } };
546+
547+
BuildCheckTracingEventArgs args = new BuildCheckTracingEventArgs(stats);
548+
549+
Roundtrip(args,
550+
e => e.TracingData.InfrastructureTracingData.Keys.Count.ToString(),
551+
e => e.TracingData.InfrastructureTracingData.Keys.ToCsvString(false),
552+
e => e.TracingData.InfrastructureTracingData.Values
553+
.Select(v => v.TotalSeconds.ToString(CultureInfo.InvariantCulture)).ToCsvString(false));
554+
}
555+
533556
[Theory]
534557
[InlineData(true)]
535558
[InlineData(false)]

‎src/Build/BackEnd/BuildManager/BuildManager.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,6 +1070,11 @@ public void EndBuild()
10701070
}
10711071
_buildTelemetry.Host = host;
10721072

1073+
_buildTelemetry.BuildCheckEnabled = _buildParameters!.IsBuildCheckEnabled;
1074+
var sacState = NativeMethodsShared.GetSACState();
1075+
// The Enforcement would lead to build crash - but let's have the check for completeness sake.
1076+
_buildTelemetry.SACEnabled = sacState == NativeMethodsShared.SAC_State.Evaluation || sacState == NativeMethodsShared.SAC_State.Enforcement;
1077+
10731078
loggingService.LogTelemetry(buildEventContext: null, _buildTelemetry.EventName, _buildTelemetry.GetProperties());
10741079
// Clean telemetry to make it ready for next build submission.
10751080
_buildTelemetry = null;

‎src/Build/BuildCheck/API/Check.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ public abstract class Check : IDisposable
4343
/// </param>
4444
public abstract void RegisterActions(IBuildCheckRegistrationContext registrationContext);
4545

46+
internal virtual bool IsBuiltIn => false;
47+
4648
public virtual void Dispose()
4749
{ }
4850
}

‎src/Build/BuildCheck/API/InternalCheck.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,6 @@ public override void RegisterActions(IBuildCheckRegistrationContext registration
2828

2929
this.RegisterInternalActions(internalRegistrationContext);
3030
}
31+
32+
internal override bool IsBuiltIn => true;
3133
}

‎src/Build/BuildCheck/Acquisition/BuildCheckAcquisitionModule.cs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Reflection;
88
using Microsoft.Build.Experimental.BuildCheck.Infrastructure;
99
using Microsoft.Build.Framework;
10+
using Microsoft.Build.Framework.Telemetry;
1011
using Microsoft.Build.Shared;
1112

1213
namespace Microsoft.Build.Experimental.BuildCheck.Acquisition;
@@ -53,21 +54,24 @@ public List<CheckFactory> CreateCheckFactories(
5354
.ForEach(t => checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckBaseTypeNotAssignable", t.Name, t.Assembly));
5455
}
5556
}
56-
catch (ReflectionTypeLoadException ex)
57+
catch (ReflectionTypeLoadException ex) when (ex.LoaderExceptions.Length != 0)
5758
{
58-
if (ex.LoaderExceptions.Length != 0)
59+
foreach (Exception? unrolledEx in ex.LoaderExceptions.Where(e => e != null).Prepend(ex))
5960
{
60-
foreach (Exception? loaderException in ex.LoaderExceptions)
61-
{
62-
checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckFailedRuleLoading", loaderException?.Message);
63-
}
61+
ReportLoadingError(unrolledEx!);
6462
}
6563
}
6664
catch (Exception ex)
6765
{
68-
checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckFailedRuleLoading", ex?.Message);
66+
ReportLoadingError(ex);
6967
}
7068

7169
return checksFactories;
70+
71+
void ReportLoadingError(Exception ex)
72+
{
73+
checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckFailedRuleLoading", ex.Message);
74+
checkContext.DispatchFailedAcquisitionTelemetry(System.IO.Path.GetFileName(checkAcquisitionData.AssemblyPath), ex);
75+
}
7276
}
7377
}

‎src/Build/BuildCheck/Checks/DoubleWritesCheck.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ public override void RegisterActions(IBuildCheckRegistrationContext registration
4242
registrationContext.RegisterTaskInvocationAction(TaskInvocationAction);
4343
}
4444

45+
internal override bool IsBuiltIn => true;
46+
4547
/// <summary>
4648
/// Contains the first project file + task that wrote the given file during the build.
4749
/// </summary>
@@ -126,5 +128,5 @@ private void CheckWrite(BuildCheckDataContext<TaskInvocationCheckData> context,
126128
_filesWritten.Add(fileBeingWritten, (context.Data.ProjectFilePath, context.Data.TaskName));
127129
}
128130
}
129-
}
131+
}
130132
}

‎src/Build/BuildCheck/Checks/NoEnvironmentVariablePropertyCheck.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ public override void Initialize(ConfigurationContext configurationContext)
4949

5050
public override void RegisterActions(IBuildCheckRegistrationContext registrationContext) => registrationContext.RegisterEnvironmentVariableReadAction(ProcessEnvironmentVariableReadAction);
5151

52+
internal override bool IsBuiltIn => true;
53+
5254
private void ProcessEnvironmentVariableReadAction(BuildCheckDataContext<EnvironmentVariableCheckData> context)
5355
{
5456
EnvironmentVariableIdentityKey identityKey = new(context.Data.EnvironmentVariableName, context.Data.EnvironmentVariableLocation);

‎src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ public override void RegisterInternalActions(IInternalCheckRegistrationContext r
118118
}
119119
}
120120

121+
internal override bool IsBuiltIn => true;
122+
121123
private Dictionary<string, IMSBuildElementLocation?> _writenProperties = new(MSBuildNameIgnoreCaseComparer.Default);
122124
private HashSet<string> _readProperties = new(MSBuildNameIgnoreCaseComparer.Default);
123125
// For the 'Property Initialized after used' check - we are interested in cases where:

‎src/Build/BuildCheck/Checks/SharedOutputPathCheck.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ public override void RegisterActions(IBuildCheckRegistrationContext registration
3535
registrationContext.RegisterEvaluatedPropertiesAction(EvaluatedPropertiesAction);
3636
}
3737

38+
internal override bool IsBuiltIn => true;
39+
3840
private readonly Dictionary<string, string> _projectsPerOutputPath = new(StringComparer.CurrentCultureIgnoreCase);
3941
private readonly HashSet<string> _projects = new(StringComparer.CurrentCultureIgnoreCase);
4042

‎src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ private void HandleBuildCheckTracingEvent(BuildCheckTracingEventArgs eventArgs)
107107
{
108108
if (!eventArgs.IsAggregatedGlobalReport)
109109
{
110-
_stats.Merge(eventArgs.TracingData, (span1, span2) => span1 + span2);
110+
_tracingData.MergeIn(eventArgs.TracingData);
111111
}
112112
}
113113

@@ -138,36 +138,25 @@ private void HandleEnvironmentVariableReadEvent(EnvironmentVariableReadEventArgs
138138

139139
private bool IsMetaProjFile(string? projectFile) => projectFile?.EndsWith(".metaproj", StringComparison.OrdinalIgnoreCase) == true;
140140

141-
private readonly Dictionary<string, TimeSpan> _stats = new Dictionary<string, TimeSpan>();
141+
private readonly BuildCheckTracingData _tracingData = new BuildCheckTracingData();
142142

143143
private void HandleBuildFinishedEvent(BuildFinishedEventArgs eventArgs)
144144
{
145145
_buildCheckManager.ProcessBuildFinished(_checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!));
146146

147-
_stats.Merge(_buildCheckManager.CreateCheckTracingStats(), (span1, span2) => span1 + span2);
147+
_tracingData.MergeIn(_buildCheckManager.CreateCheckTracingStats());
148148

149149
LogCheckStats(_checkContextFactory.CreateCheckContext(GetBuildEventContext(eventArgs)));
150150
}
151151

152152
private void LogCheckStats(ICheckContext checkContext)
153153
{
154-
Dictionary<string, TimeSpan> infraStats = new Dictionary<string, TimeSpan>();
155-
Dictionary<string, TimeSpan> checkStats = new Dictionary<string, TimeSpan>();
154+
Dictionary<string, TimeSpan> infraStats = _tracingData.InfrastructureTracingData;
155+
// Stats are per rule, while runtime is per check - and check can have multiple rules.
156+
// In case of multi-rule check, the runtime stats are duplicated for each rule.
157+
Dictionary<string, TimeSpan> checkStats = _tracingData.ExtractCheckStats();
156158

157-
foreach (var stat in _stats)
158-
{
159-
if (stat.Key.StartsWith(BuildCheckConstants.infraStatPrefix))
160-
{
161-
string newKey = stat.Key.Substring(BuildCheckConstants.infraStatPrefix.Length);
162-
infraStats[newKey] = stat.Value;
163-
}
164-
else
165-
{
166-
checkStats[stat.Key] = stat.Value;
167-
}
168-
}
169-
170-
BuildCheckTracingEventArgs statEvent = new BuildCheckTracingEventArgs(_stats, true)
159+
BuildCheckTracingEventArgs statEvent = new BuildCheckTracingEventArgs(_tracingData, true)
171160
{ BuildEventContext = checkContext.BuildEventContext };
172161

173162
checkContext.DispatchBuildEvent(statEvent);
@@ -177,6 +166,7 @@ private void LogCheckStats(ICheckContext checkContext)
177166
checkContext.DispatchAsCommentFromText(MessageImportance.Low, infraData);
178167
string checkData = BuildCsvString("Checks run times", checkStats);
179168
checkContext.DispatchAsCommentFromText(MessageImportance.Low, checkData);
169+
checkContext.DispatchTelemetry(_tracingData);
180170
}
181171

182172
private string BuildCsvString(string title, Dictionary<string, TimeSpan> rowData)

‎src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,10 @@ private void SetupSingleCheck(CheckFactoryContext checkFactoryContext, string pr
229229
CheckConfigurationEffective[] configurations;
230230
if (checkFactoryContext.MaterializedCheck == null)
231231
{
232-
CheckConfiguration[] userConfigs =
232+
CheckConfiguration[] userEditorConfigs =
233233
_configurationProvider.GetUserConfigurations(projectFullPath, checkFactoryContext.RuleIds);
234234

235-
if (userConfigs.All(c => !(c.IsEnabled ?? checkFactoryContext.IsEnabledByDefault)))
235+
if (userEditorConfigs.All(c => !(c.IsEnabled ?? checkFactoryContext.IsEnabledByDefault)))
236236
{
237237
// the check was not yet instantiated nor mounted - so nothing to do here now.
238238
return;
@@ -242,7 +242,7 @@ private void SetupSingleCheck(CheckFactoryContext checkFactoryContext, string pr
242242
_configurationProvider.GetCustomConfigurations(projectFullPath, checkFactoryContext.RuleIds);
243243

244244
Check uninitializedCheck = checkFactoryContext.Factory();
245-
configurations = _configurationProvider.GetMergedConfigurations(userConfigs, uninitializedCheck);
245+
configurations = _configurationProvider.GetMergedConfigurations(userEditorConfigs, uninitializedCheck);
246246

247247
ConfigurationContext configurationContext = ConfigurationContext.FromDataEnumeration(customConfigData, configurations);
248248

@@ -271,21 +271,23 @@ private void SetupSingleCheck(CheckFactoryContext checkFactoryContext, string pr
271271
// price to be paid in that case is slight performance cost.
272272

273273
// Create the wrapper and register to central context
274-
wrapper.StartNewProject(projectFullPath, configurations);
274+
wrapper.StartNewProject(projectFullPath, configurations, userEditorConfigs);
275275
var wrappedContext = new CheckRegistrationContext(wrapper, _buildCheckCentralContext);
276276
check.RegisterActions(wrappedContext);
277277
}
278278
else
279279
{
280280
wrapper = checkFactoryContext.MaterializedCheck;
281281

282-
configurations = _configurationProvider.GetMergedConfigurations(projectFullPath, wrapper.Check);
282+
CheckConfiguration[] userEditorConfigs =
283+
_configurationProvider.GetUserConfigurations(projectFullPath, checkFactoryContext.RuleIds);
284+
configurations = _configurationProvider.GetMergedConfigurations(userEditorConfigs, wrapper.Check);
283285

284286
_configurationProvider.CheckCustomConfigurationDataValidity(projectFullPath,
285287
checkFactoryContext.RuleIds[0]);
286288

287289
// Update the wrapper
288-
wrapper.StartNewProject(projectFullPath, configurations);
290+
wrapper.StartNewProject(projectFullPath, configurations, userEditorConfigs);
289291
}
290292
}
291293

@@ -346,7 +348,7 @@ private void RemoveCheck(CheckFactoryContext checkToRemove)
346348
if (checkToRemove.MaterializedCheck is not null)
347349
{
348350
_buildCheckCentralContext.DeregisterCheck(checkToRemove.MaterializedCheck);
349-
_tracingReporter.AddCheckStats(checkToRemove.MaterializedCheck.Check.FriendlyName, checkToRemove.MaterializedCheck.Elapsed);
351+
_ruleTelemetryData.AddRange(checkToRemove.MaterializedCheck.GetRuleTelemetryData());
350352
checkToRemove.MaterializedCheck.Check.Dispose();
351353
}
352354
}
@@ -411,19 +413,18 @@ public void ProcessTaskParameterEventArgs(
411413
=> _buildEventsProcessor
412414
.ProcessTaskParameterEventArgs(checkContext, taskParameterEventArgs);
413415

414-
public Dictionary<string, TimeSpan> CreateCheckTracingStats()
416+
private readonly List<BuildCheckRuleTelemetryData> _ruleTelemetryData = [];
417+
public BuildCheckTracingData CreateCheckTracingStats()
415418
{
416419
foreach (CheckFactoryContext checkFactoryContext in _checkRegistry)
417420
{
418421
if (checkFactoryContext.MaterializedCheck != null)
419422
{
420-
_tracingReporter.AddCheckStats(checkFactoryContext.FriendlyName, checkFactoryContext.MaterializedCheck.Elapsed);
421-
checkFactoryContext.MaterializedCheck.ClearStats();
423+
_ruleTelemetryData.AddRange(checkFactoryContext.MaterializedCheck.GetRuleTelemetryData());
422424
}
423425
}
424426

425-
_tracingReporter.AddCheckInfraStats();
426-
return _tracingReporter.TracingStats;
427+
return new BuildCheckTracingData(_ruleTelemetryData, _tracingReporter.GetInfrastructureTracingStats());
427428
}
428429

429430
public void FinalizeProcessing(LoggingContext loggingContext)

‎src/Build/BuildCheck/Infrastructure/CheckContext/CheckDispatchingContext.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,12 @@ public void DispatchAsWarningFromText(string? subcategoryResourceName, string? e
6969

7070
_eventDispatcher.Dispatch(buildEvent);
7171
}
72+
73+
public void DispatchFailedAcquisitionTelemetry(string assemblyName, Exception exception)
74+
// This is it - no action for replay mode.
75+
{ }
76+
77+
public void DispatchTelemetry(BuildCheckTracingData data)
78+
// This is it - no action for replay mode.
79+
{ }
7280
}

‎src/Build/BuildCheck/Infrastructure/CheckContext/CheckLoggingContext.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Threading.Tasks;
99
using Microsoft.Build.BackEnd.Logging;
1010
using Microsoft.Build.Framework;
11+
using Microsoft.Build.Framework.Telemetry;
1112
using Microsoft.Build.Shared;
1213

1314
namespace Microsoft.Build.Experimental.BuildCheck;
@@ -43,4 +44,18 @@ public void DispatchAsErrorFromText(string? subcategoryResourceName, string? err
4344
public void DispatchAsWarningFromText(string? subcategoryResourceName, string? errorCode, string? helpKeyword, BuildEventFileInfo file, string message)
4445
=> loggingService
4546
.LogWarningFromText(eventContext, subcategoryResourceName, errorCode, helpKeyword, file, message);
47+
48+
public void DispatchFailedAcquisitionTelemetry(string assemblyName, Exception exception)
49+
{
50+
var telemetryTransportData = KnownTelemetry.BuildCheckTelemetry.ProcessCustomCheckLoadingFailure(assemblyName, exception);
51+
loggingService.LogTelemetry(eventContext, telemetryTransportData.Item1, telemetryTransportData.Item2);
52+
}
53+
54+
public void DispatchTelemetry(BuildCheckTracingData data)
55+
{
56+
foreach ((string, IDictionary<string, string>) telemetryTransportData in KnownTelemetry.BuildCheckTelemetry.ProcessBuildCheckTracingData(data))
57+
{
58+
loggingService.LogTelemetry(eventContext, telemetryTransportData.Item1, telemetryTransportData.Item2);
59+
}
60+
}
4661
}

‎src/Build/BuildCheck/Infrastructure/CheckContext/ICheckContext.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,14 @@ internal interface ICheckContext
4545
/// Dispatch the instance of <see cref="BuildEventContext"/> as a warning message.
4646
/// </summary>
4747
void DispatchAsWarningFromText(string? subcategoryResourceName, string? errorCode, string? helpKeyword, BuildEventFileInfo file, string message);
48+
49+
/// <summary>
50+
/// Dispatch the telemetry data for a failed acquisition.
51+
/// </summary>
52+
void DispatchFailedAcquisitionTelemetry(string assemblyName, Exception exception);
53+
54+
/// <summary>
55+
/// If supported - dispatches the telemetry data.
56+
/// </summary>
57+
void DispatchTelemetry(BuildCheckTracingData data);
4858
}

0 commit comments

Comments
 (0)