Skip to content

Commit 515deab

Browse files
committed
Remove unused telemetry
While this implementation is useful, we are not currently doing anything with said telemetry and therefore should not be sending it. We can bring it back by reference this commit.
1 parent 0fcb641 commit 515deab

File tree

2 files changed

+21
-102
lines changed

2 files changed

+21
-102
lines changed

src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs

-62
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,9 @@
77
using System.Threading.Tasks;
88
using MediatR;
99
using Microsoft.Extensions.Logging;
10-
using Microsoft.PowerShell.EditorServices.Logging;
1110
using Microsoft.PowerShell.EditorServices.Services;
1211
using Microsoft.PowerShell.EditorServices.Services.Configuration;
13-
using Newtonsoft.Json.Linq;
1412
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
15-
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
16-
using OmniSharp.Extensions.LanguageServer.Protocol.Window;
1713
using OmniSharp.Extensions.LanguageServer.Protocol.Workspace;
1814

1915
namespace Microsoft.PowerShell.EditorServices.Handlers
@@ -23,19 +19,16 @@ internal class PsesConfigurationHandler : DidChangeConfigurationHandlerBase
2319
private readonly ILogger _logger;
2420
private readonly WorkspaceService _workspaceService;
2521
private readonly ConfigurationService _configurationService;
26-
private readonly ILanguageServerFacade _languageServer;
2722
public PsesConfigurationHandler(
2823
ILoggerFactory factory,
2924
WorkspaceService workspaceService,
3025
AnalysisService analysisService,
3126
ConfigurationService configurationService,
32-
ILanguageServerFacade languageServer,
3327
SymbolsService symbolsService)
3428
{
3529
_logger = factory.CreateLogger<PsesConfigurationHandler>();
3630
_workspaceService = workspaceService;
3731
_configurationService = configurationService;
38-
_languageServer = languageServer;
3932

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

54-
SendFeatureChangesTelemetry(incomingSettings);
55-
5647
bool oldScriptAnalysisEnabled = _configurationService.CurrentSettings.ScriptAnalysis.Enable;
5748
string oldScriptAnalysisSettingsPath = _configurationService.CurrentSettings.ScriptAnalysis?.SettingsPath;
5849

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

112-
private void SendFeatureChangesTelemetry(LanguageServerSettingsWrapper incomingSettings)
113-
{
114-
if (incomingSettings is null)
115-
{
116-
_logger.LogTrace("Incoming settings were null");
117-
return;
118-
}
119-
120-
Dictionary<string, bool> configChanges = new();
121-
// Send telemetry if the user opted-out of ScriptAnalysis
122-
if (!incomingSettings.Powershell.ScriptAnalysis.Enable &&
123-
_configurationService.CurrentSettings.ScriptAnalysis.Enable != incomingSettings.Powershell.ScriptAnalysis.Enable)
124-
{
125-
configChanges["ScriptAnalysis"] = incomingSettings.Powershell.ScriptAnalysis.Enable;
126-
}
127-
128-
// Send telemetry if the user opted-out of CodeFolding
129-
if (!incomingSettings.Powershell.CodeFolding.Enable &&
130-
_configurationService.CurrentSettings.CodeFolding.Enable != incomingSettings.Powershell.CodeFolding.Enable)
131-
{
132-
configChanges["CodeFolding"] = incomingSettings.Powershell.CodeFolding.Enable;
133-
}
134-
135-
// Send telemetry if the user opted-out of Profile loading
136-
if (!incomingSettings.Powershell.EnableProfileLoading &&
137-
_configurationService.CurrentSettings.EnableProfileLoading != incomingSettings.Powershell.EnableProfileLoading)
138-
{
139-
configChanges["ProfileLoading"] = incomingSettings.Powershell.EnableProfileLoading;
140-
}
141-
142-
// Send telemetry if the user opted-in to Pester 5+ CodeLens
143-
if (!incomingSettings.Powershell.Pester.UseLegacyCodeLens &&
144-
_configurationService.CurrentSettings.Pester.UseLegacyCodeLens != incomingSettings.Powershell.Pester.UseLegacyCodeLens)
145-
{
146-
// From our perspective we want to see how many people are opting in to this so we flip the value
147-
configChanges["Pester5CodeLens"] = !incomingSettings.Powershell.Pester.UseLegacyCodeLens;
148-
}
149-
150-
// No need to send any telemetry since nothing changed
151-
if (configChanges.Count == 0)
152-
{
153-
return;
154-
}
155-
156-
_languageServer.Window.SendTelemetryEvent(new TelemetryEventParams
157-
{
158-
ExtensionData = new PsesTelemetryEvent
159-
{
160-
EventName = "NonDefaultPsesFeatureConfiguration",
161-
Data = JObject.FromObject(configChanges)
162-
}
163-
});
164-
}
165103

166104
public event EventHandler<LanguageServerSettings> ConfigurationUpdated;
167105
}

test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs

+21-40
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
using System.Threading;
1313
using System.Threading.Tasks;
1414
using Microsoft.PowerShell.EditorServices.Handlers;
15-
using Microsoft.PowerShell.EditorServices.Logging;
1615
using Microsoft.PowerShell.EditorServices.Services.Configuration;
1716
using Microsoft.PowerShell.EditorServices.Services.PowerShell;
1817
using Microsoft.PowerShell.EditorServices.Services.Template;
@@ -40,7 +39,6 @@ public class LanguageServerProtocolMessageTests : IClassFixture<LSPTestsFixture>
4039
private readonly ILanguageClient PsesLanguageClient;
4140
private readonly List<LogMessageParams> Messages;
4241
private readonly List<Diagnostic> Diagnostics;
43-
private readonly List<PsesTelemetryEvent> TelemetryEvents;
4442
private readonly string PwshExe;
4543

4644
public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixture data)
@@ -51,15 +49,12 @@ public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixt
5149
Messages.Clear();
5250
Diagnostics = data.Diagnostics;
5351
Diagnostics.Clear();
54-
TelemetryEvents = data.TelemetryEvents;
55-
TelemetryEvents.Clear();
5652
PwshExe = PsesStdioProcess.PwshExe;
5753
}
5854

5955
public void Dispose()
6056
{
6157
Diagnostics.Clear();
62-
TelemetryEvents.Clear();
6358
GC.SuppressFinalize(this);
6459
}
6560

@@ -91,26 +86,12 @@ private async Task WaitForDiagnosticsAsync()
9186
// Wait for PSSA to finish.
9287
for (int i = 0; Diagnostics.Count == 0; i++)
9388
{
94-
if (i >= 10)
89+
if (i >= 30)
9590
{
9691
throw new InvalidDataException("No diagnostics showed up after 20s.");
9792
}
9893

99-
await Task.Delay(2000).ConfigureAwait(true);
100-
}
101-
}
102-
103-
private async Task WaitForTelemetryEventsAsync()
104-
{
105-
// Wait for PSSA to finish.
106-
for (int i = 0; TelemetryEvents.Count == 0; i++)
107-
{
108-
if (i >= 10)
109-
{
110-
throw new InvalidDataException("No telemetry events showed up after 20s.");
111-
}
112-
113-
await Task.Delay(2000).ConfigureAwait(true);
94+
await Task.Delay(1000).ConfigureAwait(true);
11495
}
11596
}
11697

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

236-
NewTestFile("gci | % { $_ }");
237-
await WaitForDiagnosticsAsync().ConfigureAwait(true);
238-
239-
// NewTestFile doesn't clear diagnostic notifications so we need to do that for this test.
240-
Diagnostics.Clear();
241-
242217
PsesLanguageClient.SendNotification("workspace/didChangeConfiguration",
243218
new DidChangeConfigurationParams
244219
{
@@ -256,18 +231,12 @@ public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync()
256231
})
257232
});
258233

259-
await WaitForTelemetryEventsAsync().ConfigureAwait(true);
260-
PsesTelemetryEvent telemetryEvent = Assert.Single(TelemetryEvents);
261-
Assert.Equal("NonDefaultPsesFeatureConfiguration", telemetryEvent.EventName);
262-
Assert.False((bool)telemetryEvent.Data.GetValue("ScriptAnalysis"));
234+
string filePath = NewTestFile("$a = 4");
263235

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

267-
// Clear telemetry events so we can test to make sure telemetry doesn't
268-
// come through with default settings.
269-
TelemetryEvents.Clear();
270-
271240
// Restore default configuration
272241
PsesLanguageClient.SendNotification("workspace/didChangeConfiguration",
273242
new DidChangeConfigurationParams
@@ -280,10 +249,22 @@ public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync()
280249
})
281250
});
282251

283-
// Wait a bit to make sure no telemetry events came through
284-
await Task.Delay(2000).ConfigureAwait(true);
285-
// Since we have default settings we should not get any telemetry events about
286-
Assert.Empty(TelemetryEvents.Where(e => e.EventName == "NonDefaultPsesFeatureConfiguration"));
252+
// That notification does not trigger re-analyzing open files. For that we have to send
253+
// a textDocument/didChange notification.
254+
PsesLanguageClient.SendNotification("textDocument/didChange", new DidChangeTextDocumentParams
255+
{
256+
ContentChanges = new Container<TextDocumentContentChangeEvent>(),
257+
TextDocument = new OptionalVersionedTextDocumentIdentifier
258+
{
259+
Version = 4,
260+
Uri = new Uri(filePath)
261+
}
262+
});
263+
264+
await WaitForDiagnosticsAsync().ConfigureAwait(true);
265+
266+
Diagnostic diagnostic = Assert.Single(Diagnostics);
267+
Assert.Equal("PSUseDeclaredVarsMoreThanAssignments", diagnostic.Code);
287268
}
288269

289270
[Fact]

0 commit comments

Comments
 (0)