Skip to content

Commit 9418427

Browse files
baronfelbaronfel
authored
ensure that all TargetFrameworkEval properties are sent without any m… (#50930)
1 parent 14a1d9e commit 9418427

File tree

5 files changed

+44
-61
lines changed

5 files changed

+44
-61
lines changed

‎src/Cli/dotnet/Commands/MSBuild/MSBuildLogger.cs‎

Lines changed: 33 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
#nullable disable
5-
64
using System.Globalization;
75
using Microsoft.Build.Framework;
86
using Microsoft.DotNet.Cli.Telemetry;
@@ -15,7 +13,7 @@ public sealed class MSBuildLogger : INodeLogger
1513
{
1614
private readonly IFirstTimeUseNoticeSentinel _sentinel =
1715
new FirstTimeUseNoticeSentinel();
18-
private readonly ITelemetry _telemetry;
16+
private readonly ITelemetry? _telemetry = null;
1917

2018
internal const string TargetFrameworkTelemetryEventName = "targetframeworkeval";
2119
internal const string BuildTelemetryEventName = "build";
@@ -54,7 +52,7 @@ public MSBuildLogger()
5452
{
5553
try
5654
{
57-
string sessionId =
55+
string? sessionId =
5856
Environment.GetEnvironmentVariable(MSBuildForwardingApp.TelemetrySessionIdEnvironmentVariableName);
5957

6058
if (sessionId != null)
@@ -105,34 +103,13 @@ public void Initialize(IEventSource eventSource)
105103
}
106104
}
107105

108-
internal static void FormatAndSend(ITelemetry telemetry, TelemetryEventArgs args)
106+
internal static void FormatAndSend(ITelemetry? telemetry, TelemetryEventArgs args)
109107
{
110108
switch (args.EventName)
111109
{
112110
case TargetFrameworkTelemetryEventName:
113-
{
114-
var newEventName = $"msbuild/{TargetFrameworkTelemetryEventName}";
115-
Dictionary<string, string> maskedProperties = [];
116-
117-
foreach (var key in new[] {
118-
TargetFrameworkVersionTelemetryPropertyKey,
119-
RuntimeIdentifierTelemetryPropertyKey,
120-
SelfContainedTelemetryPropertyKey,
121-
UseApphostTelemetryPropertyKey,
122-
OutputTypeTelemetryPropertyKey,
123-
UseArtifactsOutputTelemetryPropertyKey,
124-
ArtifactsPathLocationTypeTelemetryPropertyKey
125-
})
126-
{
127-
if (args.Properties.TryGetValue(key, out string value))
128-
{
129-
maskedProperties.Add(key, Sha256Hasher.HashWithNormalizedCasing(value));
130-
}
131-
}
132-
133-
telemetry.TrackEvent(newEventName, maskedProperties, measurements: null);
134-
break;
135-
}
111+
TrackEvent(telemetry, $"msbuild/{TargetFrameworkTelemetryEventName}", args.Properties, [], []);
112+
break;
136113
case BuildTelemetryEventName:
137114
TrackEvent(telemetry, $"msbuild/{BuildTelemetryEventName}", args.Properties,
138115
toBeHashed: ["ProjectPath", "BuildTarget"],
@@ -174,33 +151,44 @@ internal static void FormatAndSend(ITelemetry telemetry, TelemetryEventArgs args
174151
}
175152
}
176153

177-
private static void TrackEvent(ITelemetry telemetry, string eventName, IDictionary<string, string> eventProperties, string[] toBeHashed, string[] toBeMeasured)
154+
private static void TrackEvent(ITelemetry? telemetry, string eventName, IDictionary<string, string?> eventProperties, string[]? toBeHashed, string[]? toBeMeasured)
178155
{
179-
Dictionary<string, string> properties = null;
180-
Dictionary<string, double> measurements = null;
156+
if (telemetry == null || !telemetry.Enabled)
157+
{
158+
return;
159+
}
160+
161+
Dictionary<string, string?>? properties = null;
162+
Dictionary<string, double>? measurements = null;
181163

182-
foreach (var propertyToBeHashed in toBeHashed)
164+
if (toBeHashed is not null)
183165
{
184-
if (eventProperties.TryGetValue(propertyToBeHashed, out string value))
166+
foreach (var propertyToBeHashed in toBeHashed)
185167
{
186-
// Lets lazy allocate in case there is tons of telemetry
187-
properties ??= new Dictionary<string, string>(eventProperties);
188-
properties[propertyToBeHashed] = Sha256Hasher.HashWithNormalizedCasing(value);
168+
if (eventProperties.TryGetValue(propertyToBeHashed, out var value))
169+
{
170+
// Lets lazy allocate in case there is tons of telemetry
171+
properties ??= new Dictionary<string, string?>(eventProperties);
172+
properties[propertyToBeHashed] = Sha256Hasher.HashWithNormalizedCasing(value!);
173+
}
189174
}
190175
}
191176

192-
foreach (var propertyToBeMeasured in toBeMeasured)
177+
if (toBeMeasured is not null)
193178
{
194-
if (eventProperties.TryGetValue(propertyToBeMeasured, out string value))
179+
foreach (var propertyToBeMeasured in toBeMeasured)
195180
{
196-
// Lets lazy allocate in case there is tons of telemetry
197-
properties ??= new Dictionary<string, string>(eventProperties);
198-
properties.Remove(propertyToBeMeasured);
199-
if (double.TryParse(value, CultureInfo.InvariantCulture, out double realValue))
181+
if (eventProperties.TryGetValue(propertyToBeMeasured, out string? value))
200182
{
201183
// Lets lazy allocate in case there is tons of telemetry
202-
measurements ??= [];
203-
measurements[propertyToBeMeasured] = realValue;
184+
properties ??= new Dictionary<string, string?>(eventProperties);
185+
properties.Remove(propertyToBeMeasured);
186+
if (double.TryParse(value, CultureInfo.InvariantCulture, out double realValue))
187+
{
188+
// Lets lazy allocate in case there is tons of telemetry
189+
measurements ??= [];
190+
measurements[propertyToBeMeasured] = realValue;
191+
}
204192
}
205193
}
206194
}
@@ -226,6 +214,5 @@ public void Shutdown()
226214
}
227215

228216
public LoggerVerbosity Verbosity { get; set; }
229-
230-
public string Parameters { get; set; }
217+
public string? Parameters { get; set; }
231218
}

‎src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.TargetFrameworkInference.targets‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ Copyright (c) .NET Foundation. All rights reserved.
5454
<TargetFrameworkIdentifier>$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)'))</TargetFrameworkIdentifier>
5555
<TargetFrameworkVersion>v$([MSBuild]::GetTargetFrameworkVersion('$(TargetFramework)', 2))</TargetFrameworkVersion>
5656
</PropertyGroup>
57-
57+
5858
<PropertyGroup>
5959
<_TargetFrameworkVersionWithoutV>$(TargetFrameworkVersion.TrimStart('vV'))</_TargetFrameworkVersionWithoutV>
6060
</PropertyGroup>
@@ -154,8 +154,8 @@ Copyright (c) .NET Foundation. All rights reserved.
154154
<TFTelemetry Include="PublishSelfContained" Value="$(PublishSelfContained)" />
155155
<TFTelemetry Include="PublishReadyToRun" Value="$(PublishReadyToRun)" />
156156
<TFTelemetry Include="PublishReadyToRunComposite" Value="$(PublishReadyToRunComposite)" />
157-
<TFTelemetry Include="PublishProtocol" Value="$(PublishProtocol)" />
158-
<TFTelemetry Include="Configuration" Value="$(Configuration)" />
157+
<TFTelemetry Include="PublishProtocol" Value="$(PublishProtocol)" />
158+
<TFTelemetry Include="Configuration" Value="$(Configuration)" Hash="true" />
159159
</ItemGroup>
160160
<AllowEmptyTelemetry EventName="targetframeworkeval" EventData="@(TFTelemetry)" />
161161
</Target>

‎test/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildANetCoreAppForTelemetry.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Reflection;
5+
using Microsoft.DotNet.Cli.Utils;
56

67
namespace Microsoft.NET.Build.Tests
78
{
@@ -29,7 +30,7 @@ private string CreateTargetFrameworkEvalTelemetryJson(
2930
string publishProtocol = "null",
3031
string configuration = "Debug")
3132
{
32-
return $"{{\"EventName\":\"targetframeworkeval\",\"Properties\":{{\"TargetFrameworkVersion\":\"{targetFrameworkVersion}\",\"RuntimeIdentifier\":\"{runtimeIdentifier}\",\"SelfContained\":\"{selfContained}\",\"UseApphost\":\"{useApphost}\",\"OutputType\":\"{outputType}\",\"UseArtifactsOutput\":\"{useArtifactsOutput}\",\"ArtifactsPathLocationType\":\"{artifactsPathLocationType}\",\"TargetPlatformIdentifier\":\"{targetPlatformIdentifier}\",\"UseMonoRuntime\":\"{useMonoRuntime}\",\"PublishAot\":\"{publishAot}\",\"PublishTrimmed\":\"{publishTrimmed}\",\"PublishSelfContained\":\"{publishSelfContained}\",\"PublishReadyToRun\":\"{publishReadyToRun}\",\"PublishReadyToRunComposite\":\"{publishReadyToRunComposite}\",\"PublishProtocol\":\"{publishProtocol}\",\"Configuration\":\"{configuration}\"}}";
33+
return $"{{\"EventName\":\"targetframeworkeval\",\"Properties\":{{\"TargetFrameworkVersion\":\"{targetFrameworkVersion}\",\"RuntimeIdentifier\":\"{runtimeIdentifier}\",\"SelfContained\":\"{selfContained}\",\"UseApphost\":\"{useApphost}\",\"OutputType\":\"{outputType}\",\"UseArtifactsOutput\":\"{useArtifactsOutput}\",\"ArtifactsPathLocationType\":\"{artifactsPathLocationType}\",\"TargetPlatformIdentifier\":\"{targetPlatformIdentifier}\",\"UseMonoRuntime\":\"{useMonoRuntime}\",\"PublishAot\":\"{publishAot}\",\"PublishTrimmed\":\"{publishTrimmed}\",\"PublishSelfContained\":\"{publishSelfContained}\",\"PublishReadyToRun\":\"{publishReadyToRun}\",\"PublishReadyToRunComposite\":\"{publishReadyToRunComposite}\",\"PublishProtocol\":\"{publishProtocol}\",\"Configuration\":\"{Sha256Hasher.HashWithNormalizedCasing(configuration)}\"}}";
3334
}
3435

3536
[CoreMSBuildOnlyFact]
@@ -82,7 +83,7 @@ public void It_collects_multi_TargetFramework_version_and_other_properties()
8283
result
8384
.StdOut.Should()
8485
.Contain(CreateTargetFrameworkEvalTelemetryJson(
85-
".NETFramework,Version=v4.6",
86+
".NETFramework,Version=v4.6",
8687
targetPlatformIdentifier: "Windows",
8788
publishReadyToRunComposite: "null"))
8889
.And

‎test/dotnet.Tests/CommandTests/MSBuild/FakeTelemetry.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
99
{
1010
public class FakeTelemetry : ITelemetry
1111
{
12-
public bool Enabled { get; set; }
12+
public bool Enabled { get; set; } = true;
1313

1414
public void TrackEvent(string eventName, IDictionary<string, string> properties, IDictionary<string, double> measurements)
1515
{

‎test/dotnet.Tests/CommandTests/MSBuild/GivenMSBuildLogger.cs‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
using Microsoft.Build.Framework;
77
using Microsoft.DotNet.Cli.Commands.MSBuild;
8+
using Microsoft.DotNet.Cli.Utils;
89

910
namespace Microsoft.DotNet.Cli.MSBuild.Tests
1011
{
@@ -44,6 +45,7 @@ public void ItDoesNotMasksExceptionTelemetry()
4445

4546
MSBuildLogger.FormatAndSend(fakeTelemetry, telemetryEventArgs);
4647

48+
fakeTelemetry.LogEntry.Should().NotBeNull();
4749
fakeTelemetry.LogEntry.EventName.Should().Be(MSBuildLogger.SdkTaskBaseCatchExceptionTelemetryEventName);
4850
fakeTelemetry.LogEntry.Properties.Keys.Count.Should().Be(2);
4951
fakeTelemetry.LogEntry.Properties["exceptionType"].Should().Be("System.Exception");
@@ -108,20 +110,13 @@ public void ItCanSendProperties()
108110
{ "RuntimeIdentifier", "null"},
109111
{ "SelfContained", "null"},
110112
{ "UseApphost", "null"},
111-
{ "OutputType", "Library"},
113+
{ "OutputType", "Library"}
112114
}
113115
};
114116

115117
MSBuildLogger.FormatAndSend(fakeTelemetry, telemetryEventArgs);
116118

117-
fakeTelemetry.LogEntry.Properties.Should().BeEquivalentTo(new Dictionary<string, string>
118-
{
119-
{ "TargetFrameworkVersion", "9a871d7066260764d4cb5047e4b10570271d04bd1da275681a4b12bce0b27496"},
120-
{ "RuntimeIdentifier", "fb329000228cc5a24c264c57139de8bf854fc86fc18bf1c04ab61a2b5cb4b921"},
121-
{ "SelfContained", "fb329000228cc5a24c264c57139de8bf854fc86fc18bf1c04ab61a2b5cb4b921"},
122-
{ "UseApphost", "fb329000228cc5a24c264c57139de8bf854fc86fc18bf1c04ab61a2b5cb4b921"},
123-
{ "OutputType", "d77982267d9699c2a57bcab5bb975a1935f6427002f52fd4569762fd72db3a94"},
124-
});
119+
fakeTelemetry.LogEntry.Properties.Should().BeEquivalentTo(telemetryEventArgs.Properties);
125120
}
126121
}
127122
}

0 commit comments

Comments
 (0)