From a119c39367e02087731503e27aa62ea31a4728ae Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Thu, 10 Aug 2023 16:03:41 +0200 Subject: [PATCH 1/4] Added fault reporting in telemetry + added opt out for all events for now --- .../FSharp.Editor/Common/CancellableTasks.fs | 2 + .../Navigation/GoToDefinition.fs | 4 ++ .../Navigation/NavigableSymbolsService.fs | 6 ++ .../Telemetry/TelemetryReporter.fs | 56 +++++++++++++------ 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs b/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs index cb81934debb..3cfafa84e88 100644 --- a/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs +++ b/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs @@ -96,6 +96,8 @@ module CancellableTasks = member inline this.ThrowIfCancellationRequested() = this.CancellationToken.ThrowIfCancellationRequested() + member inline this.IsCancellationRequested = this.CancellationToken.IsCancellationRequested + /// This is used by the compiler as a template for creating state machine structs and CancellableTaskStateMachine<'TOverall> = ResumableStateMachine> diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index 96f7f0d54a8..7ac041eaa1c 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -29,6 +29,7 @@ open FSharp.Compiler.Tokenization open System.Composition open System.Text.RegularExpressions open CancellableTasks +open Telemetry module private Symbol = let fullName (root: ISymbol) : string = @@ -679,6 +680,8 @@ type internal FSharpNavigation(metadataAsSource: FSharpMetadataAsSourceService, // Task.Wait throws an exception if the task is cancelled, so be sure to catch it. try // This call to Wait() is fine because we want to be able to provide the error message in the status bar. + use _ = TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.GoToDefinition, [||]) + gtdTask.Wait(cancellationToken) if gtdTask.Status = TaskStatus.RanToCompletion && gtdTask.Result.IsSome then @@ -695,6 +698,7 @@ type internal FSharpNavigation(metadataAsSource: FSharpMetadataAsSourceService, statusBar.TempMessage(SR.CannotDetermineSymbol()) false with exc -> + TelemetryReporter.ReportFault(TelemetryEvents.GoToDefinition, FaultSeverity.General, exc) statusBar.TempMessage(String.Format(SR.NavigateToFailed(), Exception.flattenMessage exc)) // Don't show the dialog box as it's most likely that the user cancelled. diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigableSymbolsService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigableSymbolsService.fs index 99a07f8fd1d..fea3bcfe8c5 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigableSymbolsService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigableSymbolsService.fs @@ -14,6 +14,8 @@ open Microsoft.VisualStudio.Language.Intellisense open Microsoft.VisualStudio.Text open Microsoft.VisualStudio.Text.Editor open Microsoft.VisualStudio.Utilities +open Microsoft.VisualStudio.FSharp.Editor.Telemetry +open Microsoft.VisualStudio.Telemetry [] type internal FSharpNavigableSymbol(item: FSharpNavigableItem, span: SnapshotSpan, gtd: GoToDefinition) = @@ -52,6 +54,8 @@ type internal FSharpNavigableSymbolSource(metadataAsSource) = // Task.Wait throws an exception if the task is cancelled, so be sure to catch it. try // This call to Wait() is fine because we want to be able to provide the error message in the status bar. + use _ = TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.GoToDefinitionGetSymbol, [||]) + gtdTask.Wait(cancellationToken) statusBar.Clear() @@ -88,6 +92,8 @@ type internal FSharpNavigableSymbolSource(metadataAsSource) = // The NavigableSymbols API accepts 'null' when there's nothing to navigate to. return null with exc -> + TelemetryReporter.ReportFault(TelemetryEvents.GoToDefinitionGetSymbol, FaultSeverity.General, exc) + statusBar.TempMessage(String.Format(SR.NavigateToFailed(), Exception.flattenMessage exc)) // The NavigableSymbols API accepts 'null' when there's nothing to navigate to. diff --git a/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs b/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs index 753274a26ec..3a0bf06ac1a 100644 --- a/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs +++ b/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs @@ -25,10 +25,10 @@ module TelemetryEvents = let LanguageServiceStarted = "languageservicestarted" [] - let GetSymbolUsesInProjectsStarted = "getSymbolUsesInProjectsStarted" + let GetSymbolUsesInProjectsStarted = "getsymbolusesinprojectsstarted" [] - let GetSymbolUsesInProjectsFinished = "getSymbolUsesInProjectsFinished" + let GetSymbolUsesInProjectsFinished = "getsymbolusesinprojectsfinished" [] let AddSyntacticCalssifications = "addsyntacticclassifications" @@ -41,6 +41,12 @@ module TelemetryEvents = [] let ProvideCompletions = "providecompletions" + + [] + let GoToDefinition = "gotodefinition" + + [] + let GoToDefinitionGetSymbol = "gotodefinition/getsymbol" // TODO: needs to be something more sophisticated in future [] @@ -89,18 +95,39 @@ type TelemetryReporter private (name: string, props: (string * obj) array, stopw static member val private SendAdditionalTelemetry = lazy - (let componentModel = - Package.GetGlobalService(typeof) :?> ComponentModelHost.IComponentModel - - if componentModel = null then - TelemetryService.DefaultSession.IsUserMicrosoftInternal - else - let workspace = componentModel.GetService() - workspace.Services.GetService().Advanced.SendAdditionalTelemetry) + ( + let componentModel = + Package.GetGlobalService(typeof) :?> ComponentModelHost.IComponentModel + + if isNull componentModel then + TelemetryService.DefaultSession.IsUserMicrosoftInternal + else + let workspace = componentModel.GetService() + + TelemetryService.DefaultSession.IsUserMicrosoftInternal + || workspace.Services.GetService().Advanced.SendAdditionalTelemetry) + + static member ReportFault(name, ?severity: FaultSeverity, ?e: exn) = + if TelemetryReporter.SendAdditionalTelemetry.Value then + let faultName = String.Concat(name, "/fault") + match severity, e with + | Some s, Some e -> TelemetryService.DefaultSession.PostFault(faultName, name, s, e) + | None, Some e -> TelemetryService.DefaultSession.PostFault(faultName, name, e) + | Some s, None -> TelemetryService.DefaultSession.PostFault(faultName, name, s) + | None, None -> TelemetryService.DefaultSession.PostFault(faultName, name) + |> ignore + + static member ReportCustomFailure(name, ?props) = + if TelemetryReporter.SendAdditionalTelemetry.Value then + let props = defaultArg props [||] + let name = String.Concat(name, "/failure") + let event = TelemetryReporter.createEvent name props + TelemetryService.DefaultSession.PostEvent event static member ReportSingleEvent(name, props) = - let event = TelemetryReporter.createEvent name props - TelemetryService.DefaultSession.PostEvent event + if TelemetryReporter.SendAdditionalTelemetry.Value then + let event = TelemetryReporter.createEvent name props + TelemetryService.DefaultSession.PostEvent event // A naïve implementation using stopwatch and returning an IDisposable // TODO: needs a careful review, since it will be a hot path when we are sending telemetry @@ -108,10 +135,7 @@ type TelemetryReporter private (name: string, props: (string * obj) array, stopw let additionalTelemetryEnabled = TelemetryReporter.SendAdditionalTelemetry.Value - let isUserMicrosoftInternal = - TelemetryService.DefaultSession.IsUserMicrosoftInternal - - if additionalTelemetryEnabled || isUserMicrosoftInternal then + if additionalTelemetryEnabled then let throttlingStrategy = defaultArg throttlingStrategy TelemetryThrottlingStrategy.Default From 6c92da60f06ac8a003a612d96b5993e7a386e42d Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Thu, 10 Aug 2023 16:05:28 +0200 Subject: [PATCH 2/4] Removed accidentally added code --- vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs | 2 -- 1 file changed, 2 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs b/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs index 3cfafa84e88..cb81934debb 100644 --- a/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs +++ b/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs @@ -96,8 +96,6 @@ module CancellableTasks = member inline this.ThrowIfCancellationRequested() = this.CancellationToken.ThrowIfCancellationRequested() - member inline this.IsCancellationRequested = this.CancellationToken.IsCancellationRequested - /// This is used by the compiler as a template for creating state machine structs and CancellableTaskStateMachine<'TOverall> = ResumableStateMachine> From 91d7161454024404d49bb3ef3684d1e8f1e5fc6b Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Thu, 10 Aug 2023 17:14:17 +0200 Subject: [PATCH 3/4] Missing namespaces --- vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index 7ac041eaa1c..a2226f8cb1a 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -25,11 +25,11 @@ open FSharp.Compiler.EditorServices open FSharp.Compiler.Text open FSharp.Compiler.Text.Range open FSharp.Compiler.Symbols -open FSharp.Compiler.Tokenization open System.Composition open System.Text.RegularExpressions open CancellableTasks -open Telemetry +open Microsoft.VisualStudio.FSharp.Editor.Telemetry +open Microsoft.VisualStudio.Telemetry module private Symbol = let fullName (root: ISymbol) : string = From a70bd812dc12f713fda35543b93af4d51341d8c0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 11 Aug 2023 10:20:56 +0000 Subject: [PATCH 4/4] Automated command ran: fantomas Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com> --- .../Navigation/GoToDefinition.fs | 3 ++- .../Navigation/NavigableSymbolsService.fs | 3 ++- .../Telemetry/TelemetryReporter.fs | 20 +++++++++---------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index a2226f8cb1a..905919f4ce9 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -680,7 +680,8 @@ type internal FSharpNavigation(metadataAsSource: FSharpMetadataAsSourceService, // Task.Wait throws an exception if the task is cancelled, so be sure to catch it. try // This call to Wait() is fine because we want to be able to provide the error message in the status bar. - use _ = TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.GoToDefinition, [||]) + use _ = + TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.GoToDefinition, [||]) gtdTask.Wait(cancellationToken) diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigableSymbolsService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigableSymbolsService.fs index fea3bcfe8c5..ab7a4b6a29a 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigableSymbolsService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigableSymbolsService.fs @@ -54,7 +54,8 @@ type internal FSharpNavigableSymbolSource(metadataAsSource) = // Task.Wait throws an exception if the task is cancelled, so be sure to catch it. try // This call to Wait() is fine because we want to be able to provide the error message in the status bar. - use _ = TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.GoToDefinitionGetSymbol, [||]) + use _ = + TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.GoToDefinitionGetSymbol, [||]) gtdTask.Wait(cancellationToken) statusBar.Clear() diff --git a/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs b/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs index 3a0bf06ac1a..fae5dde7797 100644 --- a/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs +++ b/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs @@ -41,7 +41,7 @@ module TelemetryEvents = [] let ProvideCompletions = "providecompletions" - + [] let GoToDefinition = "gotodefinition" @@ -95,21 +95,21 @@ type TelemetryReporter private (name: string, props: (string * obj) array, stopw static member val private SendAdditionalTelemetry = lazy - ( - let componentModel = - Package.GetGlobalService(typeof) :?> ComponentModelHost.IComponentModel + (let componentModel = + Package.GetGlobalService(typeof) :?> ComponentModelHost.IComponentModel - if isNull componentModel then - TelemetryService.DefaultSession.IsUserMicrosoftInternal - else - let workspace = componentModel.GetService() + if isNull componentModel then + TelemetryService.DefaultSession.IsUserMicrosoftInternal + else + let workspace = componentModel.GetService() - TelemetryService.DefaultSession.IsUserMicrosoftInternal - || workspace.Services.GetService().Advanced.SendAdditionalTelemetry) + TelemetryService.DefaultSession.IsUserMicrosoftInternal + || workspace.Services.GetService().Advanced.SendAdditionalTelemetry) static member ReportFault(name, ?severity: FaultSeverity, ?e: exn) = if TelemetryReporter.SendAdditionalTelemetry.Value then let faultName = String.Concat(name, "/fault") + match severity, e with | Some s, Some e -> TelemetryService.DefaultSession.PostFault(faultName, name, s, e) | None, Some e -> TelemetryService.DefaultSession.PostFault(faultName, name, e)