-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Emmit BuildTelemetry event #7778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
8080b4c
e41cf8a
447225c
c2691b1
60179f8
c994838
8904e83
100887b
b40795c
af99810
b96e4e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| // Copyright (c) Microsoft. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| #nullable disable | ||
| using System; | ||
| using System.Globalization; | ||
| using Microsoft.Build.Framework.Telemetry; | ||
| using Shouldly; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.Build.UnitTests.Telemetry; | ||
|
|
||
| public class KnownTelemetry_Tests | ||
| { | ||
| [Fact] | ||
| public void BuildTelemetryCanBeSetToNull() | ||
| { | ||
| KnownTelemetry.BuildTelemetry = new BuildTelemetry(); | ||
| KnownTelemetry.BuildTelemetry = null; | ||
|
|
||
| KnownTelemetry.BuildTelemetry.ShouldBeNull(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildTelemetryCanBeSet() | ||
| { | ||
| BuildTelemetry buildTelemetry = new BuildTelemetry(); | ||
| KnownTelemetry.BuildTelemetry = buildTelemetry; | ||
|
|
||
| KnownTelemetry.BuildTelemetry.ShouldBeSameAs(buildTelemetry); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildTelemetryConstructedHasNoProperties() | ||
| { | ||
| BuildTelemetry buildTelemetry = new BuildTelemetry(); | ||
|
|
||
| buildTelemetry.DisplayVersion.ShouldBeNull(); | ||
| buildTelemetry.EventName.ShouldBe("build"); | ||
| buildTelemetry.FinishedAt.ShouldBeNull(); | ||
| buildTelemetry.FrameworkName.ShouldBeNull(); | ||
| buildTelemetry.Host.ShouldBeNull(); | ||
| buildTelemetry.InitialServerState.ShouldBeNull(); | ||
| buildTelemetry.InnerStartAt.ShouldBeNull(); | ||
| buildTelemetry.Project.ShouldBeNull(); | ||
| buildTelemetry.ServerFallbackReason.ShouldBeNull(); | ||
| buildTelemetry.StartAt.ShouldBeNull(); | ||
| buildTelemetry.Success.ShouldBeNull(); | ||
| buildTelemetry.Target.ShouldBeNull(); | ||
| buildTelemetry.Version.ShouldBeNull(); | ||
|
|
||
| buildTelemetry.UpdateEventProperties(); | ||
| buildTelemetry.Properties.ShouldBeEmpty(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildTelemetryCreateProperProperties() | ||
| { | ||
| BuildTelemetry buildTelemetry = new BuildTelemetry(); | ||
|
|
||
| DateTime startAt = new DateTime(2023, 01, 02, 10, 11, 22); | ||
| DateTime innerStartAt = new DateTime(2023, 01, 02, 10, 20, 30); | ||
| DateTime finishedAt = new DateTime(2023, 12, 13, 14, 15, 16); | ||
|
|
||
| buildTelemetry.DisplayVersion = "Some Display Version"; | ||
| buildTelemetry.FinishedAt = finishedAt; | ||
| buildTelemetry.FrameworkName = "new .NET"; | ||
| buildTelemetry.Host = "Host description"; | ||
| buildTelemetry.InitialServerState = "hot"; | ||
| buildTelemetry.InnerStartAt = innerStartAt; | ||
| buildTelemetry.Project = @"C:\\dev\\theProject"; | ||
| buildTelemetry.ServerFallbackReason = "busy"; | ||
| buildTelemetry.StartAt = startAt; | ||
| buildTelemetry.Success = true; | ||
| buildTelemetry.Target = "clean"; | ||
| buildTelemetry.Version = new Version(1, 2, 3, 4); | ||
|
|
||
| buildTelemetry.UpdateEventProperties(); | ||
| buildTelemetry.Properties.Count.ShouldBe(11); | ||
|
|
||
| buildTelemetry.Properties["BuildEngineDisplayVersion"].ShouldBe("Some Display Version"); | ||
| buildTelemetry.Properties["BuildEngineFrameworkName"].ShouldBe("new .NET"); | ||
| buildTelemetry.Properties["BuildEngineHost"].ShouldBe("Host description"); | ||
| buildTelemetry.Properties["InitialMSBuildServerState"].ShouldBe("hot"); | ||
| buildTelemetry.Properties["ProjectPath"].ShouldBe(@"C:\\dev\\theProject"); | ||
| buildTelemetry.Properties["ServerFallbackReason"].ShouldBe("busy"); | ||
| buildTelemetry.Properties["BuildSuccess"].ShouldBe("True"); | ||
| buildTelemetry.Properties["BuildTarget"].ShouldBe("clean"); | ||
| buildTelemetry.Properties["BuildEngineVersion"].ShouldBe("1.2.3.4"); | ||
|
|
||
| // verify computed | ||
| buildTelemetry.Properties["BuildDurationInMilliseconds"] = (finishedAt - startAt).TotalMilliseconds.ToString(CultureInfo.InvariantCulture); | ||
| buildTelemetry.Properties["InnerBuildDurationInMilliseconds"] = (finishedAt - innerStartAt).TotalMilliseconds.ToString(CultureInfo.InvariantCulture); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildTelemetryHandleNullsInRecordedTimes() | ||
| { | ||
| BuildTelemetry buildTelemetry = new BuildTelemetry(); | ||
|
|
||
| buildTelemetry.StartAt = DateTime.MinValue; | ||
| buildTelemetry.FinishedAt = null; | ||
| buildTelemetry.UpdateEventProperties(); | ||
| buildTelemetry.Properties.ShouldBeEmpty(); | ||
|
|
||
| buildTelemetry.StartAt = null; | ||
| buildTelemetry.FinishedAt = DateTime.MaxValue; | ||
| buildTelemetry.UpdateEventProperties(); | ||
| buildTelemetry.Properties.ShouldBeEmpty(); | ||
|
|
||
| buildTelemetry.InnerStartAt = DateTime.MinValue; | ||
| buildTelemetry.FinishedAt = null; | ||
| buildTelemetry.UpdateEventProperties(); | ||
| buildTelemetry.Properties.ShouldBeEmpty(); | ||
|
|
||
| buildTelemetry.InnerStartAt = null; | ||
| buildTelemetry.FinishedAt = DateTime.MaxValue; | ||
| buildTelemetry.UpdateEventProperties(); | ||
| buildTelemetry.Properties.ShouldBeEmpty(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
| using Microsoft.Build.Exceptions; | ||
| using Microsoft.Build.Experimental.ProjectCache; | ||
| using Microsoft.Build.Framework; | ||
| using Microsoft.Build.Framework.Telemetry; | ||
| using Microsoft.Build.Graph; | ||
| using Microsoft.Build.Internal; | ||
| using Microsoft.Build.Logging; | ||
|
|
@@ -456,7 +457,7 @@ public void BeginBuild(BuildParameters parameters) | |
| _nodeManager?.ShutdownAllNodes(); | ||
| _taskHostNodeManager?.ShutdownAllNodes(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| _previousLowPriority = parameters.LowPriority; | ||
|
|
@@ -470,6 +471,14 @@ public void BeginBuild(BuildParameters parameters) | |
|
|
||
| MSBuildEventSource.Log.BuildStart(); | ||
|
|
||
| // Initiate build telemetry data | ||
| DateTime now = DateTime.UtcNow; | ||
| KnownTelemetry.BuildTelemetry ??= new() | ||
| { | ||
| StartAt = now, | ||
| }; | ||
| KnownTelemetry.BuildTelemetry.InnerStartAt = now; | ||
|
|
||
| if (BuildParameters.DumpOpportunisticInternStats) | ||
| { | ||
| Strings.EnableDiagnostics(); | ||
|
|
@@ -796,6 +805,13 @@ public BuildSubmission PendBuildRequest(BuildRequestData requestData) | |
| VerifyStateInternal(BuildManagerState.Building); | ||
|
|
||
| var newSubmission = new BuildSubmission(this, GetNextSubmissionId(), requestData, _buildParameters.LegacyThreadingSemantics); | ||
|
|
||
| if (KnownTelemetry.BuildTelemetry != null) | ||
| { | ||
| KnownTelemetry.BuildTelemetry.Project ??= requestData.ProjectFullPath; | ||
| KnownTelemetry.BuildTelemetry.Target ??= string.Join(",", requestData.TargetNames); | ||
| } | ||
|
|
||
| _buildSubmissions.Add(newSubmission.SubmissionId, newSubmission); | ||
| _noActiveSubmissionsEvent.Reset(); | ||
| return newSubmission; | ||
|
|
@@ -817,6 +833,13 @@ public GraphBuildSubmission PendBuildRequest(GraphBuildRequestData requestData) | |
| VerifyStateInternal(BuildManagerState.Building); | ||
|
|
||
| var newSubmission = new GraphBuildSubmission(this, GetNextSubmissionId(), requestData); | ||
|
|
||
| if (KnownTelemetry.BuildTelemetry != null) | ||
| { | ||
| KnownTelemetry.BuildTelemetry.Project ??= requestData.ProjectGraphEntryPoints?.FirstOrDefault().ProjectFile; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can there be multiple graph entry points? If so, is this right?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There can be, however, this is probably very rare situation and we are fine to have it slightly wrong. |
||
| KnownTelemetry.BuildTelemetry.Target ??= string.Join(",", requestData.TargetNames); | ||
| } | ||
|
|
||
| _graphBuildSubmissions.Add(newSubmission.SubmissionId, newSubmission); | ||
| _noActiveSubmissionsEvent.Reset(); | ||
| return newSubmission; | ||
|
|
@@ -965,6 +988,36 @@ public void EndBuild() | |
| } | ||
|
|
||
| loggingService.LogBuildFinished(_overallBuildSuccess); | ||
|
|
||
| if (KnownTelemetry.BuildTelemetry != null) | ||
| { | ||
| KnownTelemetry.BuildTelemetry.FinishedAt = DateTime.UtcNow; | ||
| KnownTelemetry.BuildTelemetry.Success = _overallBuildSuccess; | ||
| KnownTelemetry.BuildTelemetry.Version = ProjectCollection.Version; | ||
| KnownTelemetry.BuildTelemetry.DisplayVersion = ProjectCollection.DisplayVersion; | ||
| KnownTelemetry.BuildTelemetry.FrameworkName = NativeMethodsShared.FrameworkName; | ||
| NativeMethodsShared.GetOSNameForExtensionsPath(); | ||
|
rokonec marked this conversation as resolved.
Outdated
|
||
|
|
||
| string host = null; | ||
| if (BuildEnvironmentState.s_runningInVisualStudio) | ||
| { | ||
| host = "VS"; | ||
| } | ||
| else if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILD_HOST_NAME"))) | ||
| { | ||
| host = Environment.GetEnvironmentVariable("MSBUILD_HOST_NAME"); | ||
| } | ||
| else if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable("VSCODE_CWD")) || Environment.GetEnvironmentVariable("TERM_PROGRAM") == "vscode") | ||
| { | ||
| host = "VSCode"; | ||
| } | ||
| KnownTelemetry.BuildTelemetry.Host = host; | ||
|
|
||
| KnownTelemetry.BuildTelemetry.UpdateEventProperties(); | ||
| loggingService.LogTelemetry(buildEventContext: null, KnownTelemetry.BuildTelemetry.EventName, KnownTelemetry.BuildTelemetry.Properties); | ||
| // Clean telemetry which makes it ready for next build submission. | ||
| KnownTelemetry.BuildTelemetry = null; | ||
| } | ||
| } | ||
|
|
||
| ShutdownLoggingService(loggingService); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| using Microsoft.Build.Eventing; | ||
| using Microsoft.Build.Execution; | ||
| using Microsoft.Build.Framework; | ||
| using Microsoft.Build.Framework.Telemetry; | ||
| using Microsoft.Build.Internal; | ||
| using Microsoft.Build.Shared; | ||
|
|
||
|
|
@@ -152,6 +153,10 @@ public MSBuildClientExitResult Execute(CancellationToken cancellationToken) | |
|
|
||
| // Start server it if is not running. | ||
| bool serverIsAlreadyRunning = ServerNamedMutex.WasOpen(serverRunningMutexName); | ||
| if (KnownTelemetry.BuildTelemetry != null) | ||
| { | ||
| KnownTelemetry.BuildTelemetry.InitialServerState = serverIsAlreadyRunning ? "hot" : "cold"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this be an enum?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer it this way. For telemetry data I like the simplicity of adding new value and transparent passing to telemetry storage. |
||
| } | ||
| if (!serverIsAlreadyRunning) | ||
| { | ||
| CommunicationsUtilities.Trace("Server was not running. Starting server now."); | ||
|
|
@@ -432,13 +437,23 @@ private ServerNodeBuildCommand GetServerNodeBuildCommand() | |
| // We remove env variable used to invoke MSBuild server as that might be equal to 1, so we do not get an infinite recursion here. | ||
| envVars[Traits.UseMSBuildServerEnvVarName] = "0"; | ||
|
|
||
| Debug.Assert(KnownTelemetry.BuildTelemetry == null || KnownTelemetry.BuildTelemetry.StartAt.HasValue, "BuildTelemetry.StartAt was not initialized!"); | ||
|
|
||
| PartialBuildTelemetry? partialBuildTelemetry = KnownTelemetry.BuildTelemetry == null | ||
| ? null | ||
| : new PartialBuildTelemetry( | ||
| startedAt: KnownTelemetry.BuildTelemetry.StartAt.GetValueOrDefault(), | ||
| initialServerState: KnownTelemetry.BuildTelemetry.InitialServerState, | ||
| serverFallbackReason: KnownTelemetry.BuildTelemetry.ServerFallbackReason); | ||
|
|
||
| return new ServerNodeBuildCommand( | ||
| _commandLine, | ||
| startupDirectory: Directory.GetCurrentDirectory(), | ||
| buildProcessEnvironment: envVars, | ||
| CultureInfo.CurrentCulture, | ||
| CultureInfo.CurrentUICulture, | ||
| _consoleConfiguration!); | ||
| _consoleConfiguration!, | ||
| partialBuildTelemetry); | ||
| } | ||
|
|
||
| private ServerNodeHandshake GetHandshake() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| // Copyright (c) Microsoft. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using System; | ||
|
|
||
| namespace Microsoft.Build.BackEnd; | ||
|
|
||
| /// <summary> | ||
| /// Part of BuildTelemetry which is collected on client and needs to be sent to server, | ||
| /// so server can log BuildTelemetry once it is finished. | ||
| /// </summary> | ||
| internal sealed class PartialBuildTelemetry : ITranslatable | ||
| { | ||
| private DateTime _startedAt = default; | ||
| private string? _initialServerState = default; | ||
| private string? _serverFallbackReason = default; | ||
|
|
||
| public PartialBuildTelemetry(DateTime startedAt, string? initialServerState, string? serverFallbackReason) | ||
| { | ||
| _startedAt = startedAt; | ||
| _initialServerState = initialServerState; | ||
| _serverFallbackReason = serverFallbackReason; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Constructor for deserialization | ||
| /// </summary> | ||
| private PartialBuildTelemetry() | ||
| { | ||
| } | ||
|
|
||
| public DateTime? StartedAt => _startedAt; | ||
|
|
||
| public string? InitialServerState => _initialServerState; | ||
|
|
||
| public string? ServerFallbackReason => _serverFallbackReason; | ||
|
|
||
| public void Translate(ITranslator translator) | ||
| { | ||
| translator.Translate(ref _startedAt); | ||
| translator.Translate(ref _initialServerState); | ||
| translator.Translate(ref _serverFallbackReason); | ||
| } | ||
|
|
||
| internal static PartialBuildTelemetry FactoryForDeserialization(ITranslator translator) | ||
| { | ||
| PartialBuildTelemetry partialTelemetryData = new(); | ||
| partialTelemetryData.Translate(translator); | ||
| return partialTelemetryData; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My general worry with allocating a lot of strings here for telemetry is that although it isn't a huge deal because it's a cold path, it worsens perf for users without helping them in any way, just helping us help them. Am I wrong about that? Are these telemetry events helpful to users in some way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no direct value for users. They could subscribe to our telemetry events in their own logger and somehow process it, but this scenarios is farfetched.
However, this is only one telemetry allocation per whole build and its affect on overall performance will be negligible.