Merge pull request #9582 from drewnoakes/telemetry-values-can-be-null

Telemetry values can be null

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Build/LanguageServiceErrorListProvider.cs

@@ -47,15 +47,10 @@ public void ResumeRefresh()
         {
         }
 
-        public Task<AddMessageResult> AddMessageAsync(TargetGeneratedError error)
+        public async Task<AddMessageResult> AddMessageAsync(TargetGeneratedError error)
         {
             Requires.NotNull(error);
 
-            return AddMessageCoreAsync(error);
-        }
-
-        private async Task<AddMessageResult> AddMessageCoreAsync(TargetGeneratedError error)
-        {
             // We only want to pass compiler, analyzers, etc to the language
             // service, so we skip tasks that do not have a code
             if (!TryExtractErrorListDetails(error.BuildEventArgs, out ErrorListDetails details) || string.IsNullOrEmpty(details.Code))
@@ -121,9 +116,9 @@ await _workspaceWriter.WriteAsync(workspace =>
         /// <summary>
         ///     Attempts to extract the details required by the VS Error List from an MSBuild build event.
         /// </summary>
-        /// <param name="eventArgs">The build event.  May be null.</param>
-        /// <param name="result">The extracted details, or <see langword="null"/> if <paramref name="eventArgs"/> was <see langword="null"/> or of an unrecognized type.</param>
-        internal static bool TryExtractErrorListDetails(BuildEventArgs eventArgs, out ErrorListDetails result)
+        /// <param name="eventArgs">The build event.</param>
+        /// <param name="result">The extracted details, or <see langword="default"/> if <paramref name="eventArgs"/> was <see langword="null"/> or of an unrecognized type.</param>
+        internal static bool TryExtractErrorListDetails(BuildEventArgs? eventArgs, out ErrorListDetails result)
         {
             if (eventArgs is BuildErrorEventArgs errorMessage)
             {

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/TempPE/DesignTimeInputsCompiler.cs

@@ -162,13 +162,13 @@ private Task ProcessCompileQueueAsync(CancellationToken token)
             void LogTelemetry(bool cancelled)
             {
                 compileStopWatch!.Stop();
-                _telemetryService.PostProperties(TelemetryEventName.TempPEProcessQueue, new[]
-                {
-                    ( TelemetryPropertyName.TempPE.CompileCount,        (object)compileCount),
-                    ( TelemetryPropertyName.TempPE.InitialQueueLength,  initialQueueLength),
-                    ( TelemetryPropertyName.TempPE.CompileWasCancelled, cancelled),
-                    ( TelemetryPropertyName.TempPE.CompileDuration,     compileStopWatch.ElapsedMilliseconds)
-                });
+                _telemetryService.PostProperties(TelemetryEventName.TempPEProcessQueue,
+                [
+                    (TelemetryPropertyName.TempPE.CompileCount,        compileCount),
+                    (TelemetryPropertyName.TempPE.InitialQueueLength,  initialQueueLength),
+                    (TelemetryPropertyName.TempPE.CompileWasCancelled, cancelled),
+                    (TelemetryPropertyName.TempPE.CompileDuration,     compileStopWatch.ElapsedMilliseconds)
+                ]);
             }
         }
 
@@ -202,22 +202,24 @@ public async Task<string> BuildDesignTimeOutputAsync(string relativeFileName, st
 
             if (compiled)
             {
-                _telemetryService.PostProperties(TelemetryEventName.TempPECompileOnDemand, new[]
-                {
-                    ( TelemetryPropertyName.TempPE.InitialQueueLength, (object)initialQueueLength)
-                });
+                _telemetryService.PostProperties(TelemetryEventName.TempPECompileOnDemand,
+                [
+                    (TelemetryPropertyName.TempPE.InitialQueueLength, initialQueueLength)
+                ]);
             }
 
-            return $@"<root>
-  <Application private_binpath = ""{Path.GetDirectoryName(outputFileName)}""/>
-  <Assembly
-    codebase = ""{Path.GetFileName(outputFileName)}""
-    name = ""{relativeFileName}""
-    version = ""0.0.0.0""
-    snapshot_id = ""1""
-    replaceable = ""True""
-  />
-</root>";
+            return $"""
+                <root>
+                  <Application private_binpath = "{Path.GetDirectoryName(outputFileName)}"/>
+                  <Assembly
+                    codebase = "{Path.GetFileName(outputFileName)}"
+                    name = "{relativeFileName}"
+                    version = "0.0.0.0"
+                    snapshot_id = "1"
+                    replaceable = "True"
+                  />
+                </root>
+                """;
         }
 
         private async Task<bool> CompileDesignTimeInputAsync(IWorkspaceProjectContext context, string designTimeInput, string outputFileName, ImmutableHashSet<string> sharedInputs, bool ignoreFileWriteTime, CancellationToken token = default)

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/UpToDate/BuildUpToDateCheck.Log.cs

@@ -199,9 +199,9 @@ public bool Fail(string reason, string resourceName, params object[] values)
                     logLevel: Level);
 
                 // Send telemetry.
-                _telemetryService?.PostProperties(TelemetryEventName.UpToDateCheckFail, new[]
-                {
-                    (TelemetryPropertyName.UpToDateCheck.FailReason, (object)reason),
+                _telemetryService?.PostProperties(TelemetryEventName.UpToDateCheckFail,
+                [
+                    (TelemetryPropertyName.UpToDateCheck.FailReason, reason),
                     (TelemetryPropertyName.UpToDateCheck.DurationMillis, _stopwatch.Elapsed.TotalMilliseconds),
                     (TelemetryPropertyName.UpToDateCheck.WaitDurationMillis, _waitTime.TotalMilliseconds),
                     (TelemetryPropertyName.UpToDateCheck.FileCount, _timestampCache.Count),
@@ -211,7 +211,7 @@ public bool Fail(string reason, string resourceName, params object[] values)
                     (TelemetryPropertyName.UpToDateCheck.CheckNumber, _checkNumber),
                     (TelemetryPropertyName.UpToDateCheck.IgnoreKinds, _ignoreKinds ?? ""),
                     (TelemetryPropertyName.UpToDateCheck.AccelerationResult, FileSystemOperations.AccelerationResult)
-                });
+                ]);
 
                 // Remember the failure reason and description for use in IncrementalBuildFailureDetector.
                 // First, ensure times are converted to local time (in case logging is not enabled).
@@ -245,9 +245,9 @@ public void UpToDate(int copyCount)
                     logLevel: Level);
 
                 // Send telemetry.
-                _telemetryService?.PostProperties(TelemetryEventName.UpToDateCheckSuccess, new[]
-                {
-                    (TelemetryPropertyName.UpToDateCheck.DurationMillis, (object)_stopwatch.Elapsed.TotalMilliseconds),
+                _telemetryService?.PostProperties(TelemetryEventName.UpToDateCheckSuccess,
+                [
+                    (TelemetryPropertyName.UpToDateCheck.DurationMillis, _stopwatch.Elapsed.TotalMilliseconds),
                     (TelemetryPropertyName.UpToDateCheck.WaitDurationMillis, _waitTime.TotalMilliseconds),
                     (TelemetryPropertyName.UpToDateCheck.FileCount, _timestampCache.Count),
                     (TelemetryPropertyName.UpToDateCheck.ConfigurationCount, _upToDateCheckConfiguredInput.ImplicitInputs.Length),
@@ -257,7 +257,7 @@ public void UpToDate(int copyCount)
                     (TelemetryPropertyName.UpToDateCheck.IgnoreKinds, _ignoreKinds ?? ""),
                     (TelemetryPropertyName.UpToDateCheck.AccelerationResult, FileSystemOperations.AccelerationResult),
                     (TelemetryPropertyName.UpToDateCheck.AcceleratedCopyCount, copyCount)
-                });
+                ]);
 
                 Info(nameof(VSResources.FUTD_UpToDate));
 

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/Telemetry/DesignTimeTelemetryLogger.cs

@@ -67,11 +67,11 @@ public void Shutdown()
 
             string targetResults = builder.ToStringAndFree();
 
-            _telemetryService.PostProperties(TelemetryEventName.DesignTimeBuildComplete, new[]
-            {
-                (TelemetryPropertyName.DesignTimeBuildComplete.Succeeded, (object)_succeeded),
+            _telemetryService.PostProperties(TelemetryEventName.DesignTimeBuildComplete,
+            [
+                (TelemetryPropertyName.DesignTimeBuildComplete.Succeeded, _succeeded),
                 (TelemetryPropertyName.DesignTimeBuildComplete.Targets, targetResults)
-            });
+            ]);
         }
     }
 }

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/PackageRestore/PackageRestoreCycleDetector.cs

@@ -120,12 +120,12 @@ async Task OnCycleDetectedAsync()
             _nuGetRestoreCyclesDetected++;
 
             // Send telemetry.
-            _telemetryService.PostProperties(TelemetryEventName.NuGetRestoreCycleDetected, new[]
-            {
-                (TelemetryPropertyName.NuGetRestoreCycleDetected.RestoreDurationMillis, (object)_stopwatch.Elapsed.TotalMilliseconds),
+            _telemetryService.PostProperties(TelemetryEventName.NuGetRestoreCycleDetected,
+            [
+                (TelemetryPropertyName.NuGetRestoreCycleDetected.RestoreDurationMillis, _stopwatch.Elapsed.TotalMilliseconds),
                 (TelemetryPropertyName.NuGetRestoreCycleDetected.RestoreSuccesses, _nuGetRestoreSuccesses),
                 (TelemetryPropertyName.NuGetRestoreCycleDetected.RestoreCyclesDetected, _nuGetRestoreCyclesDetected)
-            });
+            ]);
 
             // Notify the user.
             await _userNotificationService.ShowErrorAsync(

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Telemetry/SdkVersionReporter.cs

@@ -46,11 +46,10 @@ public Task LoadAsync()
                 {
                     _telemetryService.PostProperties(
                         TelemetryEventName.SDKVersion,
-                        new[]
-                        {
-                            (TelemetryPropertyName.SDKVersion.Project, (object)projectGuid.ToString()),
+                        [
+                            (TelemetryPropertyName.SDKVersion.Project, projectGuid.ToString()),
                             (TelemetryPropertyName.SDKVersion.NETCoreSDKVersion, version)
-                        });
+                        ]);
                 }
             },
             unconfiguredProject: _projectVsServices.Project);

src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryOperation.cs

@@ -27,7 +27,7 @@ internal interface ITelemetryOperation : IDisposable
     /// <exception cref="ArgumentException">
     ///     <paramref name="properties"/> is contains no elements.
     /// </exception>
-    void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties);
+    void SetProperties(IEnumerable<(string propertyName, object? propertyValue)> properties);
 
     /// <summary>
     ///     Ends the operation and reports the result.

src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs

@@ -40,10 +40,6 @@ internal interface ITelemetryService
     ///         -or-
     ///     </para>
     ///     <paramref name="propertyName"/> is <see langword="null"/>.
-    ///     <para>
-    ///         -or-
-    ///     </para>
-    ///     <paramref name="propertyValue"/> is <see langword="null"/>.
     /// </exception>
     /// <exception cref="ArgumentException">
     ///     <paramref name="eventName"/> is an empty string ("").
@@ -52,7 +48,7 @@ internal interface ITelemetryService
     ///     </para>
     ///     <paramref name="propertyName"/> is an empty string ("").
     /// </exception>
-    void PostProperty(string eventName, string propertyName, object propertyValue);
+    void PostProperty(string eventName, string propertyName, object? propertyValue);
 
     /// <summary>
     ///     Posts an event with the specified event name and properties with the
@@ -78,7 +74,7 @@ internal interface ITelemetryService
     ///     </para>
     ///     <paramref name="properties"/> is contains no elements.
     /// </exception>
-    void PostProperties(string eventName, IEnumerable<(string propertyName, object propertyValue)> properties);
+    void PostProperties(string eventName, IEnumerable<(string propertyName, object? propertyValue)> properties);
 
     /// <summary>
     ///     Begins an operation with a recorded duration. Consumers must call <see cref="ITelemetryOperation.End(TelemetryResult)"/>

src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs

@@ -20,19 +20,18 @@ public void PostEvent(string eventName)
         PostTelemetryEvent(new TelemetryEvent(eventName));
     }
 
-    public void PostProperty(string eventName, string propertyName, object propertyValue)
+    public void PostProperty(string eventName, string propertyName, object? propertyValue)
     {
         Requires.NotNullOrEmpty(eventName);
         Requires.NotNullOrEmpty(propertyName);
-        Requires.NotNull(propertyValue);
 
         TelemetryEvent telemetryEvent = new(eventName);
         telemetryEvent.Properties.Add(propertyName, propertyValue);
 
         PostTelemetryEvent(telemetryEvent);
     }
 
-    public void PostProperties(string eventName, IEnumerable<(string propertyName, object propertyValue)> properties)
+    public void PostProperties(string eventName, IEnumerable<(string propertyName, object? propertyValue)> properties)
     {
         Requires.NotNullOrEmpty(eventName);
         Requires.NotNullOrEmpty(properties);
@@ -43,9 +42,9 @@ public void PostProperties(string eventName, IEnumerable<(string propertyName, o
         PostTelemetryEvent(telemetryEvent);
     }
 
-    private static void AddPropertiesToEvent(IEnumerable<(string propertyName, object propertyValue)> properties, TelemetryEvent telemetryEvent)
+    private static void AddPropertiesToEvent(IEnumerable<(string propertyName, object? propertyValue)> properties, TelemetryEvent telemetryEvent)
     {
-        foreach ((string propertyName, object propertyValue) in properties)
+        foreach ((string propertyName, object? propertyValue) in properties)
         {
             if (propertyValue is ComplexPropertyValue complexProperty)
             {
@@ -63,7 +62,7 @@ private void PostTelemetryEvent(TelemetryEvent telemetryEvent)
 #if DEBUG
         Assumes.True(telemetryEvent.Name.StartsWith(EventNamePrefix, StringComparisons.TelemetryEventNames));
 
-        foreach (string propertyName in telemetryEvent.Properties.Keys)
+        foreach ((string propertyName, _) in telemetryEvent.Properties)
         {
             Assumes.True(propertyName.StartsWith(PropertyNamePrefix, StringComparisons.TelemetryEventNames));
         }
@@ -80,11 +79,11 @@ protected virtual void PostEventToSession(TelemetryEvent telemetryEvent)
     public ITelemetryOperation BeginOperation(string eventName)
     {
         Requires.NotNullOrEmpty(eventName);
-        
+
 #if DEBUG
         Assumes.True(eventName.StartsWith(EventNamePrefix, StringComparisons.TelemetryEventNames));
 #endif
-        return new TelemetryOperation(TelemetryService.DefaultSession.StartOperation(eventName));            
+        return new TelemetryOperation(TelemetryService.DefaultSession.StartOperation(eventName));
     }
 
     public string HashValue(string value)
@@ -96,8 +95,15 @@ public string HashValue(string value)
         }
 
         byte[] inputBytes = Encoding.UTF8.GetBytes(value);
+
+#if NET8_0_OR_GREATER
+        byte[] hash = SHA256.HashData(inputBytes);
+#else
         using var cryptoServiceProvider = SHA256.Create();
-        return BitConverter.ToString(cryptoServiceProvider.ComputeHash(inputBytes));
+        byte[] hash = cryptoServiceProvider.ComputeHash(inputBytes);
+#endif
+
+        return BitConverter.ToString(hash);
     }
 
     private class TelemetryOperation : ITelemetryOperation
@@ -125,7 +131,7 @@ public void End(TelemetryResult result)
             _scope.End(result);
         }
 
-        public void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties)
+        public void SetProperties(IEnumerable<(string propertyName, object? propertyValue)> properties)
         {
             Requires.NotNullOrEmpty(properties);
 

tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Mocks/ITelemetryServiceFactory.cs

@@ -35,7 +35,7 @@ public static ITelemetryService Create(TelemetryParameters callParameters)
                     };
                 });
 
-            telemetryService.Setup(t => t.PostProperties(It.IsAny<string>(), It.IsAny<IEnumerable<(string propertyName, object propertyValue)>>()))
+            telemetryService.Setup(t => t.PostProperties(It.IsAny<string>(), It.IsAny<IEnumerable<(string propertyName, object? propertyValue)>>()))
                 .Callback((string e, IEnumerable<(string propertyName, object propertyValue)> p) =>
                 {
                     callParameters.EventName = e;
@@ -73,7 +73,7 @@ public static ITelemetryService Create(Action<TelemetryParameters> onTelemetryLo
                     onTelemetryLogged(callParameters);
                 });
 
-            telemetryService.Setup(t => t.PostProperties(It.IsAny<string>(), It.IsAny<IEnumerable<(string propertyName, object propertyValue)>>()))
+            telemetryService.Setup(t => t.PostProperties(It.IsAny<string>(), It.IsAny<IEnumerable<(string propertyName, object? propertyValue)>>()))
                 .Callback((string e, IEnumerable<(string propertyName, object propertyValue)> p) =>
                 {
                     var callParameters = new TelemetryParameters

tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Telemetry/ManagedTelemetryServiceTests.cs

@@ -73,14 +73,11 @@ public void PostProperty_EmptyAsPropertyName_ThrowArgument()
         }
 
         [Fact]
-        public void PostProperty_NullAsPropertyValue_ThrowArgumentNull()
+        public void PostProperty_NullAsPropertyValue()
         {
             var service = CreateInstance();
 
-            Assert.Throws<ArgumentNullException>("propertyValue", () =>
-            {
-                service.PostProperty("event1", "propName", null!);
-            });
+            service.PostProperty("vs/projectsystem/managed/test", "vs.projectsystem.managed.test", null);
         }
 
         [Fact]
@@ -90,7 +87,7 @@ public void PostProperties_NullAsEventName_ThrowArgumentNull()
 
             Assert.Throws<ArgumentNullException>("eventName", () =>
             {
-                service.PostProperties(null!, new[] { ("propertyName", (object)"propertyValue") });
+                service.PostProperties(null!, [("propertyName", "propertyValue")]);
             });
         }
 
@@ -101,7 +98,7 @@ public void PostProperties_EmptyAsEventName_ThrowArgument()
 
             Assert.Throws<ArgumentException>("eventName", () =>
             {
-                service.PostProperties(string.Empty, new[] { ("propertyName", (object)"propertyValue") });
+                service.PostProperties(string.Empty, [("propertyName", "propertyValue")]);
             });
         }
 
@@ -123,7 +120,7 @@ public void PostProperties_EmptyProperties_ThrowArgument()
 
             Assert.Throws<ArgumentException>("properties", () =>
             {
-                service.PostProperties("event1", Enumerable.Empty<(string propertyName, object propertyValue)>());
+                service.PostProperties("event1", []);
             });
         }
 
@@ -136,7 +133,7 @@ public void PostEvent_SendsTelemetryEvent()
             service.PostEvent(TelemetryEventName.UpToDateCheckSuccess);
 
             Assert.NotNull(result);
-            Assert.Equal(TelemetryEventName.UpToDateCheckSuccess, result!.Name);
+            Assert.Equal(TelemetryEventName.UpToDateCheckSuccess, result.Name);
         }
 
         [Fact]
@@ -158,11 +155,11 @@ public void PostProperties_SendsTelemetryEventWithProperties()
             TelemetryEvent? result = null;
             var service = CreateInstance((e) => { result = e; });
 
-            service.PostProperties(TelemetryEventName.DesignTimeBuildComplete, new[]
-            {
-                (TelemetryPropertyName.DesignTimeBuildComplete.Succeeded, (object)true),
+            service.PostProperties(TelemetryEventName.DesignTimeBuildComplete,
+            [
+                (TelemetryPropertyName.DesignTimeBuildComplete.Succeeded, true),
                 (TelemetryPropertyName.DesignTimeBuildComplete.Targets, "Compile")
-            });
+            ]);
 
             Assert.NotNull(result);
             Assert.Equal(TelemetryEventName.DesignTimeBuildComplete, result.Name);