From ae2c5353fcf910b84aaf1252cd4d32b3cdbca8e8 Mon Sep 17 00:00:00 2001 From: Tom Meschter Date: Tue, 14 Mar 2023 13:23:27 -0700 Subject: [PATCH 1/6] Update ITelemetryService Update ITelemetryService to support operations with reported start and end events. --- .../Telemetry/ITelemetryService.cs | 55 ++++++++++++++++++ .../Telemetry/ManagedTelemetryService.cs | 58 ++++++++++++++++++- .../Telemetry/ManagedTelemetryServiceTests.cs | 22 +++++++ 3 files changed, 133 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs index ff2e9505852..5b6e5c4d9b7 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs @@ -4,6 +4,42 @@ namespace Microsoft.VisualStudio.Telemetry { + /// + /// + /// Represents an operation with associated start and end events. An + /// is started by calling . + /// + /// + /// This type implements to ensure that operations are completed and reported, + /// but consumers must also call to report the success or failure of + /// the operation. + /// + /// + internal interface ITelemetryOperation : IDisposable + { + /// + /// Associates the given properties with the "end" event of the operation. + /// + /// + /// An of property names and values. + /// + /// + /// is . + /// + /// + /// is contains no elements. + /// + void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties); + + /// + /// Ends the operation and reports the result. + /// + /// + /// A value indicating the result of the operation. + /// + void End(TelemetryResult result); + } + [ProjectSystemContract(ProjectSystemContractScope.Global, ProjectSystemContractProvider.Private, Cardinality = ImportCardinality.ExactlyOne)] internal interface ITelemetryService { @@ -80,6 +116,25 @@ internal interface ITelemetryService /// void PostProperties(string eventName, IEnumerable<(string propertyName, object propertyValue)> properties); + /// + /// Begins an operation with a recorded duration. Consumers must call + /// on the returned to signal the end of the operation and post the + /// telemetry events. + /// + /// + /// The name of the event to associate with this operation. + /// + /// + /// is . + /// + /// + /// is an empty string (""). + /// + /// + /// An representing the operation. + /// + ITelemetryOperation BeginOperation(string eventName); + /// /// Hashes personally identifiable information for telemetry consumption. /// diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs index 7a41c1d9630..eeae9b587bd 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs @@ -33,6 +33,13 @@ public void PostProperties(string eventName, IEnumerable<(string propertyName, o Requires.NotNullOrEmpty(properties); var telemetryEvent = new TelemetryEvent(eventName); + AddPropertiesToEvent(properties, telemetryEvent); + + PostTelemetryEvent(telemetryEvent); + } + + private static void AddPropertiesToEvent(IEnumerable<(string propertyName, object propertyValue)> properties, TelemetryEvent telemetryEvent) + { foreach ((string propertyName, object propertyValue) in properties) { if (propertyValue is ComplexPropertyValue complexProperty) @@ -44,8 +51,6 @@ public void PostProperties(string eventName, IEnumerable<(string propertyName, o telemetryEvent.Properties.Add(propertyName, propertyValue); } } - - PostTelemetryEvent(telemetryEvent); } private void PostTelemetryEvent(TelemetryEvent telemetryEvent) @@ -67,6 +72,16 @@ protected virtual void PostEventToSession(TelemetryEvent telemetryEvent) TelemetryService.DefaultSession.PostEvent(telemetryEvent); } + public ITelemetryOperation BeginOperation(string eventName) + { + Requires.NotNullOrEmpty(eventName); + +#if DEBUG + Assumes.True(eventName.StartsWith("vs/projectsystem/managed/", StringComparisons.TelemetryEventNames)); +#endif + return new TelemetryOperation(TelemetryService.DefaultSession.StartOperation(eventName)); + } + public string HashValue(string value) { // Don't hash PII for internal users since we don't need to. @@ -79,5 +94,44 @@ public string HashValue(string value) using var cryptoServiceProvider = new SHA256CryptoServiceProvider(); return BitConverter.ToString(cryptoServiceProvider.ComputeHash(inputBytes)); } + + private class TelemetryOperation : ITelemetryOperation + { + private readonly TelemetryScope _scope; + + public TelemetryOperation(TelemetryScope scope) + { + _scope = scope; + } + + public void Dispose() + { +#if DEBUG + Assumes.True(_scope.IsEnd, $"Failed to call '{nameof(ITelemetryOperation.End)}' on {nameof(ITelemetryOperation)} instance."); +#endif + + _scope.End(TelemetryResult.None); + } + + public void End(TelemetryResult result) + { + _scope.End(result); + } + + public void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties) + { + Requires.NotNullOrEmpty(properties); + +#if DEBUG + foreach ((string propertyName, _) in properties) + { + Assumes.True(propertyName.StartsWith("vs.projectsystem.managed.", StringComparisons.TelemetryEventNames)); + } +#endif + + AddPropertiesToEvent(properties, _scope.EndEvent); + } + } } } + diff --git a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Telemetry/ManagedTelemetryServiceTests.cs b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Telemetry/ManagedTelemetryServiceTests.cs index 25255cee1e8..ed49d337a16 100644 --- a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Telemetry/ManagedTelemetryServiceTests.cs +++ b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Telemetry/ManagedTelemetryServiceTests.cs @@ -170,6 +170,28 @@ public void PostProperties_SendsTelemetryEventWithProperties() Assert.Contains(new KeyValuePair(TelemetryPropertyName.DesignTimeBuildComplete.Targets, "Compile"), result.Properties); } + [Fact] + public void BeginOperation_NullAsEventName_ThrowsArgumentNull() + { + var service = CreateInstance(); + + Assert.Throws("eventName", () => + { + _ = service.BeginOperation(null!); + }); + } + + [Fact] + public void BeginOperation_EmptyAsEventName_ThrowsArgument() + { + var service = CreateInstance(); + + Assert.Throws("eventName", () => + { + _ = service.BeginOperation(string.Empty); + }); + } + private static ManagedTelemetryService CreateInstance(Action? action = null) { if (action is null) From a68921479b142ae259e40013cefaaf4119708a83 Mon Sep 17 00:00:00 2001 From: Tom Meschter Date: Tue, 14 Mar 2023 13:24:00 -0700 Subject: [PATCH 2/6] Move ITelemetryOperation Move ITelemetryOperation out to its own file. --- .../Telemetry/ITelemetryOperation.cs | 40 +++++++++++++++++++ .../Telemetry/ITelemetryService.cs | 36 ----------------- 2 files changed, 40 insertions(+), 36 deletions(-) create mode 100644 src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryOperation.cs diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryOperation.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryOperation.cs new file mode 100644 index 00000000000..29a94722f11 --- /dev/null +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryOperation.cs @@ -0,0 +1,40 @@ +// Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. + +namespace Microsoft.VisualStudio.Telemetry +{ + /// + /// + /// Represents an operation with associated start and end events. An + /// is started by calling . + /// + /// + /// This type implements to ensure that operations are completed and reported, + /// but consumers must also call to report the success or failure of + /// the operation. + /// + /// + internal interface ITelemetryOperation : IDisposable + { + /// + /// Associates the given properties with the "end" event of the operation. + /// + /// + /// An of property names and values. + /// + /// + /// is . + /// + /// + /// is contains no elements. + /// + void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties); + + /// + /// Ends the operation and reports the result. + /// + /// + /// A value indicating the result of the operation. + /// + void End(TelemetryResult result); + } +} diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs index 5b6e5c4d9b7..0efb400b0d8 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs @@ -4,42 +4,6 @@ namespace Microsoft.VisualStudio.Telemetry { - /// - /// - /// Represents an operation with associated start and end events. An - /// is started by calling . - /// - /// - /// This type implements to ensure that operations are completed and reported, - /// but consumers must also call to report the success or failure of - /// the operation. - /// - /// - internal interface ITelemetryOperation : IDisposable - { - /// - /// Associates the given properties with the "end" event of the operation. - /// - /// - /// An of property names and values. - /// - /// - /// is . - /// - /// - /// is contains no elements. - /// - void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties); - - /// - /// Ends the operation and reports the result. - /// - /// - /// A value indicating the result of the operation. - /// - void End(TelemetryResult result); - } - [ProjectSystemContract(ProjectSystemContractScope.Global, ProjectSystemContractProvider.Private, Cardinality = ImportCardinality.ExactlyOne)] internal interface ITelemetryService { From f699a3257d08462db6370d544082e9cab788b14a Mon Sep 17 00:00:00 2001 From: Tom Meschter Date: Tue, 14 Mar 2023 13:24:40 -0700 Subject: [PATCH 3/6] Switch to file-scope namespaces --- .../Telemetry/ITelemetryOperation.cs | 65 +++--- .../Telemetry/ITelemetryService.cs | 197 +++++++++--------- .../Telemetry/ManagedTelemetryService.cs | 179 ++++++++-------- 3 files changed, 219 insertions(+), 222 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryOperation.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryOperation.cs index 29a94722f11..6963cc12024 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryOperation.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryOperation.cs @@ -1,40 +1,39 @@ // Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. -namespace Microsoft.VisualStudio.Telemetry +namespace Microsoft.VisualStudio.Telemetry; + +/// +/// +/// Represents an operation with associated start and end events. An +/// is started by calling . +/// +/// +/// This type implements to ensure that operations are completed and reported, +/// but consumers must also call to report the success or failure of +/// the operation. +/// +/// +internal interface ITelemetryOperation : IDisposable { /// - /// - /// Represents an operation with associated start and end events. An - /// is started by calling . - /// - /// - /// This type implements to ensure that operations are completed and reported, - /// but consumers must also call to report the success or failure of - /// the operation. - /// + /// Associates the given properties with the "end" event of the operation. /// - internal interface ITelemetryOperation : IDisposable - { - /// - /// Associates the given properties with the "end" event of the operation. - /// - /// - /// An of property names and values. - /// - /// - /// is . - /// - /// - /// is contains no elements. - /// - void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties); + /// + /// An of property names and values. + /// + /// + /// is . + /// + /// + /// is contains no elements. + /// + void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties); - /// - /// Ends the operation and reports the result. - /// - /// - /// A value indicating the result of the operation. - /// - void End(TelemetryResult result); - } + /// + /// Ends the operation and reports the result. + /// + /// + /// A value indicating the result of the operation. + /// + void End(TelemetryResult result); } diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs index 0efb400b0d8..0abd49b0214 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ITelemetryService.cs @@ -2,108 +2,107 @@ using Microsoft.VisualStudio.ProjectSystem; -namespace Microsoft.VisualStudio.Telemetry +namespace Microsoft.VisualStudio.Telemetry; + +[ProjectSystemContract(ProjectSystemContractScope.Global, ProjectSystemContractProvider.Private, Cardinality = ImportCardinality.ExactlyOne)] +internal interface ITelemetryService { - [ProjectSystemContract(ProjectSystemContractScope.Global, ProjectSystemContractProvider.Private, Cardinality = ImportCardinality.ExactlyOne)] - internal interface ITelemetryService - { - /// - /// Posts an event with the specified event name. - /// - /// - /// The name of the event. - /// - /// - /// is . - /// - /// - /// is an empty string (""). - /// - void PostEvent(string eventName); + /// + /// Posts an event with the specified event name. + /// + /// + /// The name of the event. + /// + /// + /// is . + /// + /// + /// is an empty string (""). + /// + void PostEvent(string eventName); - /// - /// Posts an event with the specified event name and property with the - /// specified name and value. - /// - /// - /// The name of the event. - /// - /// - /// The name of the property. - /// - /// - /// The value of the property. - /// - /// - /// is . - /// - /// -or- - /// - /// is . - /// - /// -or- - /// - /// is . - /// - /// - /// is an empty string (""). - /// - /// -or - /// - /// is an empty string (""). - /// - void PostProperty(string eventName, string propertyName, object propertyValue); + /// + /// Posts an event with the specified event name and property with the + /// specified name and value. + /// + /// + /// The name of the event. + /// + /// + /// The name of the property. + /// + /// + /// The value of the property. + /// + /// + /// is . + /// + /// -or- + /// + /// is . + /// + /// -or- + /// + /// is . + /// + /// + /// is an empty string (""). + /// + /// -or + /// + /// is an empty string (""). + /// + void PostProperty(string eventName, string propertyName, object propertyValue); - /// - /// Posts an event with the specified event name and properties with the - /// specified names and values. - /// - /// - /// The name of the event. - /// - /// - /// An of property names and values. - /// - /// - /// is . - /// - /// -or- - /// - /// is . - /// - /// - /// is an empty string (""). - /// - /// -or- - /// - /// is contains no elements. - /// - void PostProperties(string eventName, IEnumerable<(string propertyName, object propertyValue)> properties); + /// + /// Posts an event with the specified event name and properties with the + /// specified names and values. + /// + /// + /// The name of the event. + /// + /// + /// An of property names and values. + /// + /// + /// is . + /// + /// -or- + /// + /// is . + /// + /// + /// is an empty string (""). + /// + /// -or- + /// + /// is contains no elements. + /// + void PostProperties(string eventName, IEnumerable<(string propertyName, object propertyValue)> properties); - /// - /// Begins an operation with a recorded duration. Consumers must call - /// on the returned to signal the end of the operation and post the - /// telemetry events. - /// - /// - /// The name of the event to associate with this operation. - /// - /// - /// is . - /// - /// - /// is an empty string (""). - /// - /// - /// An representing the operation. - /// - ITelemetryOperation BeginOperation(string eventName); + /// + /// Begins an operation with a recorded duration. Consumers must call + /// on the returned to signal the end of the operation and post the + /// telemetry events. + /// + /// + /// The name of the event to associate with this operation. + /// + /// + /// is . + /// + /// + /// is an empty string (""). + /// + /// + /// An representing the operation. + /// + ITelemetryOperation BeginOperation(string eventName); - /// - /// Hashes personally identifiable information for telemetry consumption. - /// - /// Value to hashed. - /// Hashed value. - string HashValue(string value); - } + /// + /// Hashes personally identifiable information for telemetry consumption. + /// + /// Value to hashed. + /// Hashed value. + string HashValue(string value); } diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs index eeae9b587bd..76c81cd5cd1 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs @@ -3,134 +3,133 @@ using System.Security.Cryptography; using System.Text; -namespace Microsoft.VisualStudio.Telemetry +namespace Microsoft.VisualStudio.Telemetry; + +[Export(typeof(ITelemetryService))] +internal class ManagedTelemetryService : ITelemetryService { - [Export(typeof(ITelemetryService))] - internal class ManagedTelemetryService : ITelemetryService + public void PostEvent(string eventName) { - public void PostEvent(string eventName) - { - Requires.NotNullOrEmpty(eventName); + Requires.NotNullOrEmpty(eventName); - PostTelemetryEvent(new TelemetryEvent(eventName)); - } + PostTelemetryEvent(new TelemetryEvent(eventName)); + } - public void PostProperty(string eventName, string propertyName, object propertyValue) - { - Requires.NotNullOrEmpty(eventName); - Requires.NotNullOrEmpty(propertyName); - Requires.NotNull(propertyValue); + public void PostProperty(string eventName, string propertyName, object propertyValue) + { + Requires.NotNullOrEmpty(eventName); + Requires.NotNullOrEmpty(propertyName); + Requires.NotNull(propertyValue); - var telemetryEvent = new TelemetryEvent(eventName); - telemetryEvent.Properties.Add(propertyName, propertyValue); + var telemetryEvent = new TelemetryEvent(eventName); + telemetryEvent.Properties.Add(propertyName, propertyValue); - PostTelemetryEvent(telemetryEvent); - } + PostTelemetryEvent(telemetryEvent); + } - public void PostProperties(string eventName, IEnumerable<(string propertyName, object propertyValue)> properties) - { - Requires.NotNullOrEmpty(eventName); - Requires.NotNullOrEmpty(properties); + public void PostProperties(string eventName, IEnumerable<(string propertyName, object propertyValue)> properties) + { + Requires.NotNullOrEmpty(eventName); + Requires.NotNullOrEmpty(properties); - var telemetryEvent = new TelemetryEvent(eventName); - AddPropertiesToEvent(properties, telemetryEvent); + var telemetryEvent = new TelemetryEvent(eventName); + AddPropertiesToEvent(properties, telemetryEvent); - PostTelemetryEvent(telemetryEvent); - } + 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) + { + telemetryEvent.Properties.Add(propertyName, new TelemetryComplexProperty(complexProperty.Data)); + } + else { - if (propertyValue is ComplexPropertyValue complexProperty) - { - telemetryEvent.Properties.Add(propertyName, new TelemetryComplexProperty(complexProperty.Data)); - } - else - { - telemetryEvent.Properties.Add(propertyName, propertyValue); - } + telemetryEvent.Properties.Add(propertyName, propertyValue); } } + } - private void PostTelemetryEvent(TelemetryEvent telemetryEvent) - { + private void PostTelemetryEvent(TelemetryEvent telemetryEvent) + { #if DEBUG - Assumes.True(telemetryEvent.Name.StartsWith("vs/projectsystem/managed/", StringComparisons.TelemetryEventNames)); + Assumes.True(telemetryEvent.Name.StartsWith("vs/projectsystem/managed/", StringComparisons.TelemetryEventNames)); - foreach (string propertyName in telemetryEvent.Properties.Keys) - { - Assumes.True(propertyName.StartsWith("vs.projectsystem.managed.", StringComparisons.TelemetryEventNames)); - } + foreach (string propertyName in telemetryEvent.Properties.Keys) + { + Assumes.True(propertyName.StartsWith("vs.projectsystem.managed.", StringComparisons.TelemetryEventNames)); + } #endif - PostEventToSession(telemetryEvent); + PostEventToSession(telemetryEvent); + } + + protected virtual void PostEventToSession(TelemetryEvent telemetryEvent) + { + TelemetryService.DefaultSession.PostEvent(telemetryEvent); + } + + public ITelemetryOperation BeginOperation(string eventName) + { + Requires.NotNullOrEmpty(eventName); + +#if DEBUG + Assumes.True(eventName.StartsWith("vs/projectsystem/managed/", StringComparisons.TelemetryEventNames)); +#endif + return new TelemetryOperation(TelemetryService.DefaultSession.StartOperation(eventName)); + } + + public string HashValue(string value) + { + // Don't hash PII for internal users since we don't need to. + if (TelemetryService.DefaultSession.IsUserMicrosoftInternal) + { + return value; } - protected virtual void PostEventToSession(TelemetryEvent telemetryEvent) + byte[] inputBytes = Encoding.UTF8.GetBytes(value); + using var cryptoServiceProvider = new SHA256CryptoServiceProvider(); + return BitConverter.ToString(cryptoServiceProvider.ComputeHash(inputBytes)); + } + + private class TelemetryOperation : ITelemetryOperation + { + private readonly TelemetryScope _scope; + + public TelemetryOperation(TelemetryScope scope) { - TelemetryService.DefaultSession.PostEvent(telemetryEvent); + _scope = scope; } - public ITelemetryOperation BeginOperation(string eventName) + public void Dispose() { - Requires.NotNullOrEmpty(eventName); - #if DEBUG - Assumes.True(eventName.StartsWith("vs/projectsystem/managed/", StringComparisons.TelemetryEventNames)); + Assumes.True(_scope.IsEnd, $"Failed to call '{nameof(ITelemetryOperation.End)}' on {nameof(ITelemetryOperation)} instance."); #endif - return new TelemetryOperation(TelemetryService.DefaultSession.StartOperation(eventName)); + + _scope.End(TelemetryResult.None); } - public string HashValue(string value) + public void End(TelemetryResult result) { - // Don't hash PII for internal users since we don't need to. - if (TelemetryService.DefaultSession.IsUserMicrosoftInternal) - { - return value; - } - - byte[] inputBytes = Encoding.UTF8.GetBytes(value); - using var cryptoServiceProvider = new SHA256CryptoServiceProvider(); - return BitConverter.ToString(cryptoServiceProvider.ComputeHash(inputBytes)); + _scope.End(result); } - private class TelemetryOperation : ITelemetryOperation + public void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties) { - private readonly TelemetryScope _scope; - - public TelemetryOperation(TelemetryScope scope) - { - _scope = scope; - } - - public void Dispose() - { + Requires.NotNullOrEmpty(properties); + #if DEBUG - Assumes.True(_scope.IsEnd, $"Failed to call '{nameof(ITelemetryOperation.End)}' on {nameof(ITelemetryOperation)} instance."); -#endif - - _scope.End(TelemetryResult.None); - } - - public void End(TelemetryResult result) + foreach ((string propertyName, _) in properties) { - _scope.End(result); + Assumes.True(propertyName.StartsWith("vs.projectsystem.managed.", StringComparisons.TelemetryEventNames)); } - - public void SetProperties(IEnumerable<(string propertyName, object propertyValue)> properties) - { - Requires.NotNullOrEmpty(properties); - -#if DEBUG - foreach ((string propertyName, _) in properties) - { - Assumes.True(propertyName.StartsWith("vs.projectsystem.managed.", StringComparisons.TelemetryEventNames)); - } #endif - AddPropertiesToEvent(properties, _scope.EndEvent); - } + AddPropertiesToEvent(properties, _scope.EndEvent); } } } From 85c52eb7e741ca615f0803c1b018cbb4453ba0bd Mon Sep 17 00:00:00 2001 From: Tom Meschter Date: Tue, 14 Mar 2023 13:41:17 -0700 Subject: [PATCH 4/6] Prefer explicit types Prefer explicit types and target-type "new" over "var". --- .../Telemetry/ManagedTelemetryService.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs index 76c81cd5cd1..300a3f0bc68 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs @@ -21,7 +21,7 @@ public void PostProperty(string eventName, string propertyName, object propertyV Requires.NotNullOrEmpty(propertyName); Requires.NotNull(propertyValue); - var telemetryEvent = new TelemetryEvent(eventName); + TelemetryEvent telemetryEvent = new(eventName); telemetryEvent.Properties.Add(propertyName, propertyValue); PostTelemetryEvent(telemetryEvent); @@ -32,7 +32,7 @@ public void PostProperties(string eventName, IEnumerable<(string propertyName, o Requires.NotNullOrEmpty(eventName); Requires.NotNullOrEmpty(properties); - var telemetryEvent = new TelemetryEvent(eventName); + TelemetryEvent telemetryEvent = new(eventName); AddPropertiesToEvent(properties, telemetryEvent); PostTelemetryEvent(telemetryEvent); @@ -91,7 +91,7 @@ public string HashValue(string value) } byte[] inputBytes = Encoding.UTF8.GetBytes(value); - using var cryptoServiceProvider = new SHA256CryptoServiceProvider(); + using SHA256CryptoServiceProvider cryptoServiceProvider = new(); return BitConverter.ToString(cryptoServiceProvider.ComputeHash(inputBytes)); } From 1b68cd0ef931f5dd9c02a122ff0bd4b0285df977 Mon Sep 17 00:00:00 2001 From: Tom Meschter Date: Tue, 14 Mar 2023 13:51:34 -0700 Subject: [PATCH 5/6] Extract out repeated strings --- .../Telemetry/ManagedTelemetryService.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs index 300a3f0bc68..6322cbcbfc2 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs @@ -8,6 +8,9 @@ namespace Microsoft.VisualStudio.Telemetry; [Export(typeof(ITelemetryService))] internal class ManagedTelemetryService : ITelemetryService { + private const string EventNamePrefix = "vs/projectsystem/managed/"; + private const string PropertyNamePrefix = "vs.projectsystem.managed."; + public void PostEvent(string eventName) { Requires.NotNullOrEmpty(eventName); @@ -56,11 +59,11 @@ private static void AddPropertiesToEvent(IEnumerable<(string propertyName, objec private void PostTelemetryEvent(TelemetryEvent telemetryEvent) { #if DEBUG - Assumes.True(telemetryEvent.Name.StartsWith("vs/projectsystem/managed/", StringComparisons.TelemetryEventNames)); + Assumes.True(telemetryEvent.Name.StartsWith(EventNamePrefix, StringComparisons.TelemetryEventNames)); foreach (string propertyName in telemetryEvent.Properties.Keys) { - Assumes.True(propertyName.StartsWith("vs.projectsystem.managed.", StringComparisons.TelemetryEventNames)); + Assumes.True(propertyName.StartsWith(PropertyNamePrefix, StringComparisons.TelemetryEventNames)); } #endif @@ -77,7 +80,7 @@ public ITelemetryOperation BeginOperation(string eventName) Requires.NotNullOrEmpty(eventName); #if DEBUG - Assumes.True(eventName.StartsWith("vs/projectsystem/managed/", StringComparisons.TelemetryEventNames)); + Assumes.True(eventName.StartsWith(EventNamePrefix, StringComparisons.TelemetryEventNames)); #endif return new TelemetryOperation(TelemetryService.DefaultSession.StartOperation(eventName)); } @@ -125,7 +128,7 @@ public void SetProperties(IEnumerable<(string propertyName, object propertyValue #if DEBUG foreach ((string propertyName, _) in properties) { - Assumes.True(propertyName.StartsWith("vs.projectsystem.managed.", StringComparisons.TelemetryEventNames)); + Assumes.True(propertyName.StartsWith(PropertyNamePrefix, StringComparisons.TelemetryEventNames)); } #endif From 9c96ef4a1f80dd88e50d59b29ac9e619c4357877 Mon Sep 17 00:00:00 2001 From: Tom Meschter Date: Tue, 14 Mar 2023 15:15:30 -0700 Subject: [PATCH 6/6] Condition fields on DEBUG The `EventNamePrefix` and `PropertyNamePrefix` constants are only _used_ when DEBUG is defined. This leads to errors on Release builds where the compiler complains about an unused field. Here we update the fields to be conditionally _defined_ as well. --- .../Telemetry/ManagedTelemetryService.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs index 6322cbcbfc2..1407adfed93 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/ManagedTelemetryService.cs @@ -8,8 +8,10 @@ namespace Microsoft.VisualStudio.Telemetry; [Export(typeof(ITelemetryService))] internal class ManagedTelemetryService : ITelemetryService { +#if DEBUG private const string EventNamePrefix = "vs/projectsystem/managed/"; private const string PropertyNamePrefix = "vs.projectsystem.managed."; +#endif public void PostEvent(string eventName) {