From 2245fb2fd52d68172ae298951f646df5779dfec1 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Thu, 1 Oct 2020 16:01:20 +1000 Subject: [PATCH 1/8] Log telemetry item when parse or compilation options change --- .../ProjectSystem/VisualStudioProject.cs | 22 ++++++++++++++++--- .../AbstractWorkspaceTelemetryService.cs | 9 ++++++++ .../Telemetry/IWorkspaceTelemetryService.cs | 7 ++++++ .../Compiler/Core/Log/FunctionId.cs | 2 ++ 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs index f546ee08b2a88..d9fbb10d68327 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs @@ -14,6 +14,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Telemetry; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList; using Roslyn.Utilities; @@ -182,7 +183,7 @@ internal VisualStudioProject( _parseOptions = parseOptions; } - private void ChangeProjectProperty(ref T field, T newValue, Func withNewValue) + private void ChangeProjectProperty(ref T field, T newValue, Func withNewValue, bool logThrowAwayTelemetry = false) { lock (_gate) { @@ -194,6 +195,21 @@ private void ChangeProjectProperty(ref T field, T newValue, Func(); + // If we already have a compilation, then this change will mean it gets throw away, so log it + if (_workspace.CurrentSolution.State.TryGetCompilation(Id, out var previousCompilation)) + { + int numTrees = previousCompilation.SyntaxTrees.Count(); + if (numTrees > 0) + { + var projectState = _workspace.CurrentSolution.State.GetRequiredProjectState(Id); + telemetry.ReportCompilationThrownAway(projectState.ProjectInfo.Attributes.TelemetryId, numTrees); + } + } + } + if (_activeBatchScopes > 0) { _projectPropertyModificationsInBatch.Add(withNewValue); @@ -241,7 +257,7 @@ public string AssemblyName public CompilationOptions? CompilationOptions { get => _compilationOptions; - set => ChangeProjectProperty(ref _compilationOptions, value, s => s.WithProjectCompilationOptions(Id, value)); + set => ChangeProjectProperty(ref _compilationOptions, value, s => s.WithProjectCompilationOptions(Id, value), logThrowAwayTelemetry: true); } // The property could be null if this is a non-C#/VB language and we don't have one for it. But we disallow assigning null, because C#/VB cannot end up null @@ -250,7 +266,7 @@ public CompilationOptions? CompilationOptions public ParseOptions? ParseOptions { get => _parseOptions; - set => ChangeProjectProperty(ref _parseOptions, value, s => s.WithProjectParseOptions(Id, value)); + set => ChangeProjectProperty(ref _parseOptions, value, s => s.WithProjectParseOptions(Id, value), logThrowAwayTelemetry: true); } /// diff --git a/src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs b/src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs index 29fb9d20785a0..20c12ad28f6aa 100644 --- a/src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs +++ b/src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs @@ -93,5 +93,14 @@ public void ReportApiUsage(HashSet symbols, Guid solutionSessionId, Gui // no-op } } + + public void ReportCompilationThrownAway(Guid projectGuid, int syntaxTreesParsed) + { + Logger.Log(FunctionId.Workspace_Project_CompilationThrownAway, KeyValueLogMessage.Create(m => + { + m[nameof(projectGuid)] = projectGuid.ToString("B"); + m[nameof(syntaxTreesParsed)] = syntaxTreesParsed; + })); + } } } diff --git a/src/Workspaces/Core/Portable/Telemetry/IWorkspaceTelemetryService.cs b/src/Workspaces/Core/Portable/Telemetry/IWorkspaceTelemetryService.cs index 0464eb06f86cb..db720ef5becc8 100644 --- a/src/Workspaces/Core/Portable/Telemetry/IWorkspaceTelemetryService.cs +++ b/src/Workspaces/Core/Portable/Telemetry/IWorkspaceTelemetryService.cs @@ -39,5 +39,12 @@ internal interface IWorkspaceTelemetryService : IWorkspaceService /// Reports telemetry on API usage. /// void ReportApiUsage(HashSet symbols, Guid solutionSessionId, Guid projectGuid); + + /// + /// Reports telemetry on a compilation being thrown away during evaluation + /// + /// The guid of the project, for correlation with the project system + /// The number of syntax trees that have been parsed. + void ReportCompilationThrownAway(Guid projectGuid, int syntaxTreesThrownAway); } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs index e3f4598d5c36b..6695cddf0f079 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs @@ -498,5 +498,7 @@ internal enum FunctionId RemoteSemanticClassificationCacheService_ExceptionInCacheRead = 440, FeatureNotAvailable = 441, + + Workspace_Project_CompilationThrownAway = 442 } } From 0c5b3801ab8fa6a82858df0db1085a2c746a98f4 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Mon, 12 Oct 2020 11:51:37 +1100 Subject: [PATCH 2/8] Move logging code to proejct --- .../ProjectSystem/VisualStudioProject.cs | 46 ++++++++++++++----- .../AbstractWorkspaceTelemetryService.cs | 9 ---- .../Telemetry/IWorkspaceTelemetryService.cs | 7 --- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs index d9fbb10d68327..e6a0ae4ec9fc9 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs @@ -13,8 +13,8 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Shared.Extensions; -using Microsoft.CodeAnalysis.Telemetry; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList; using Roslyn.Utilities; @@ -197,17 +197,7 @@ private void ChangeProjectProperty(ref T field, T newValue, Func(); - // If we already have a compilation, then this change will mean it gets throw away, so log it - if (_workspace.CurrentSolution.State.TryGetCompilation(Id, out var previousCompilation)) - { - int numTrees = previousCompilation.SyntaxTrees.Count(); - if (numTrees > 0) - { - var projectState = _workspace.CurrentSolution.State.GetRequiredProjectState(Id); - telemetry.ReportCompilationThrownAway(projectState.ProjectInfo.Attributes.TelemetryId, numTrees); - } - } + TryReportCompilationThrownAway(_workspace.CurrentSolution.State, Id); } if (_activeBatchScopes > 0) @@ -221,6 +211,38 @@ private void ChangeProjectProperty(ref T field, T newValue, Func + /// Reports a telemetry event if compilation information is being thrown away after being previous computed + /// + private static void TryReportCompilationThrownAway(SolutionState solutionState, ProjectId projectId) + { + // We log the number of syntax trees that have been parsed even if there was no compilation created yet + var projectState = solutionState.GetRequiredProjectState(projectId); + var parsedTrees = 0; + foreach (var documentState in projectState.DocumentStates.Values) + { + if (documentState.TryGetSyntaxTree(out _)) + { + parsedTrees++; + } + } + + // But we also want to know if a compilation was created + var hadCompilation = solutionState.TryGetCompilation(projectId, out var previousCompilation); + + if (parsedTrees > 0 || hadCompilation) + { + Logger.Log(FunctionId.Workspace_Project_CompilationThrownAway, KeyValueLogMessage.Create(m => + { + // Note: Not using our project Id. This is the same ProjectGuid that the project system uses + // so data can be correlated + m["ProjectGuid"] = projectState.ProjectInfo.Attributes.TelemetryId.ToString("B"); + m["SyntaxTreesParsed"] = parsedTrees; + m["HadCompilation"] = hadCompilation; + })); + } + } + private void ChangeProjectOutputPath(ref string? field, string? newValue, Func withNewValue) { lock (_gate) diff --git a/src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs b/src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs index 20c12ad28f6aa..29fb9d20785a0 100644 --- a/src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs +++ b/src/VisualStudio/Core/Def/Telemetry/AbstractWorkspaceTelemetryService.cs @@ -93,14 +93,5 @@ public void ReportApiUsage(HashSet symbols, Guid solutionSessionId, Gui // no-op } } - - public void ReportCompilationThrownAway(Guid projectGuid, int syntaxTreesParsed) - { - Logger.Log(FunctionId.Workspace_Project_CompilationThrownAway, KeyValueLogMessage.Create(m => - { - m[nameof(projectGuid)] = projectGuid.ToString("B"); - m[nameof(syntaxTreesParsed)] = syntaxTreesParsed; - })); - } } } diff --git a/src/Workspaces/Core/Portable/Telemetry/IWorkspaceTelemetryService.cs b/src/Workspaces/Core/Portable/Telemetry/IWorkspaceTelemetryService.cs index db720ef5becc8..0464eb06f86cb 100644 --- a/src/Workspaces/Core/Portable/Telemetry/IWorkspaceTelemetryService.cs +++ b/src/Workspaces/Core/Portable/Telemetry/IWorkspaceTelemetryService.cs @@ -39,12 +39,5 @@ internal interface IWorkspaceTelemetryService : IWorkspaceService /// Reports telemetry on API usage. /// void ReportApiUsage(HashSet symbols, Guid solutionSessionId, Guid projectGuid); - - /// - /// Reports telemetry on a compilation being thrown away during evaluation - /// - /// The guid of the project, for correlation with the project system - /// The number of syntax trees that have been parsed. - void ReportCompilationThrownAway(Guid projectGuid, int syntaxTreesThrownAway); } } From bb1d44aef6173ddda9ef06044ba0e7a84d7bdf36 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Mon, 12 Oct 2020 11:51:48 +1100 Subject: [PATCH 3/8] Log when changing assembly name --- .../Def/Implementation/ProjectSystem/VisualStudioProject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs index e6a0ae4ec9fc9..065eb9aeb4b1d 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs @@ -270,7 +270,7 @@ private void ChangeProjectOutputPath(ref string? field, string? newValue, Func _assemblyName; - set => ChangeProjectProperty(ref _assemblyName, value, s => s.WithProjectAssemblyName(Id, value)); + set => ChangeProjectProperty(ref _assemblyName, value, s => s.WithProjectAssemblyName(Id, value), logThrowAwayTelemetry: true); } // The property could be null if this is a non-C#/VB language and we don't have one for it. But we disallow assigning null, because C#/VB cannot end up null From 09e2c184bc35dbf94a2dd574dc4f93c351742aeb Mon Sep 17 00:00:00 2001 From: David Wengier Date: Tue, 27 Oct 2020 07:21:18 +1100 Subject: [PATCH 4/8] Update src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs Co-authored-by: Jason Malinowski --- .../Def/Implementation/ProjectSystem/VisualStudioProject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs index 065eb9aeb4b1d..a6ca91d120add 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs @@ -212,7 +212,7 @@ private void ChangeProjectProperty(ref T field, T newValue, Func - /// Reports a telemetry event if compilation information is being thrown away after being previous computed + /// Reports a telemetry event if compilation information is being thrown away after being previously computed /// private static void TryReportCompilationThrownAway(SolutionState solutionState, ProjectId projectId) { From 3b9db662af8af2c3e66b047fa59550b48bd34b5d Mon Sep 17 00:00:00 2001 From: David Wengier Date: Tue, 27 Oct 2020 19:45:53 +1100 Subject: [PATCH 5/8] Update src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs Co-authored-by: Sam Harwell --- .../Def/Implementation/ProjectSystem/VisualStudioProject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs index a6ca91d120add..35d7c5da0ff81 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs @@ -228,7 +228,7 @@ private static void TryReportCompilationThrownAway(SolutionState solutionState, } // But we also want to know if a compilation was created - var hadCompilation = solutionState.TryGetCompilation(projectId, out var previousCompilation); + var hadCompilation = solutionState.TryGetCompilation(projectId, out _); if (parsedTrees > 0 || hadCompilation) { From 7c9d02a597c910d3eb5a5c8863baf9646e924960 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Tue, 27 Oct 2020 20:57:44 +1100 Subject: [PATCH 6/8] Check for an active session before logging --- .../Def/Implementation/ProjectSystem/VisualStudioProject.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs index 35d7c5da0ff81..e60797d3e56cc 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs @@ -15,6 +15,7 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Telemetry; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList; using Roslyn.Utilities; @@ -27,6 +28,7 @@ internal sealed class VisualStudioProject private readonly VisualStudioWorkspaceImpl _workspace; private readonly HostDiagnosticUpdateSource _hostDiagnosticUpdateSource; + private readonly IWorkspaceTelemetryService _telemetryService; /// /// Provides dynamic source files for files added through . @@ -138,6 +140,8 @@ internal VisualStudioProject( _dynamicFileInfoProviders = dynamicFileInfoProviders; _hostDiagnosticUpdateSource = hostDiagnosticUpdateSource; + _telemetryService = _workspace.Services.GetRequiredService(); + Id = id; Language = language; _displayName = displayName; @@ -195,7 +199,7 @@ private void ChangeProjectProperty(ref T field, T newValue, Func Date: Wed, 28 Oct 2020 07:51:39 +1100 Subject: [PATCH 7/8] Make service optional --- .../Def/Implementation/ProjectSystem/VisualStudioProject.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs index e60797d3e56cc..c4e94bf7e879d 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs @@ -28,7 +28,7 @@ internal sealed class VisualStudioProject private readonly VisualStudioWorkspaceImpl _workspace; private readonly HostDiagnosticUpdateSource _hostDiagnosticUpdateSource; - private readonly IWorkspaceTelemetryService _telemetryService; + private readonly IWorkspaceTelemetryService? _telemetryService; /// /// Provides dynamic source files for files added through . @@ -140,7 +140,7 @@ internal VisualStudioProject( _dynamicFileInfoProviders = dynamicFileInfoProviders; _hostDiagnosticUpdateSource = hostDiagnosticUpdateSource; - _telemetryService = _workspace.Services.GetRequiredService(); + _telemetryService = _workspace.Services.GetService(); Id = id; Language = language; @@ -199,7 +199,7 @@ private void ChangeProjectProperty(ref T field, T newValue, Func Date: Fri, 30 Oct 2020 14:42:08 +1100 Subject: [PATCH 8/8] Check if the solution is fully loaded, and don't log telemetry if not. --- .../ProjectSystem/VisualStudioProject.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs index c4e94bf7e879d..9fe8eac2e2ed6 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs @@ -29,6 +29,7 @@ internal sealed class VisualStudioProject private readonly VisualStudioWorkspaceImpl _workspace; private readonly HostDiagnosticUpdateSource _hostDiagnosticUpdateSource; private readonly IWorkspaceTelemetryService? _telemetryService; + private readonly IWorkspaceStatusService? _workspaceStatusService; /// /// Provides dynamic source files for files added through . @@ -141,6 +142,7 @@ internal VisualStudioProject( _hostDiagnosticUpdateSource = hostDiagnosticUpdateSource; _telemetryService = _workspace.Services.GetService(); + _workspaceStatusService = _workspace.Services.GetService(); Id = id; Language = language; @@ -199,7 +201,16 @@ private void ChangeProjectProperty(ref T field, T newValue, Func