Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused telemetry #2065

Merged
merged 1 commit into from
Sep 13, 2023
Merged

Remove unused telemetry #2065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 0 additions & 62 deletions src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,9 @@
using System.Threading.Tasks;
using MediatR;
using Microsoft.Extensions.Logging;
using Microsoft.PowerShell.EditorServices.Logging;
using Microsoft.PowerShell.EditorServices.Services;
using Microsoft.PowerShell.EditorServices.Services.Configuration;
using Newtonsoft.Json.Linq;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
using OmniSharp.Extensions.LanguageServer.Protocol.Window;
using OmniSharp.Extensions.LanguageServer.Protocol.Workspace;

namespace Microsoft.PowerShell.EditorServices.Handlers
Expand All @@ -23,19 +19,16 @@ internal class PsesConfigurationHandler : DidChangeConfigurationHandlerBase
private readonly ILogger _logger;
private readonly WorkspaceService _workspaceService;
private readonly ConfigurationService _configurationService;
private readonly ILanguageServerFacade _languageServer;
public PsesConfigurationHandler(
ILoggerFactory factory,
WorkspaceService workspaceService,
AnalysisService analysisService,
ConfigurationService configurationService,
ILanguageServerFacade languageServer,
SymbolsService symbolsService)
{
_logger = factory.CreateLogger<PsesConfigurationHandler>();
_workspaceService = workspaceService;
_configurationService = configurationService;
_languageServer = languageServer;

ConfigurationUpdated += analysisService.OnConfigurationUpdated;
ConfigurationUpdated += symbolsService.OnConfigurationUpdated;
Expand All @@ -51,8 +44,6 @@ public override async Task<Unit> Handle(DidChangeConfigurationParams request, Ca
return await Unit.Task.ConfigureAwait(false);
}

SendFeatureChangesTelemetry(incomingSettings);

bool oldScriptAnalysisEnabled = _configurationService.CurrentSettings.ScriptAnalysis.Enable;
string oldScriptAnalysisSettingsPath = _configurationService.CurrentSettings.ScriptAnalysis?.SettingsPath;

Expand Down Expand Up @@ -109,59 +100,6 @@ public override async Task<Unit> Handle(DidChangeConfigurationParams request, Ca
return await Unit.Task.ConfigureAwait(false);
}

private void SendFeatureChangesTelemetry(LanguageServerSettingsWrapper incomingSettings)
{
if (incomingSettings is null)
{
_logger.LogTrace("Incoming settings were null");
return;
}

Dictionary<string, bool> configChanges = new();
// Send telemetry if the user opted-out of ScriptAnalysis
if (!incomingSettings.Powershell.ScriptAnalysis.Enable &&
_configurationService.CurrentSettings.ScriptAnalysis.Enable != incomingSettings.Powershell.ScriptAnalysis.Enable)
{
configChanges["ScriptAnalysis"] = incomingSettings.Powershell.ScriptAnalysis.Enable;
}

// Send telemetry if the user opted-out of CodeFolding
if (!incomingSettings.Powershell.CodeFolding.Enable &&
_configurationService.CurrentSettings.CodeFolding.Enable != incomingSettings.Powershell.CodeFolding.Enable)
{
configChanges["CodeFolding"] = incomingSettings.Powershell.CodeFolding.Enable;
}

// Send telemetry if the user opted-out of Profile loading
if (!incomingSettings.Powershell.EnableProfileLoading &&
_configurationService.CurrentSettings.EnableProfileLoading != incomingSettings.Powershell.EnableProfileLoading)
{
configChanges["ProfileLoading"] = incomingSettings.Powershell.EnableProfileLoading;
}

// Send telemetry if the user opted-in to Pester 5+ CodeLens
if (!incomingSettings.Powershell.Pester.UseLegacyCodeLens &&
_configurationService.CurrentSettings.Pester.UseLegacyCodeLens != incomingSettings.Powershell.Pester.UseLegacyCodeLens)
{
// From our perspective we want to see how many people are opting in to this so we flip the value
configChanges["Pester5CodeLens"] = !incomingSettings.Powershell.Pester.UseLegacyCodeLens;
}

// No need to send any telemetry since nothing changed
if (configChanges.Count == 0)
{
return;
}

_languageServer.Window.SendTelemetryEvent(new TelemetryEventParams
{
ExtensionData = new PsesTelemetryEvent
{
EventName = "NonDefaultPsesFeatureConfiguration",
Data = JObject.FromObject(configChanges)
}
});
}

public event EventHandler<LanguageServerSettings> ConfigurationUpdated;
}
Expand Down
61 changes: 21 additions & 40 deletions test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.PowerShell.EditorServices.Handlers;
using Microsoft.PowerShell.EditorServices.Logging;
using Microsoft.PowerShell.EditorServices.Services.Configuration;
using Microsoft.PowerShell.EditorServices.Services.PowerShell;
using Microsoft.PowerShell.EditorServices.Services.Template;
Expand Down Expand Up @@ -40,7 +39,6 @@ public class LanguageServerProtocolMessageTests : IClassFixture<LSPTestsFixture>
private readonly ILanguageClient PsesLanguageClient;
private readonly List<LogMessageParams> Messages;
private readonly List<Diagnostic> Diagnostics;
private readonly List<PsesTelemetryEvent> TelemetryEvents;
private readonly string PwshExe;

public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixture data)
Expand All @@ -51,15 +49,12 @@ public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixt
Messages.Clear();
Diagnostics = data.Diagnostics;
Diagnostics.Clear();
TelemetryEvents = data.TelemetryEvents;
TelemetryEvents.Clear();
PwshExe = PsesStdioProcess.PwshExe;
}

public void Dispose()
{
Diagnostics.Clear();
TelemetryEvents.Clear();
GC.SuppressFinalize(this);
}

Expand Down Expand Up @@ -91,26 +86,12 @@ private async Task WaitForDiagnosticsAsync()
// Wait for PSSA to finish.
for (int i = 0; Diagnostics.Count == 0; i++)
{
if (i >= 10)
if (i >= 30)
{
throw new InvalidDataException("No diagnostics showed up after 20s.");
}

await Task.Delay(2000).ConfigureAwait(true);
}
}

private async Task WaitForTelemetryEventsAsync()
{
// Wait for PSSA to finish.
for (int i = 0; TelemetryEvents.Count == 0; i++)
{
if (i >= 10)
{
throw new InvalidDataException("No telemetry events showed up after 20s.");
}

await Task.Delay(2000).ConfigureAwait(true);
await Task.Delay(1000).ConfigureAwait(true);
}
}

Expand Down Expand Up @@ -233,12 +214,6 @@ public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync()
Skip.If(PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
"Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");

NewTestFile("gci | % { $_ }");
await WaitForDiagnosticsAsync().ConfigureAwait(true);

// NewTestFile doesn't clear diagnostic notifications so we need to do that for this test.
Diagnostics.Clear();

PsesLanguageClient.SendNotification("workspace/didChangeConfiguration",
new DidChangeConfigurationParams
{
Expand All @@ -256,18 +231,12 @@ public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync()
})
});

await WaitForTelemetryEventsAsync().ConfigureAwait(true);
PsesTelemetryEvent telemetryEvent = Assert.Single(TelemetryEvents);
Assert.Equal("NonDefaultPsesFeatureConfiguration", telemetryEvent.EventName);
Assert.False((bool)telemetryEvent.Data.GetValue("ScriptAnalysis"));
string filePath = NewTestFile("$a = 4");

// We also shouldn't get any Diagnostics because ScriptAnalysis is disabled.
// Wait a bit to make sure no diagnostics came through
await Task.Delay(2000).ConfigureAwait(true);
Assert.Empty(Diagnostics);

// Clear telemetry events so we can test to make sure telemetry doesn't
// come through with default settings.
TelemetryEvents.Clear();

// Restore default configuration
PsesLanguageClient.SendNotification("workspace/didChangeConfiguration",
new DidChangeConfigurationParams
Expand All @@ -280,10 +249,22 @@ public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync()
})
});

// Wait a bit to make sure no telemetry events came through
await Task.Delay(2000).ConfigureAwait(true);
// Since we have default settings we should not get any telemetry events about
Assert.Empty(TelemetryEvents.Where(e => e.EventName == "NonDefaultPsesFeatureConfiguration"));
// That notification does not trigger re-analyzing open files. For that we have to send
// a textDocument/didChange notification.
PsesLanguageClient.SendNotification("textDocument/didChange", new DidChangeTextDocumentParams
{
ContentChanges = new Container<TextDocumentContentChangeEvent>(),
TextDocument = new OptionalVersionedTextDocumentIdentifier
{
Version = 4,
Uri = new Uri(filePath)
}
});

await WaitForDiagnosticsAsync().ConfigureAwait(true);

Diagnostic diagnostic = Assert.Single(Diagnostics);
Assert.Equal("PSUseDeclaredVarsMoreThanAssignments", diagnostic.Code);
}

[Fact]
Expand Down