Skip to content

Commit

Permalink
Add flush method and make it implementation detail on how that happens (
Browse files Browse the repository at this point in the history
#11087)

rzls imports telemetry by creating an ExportProvider with the path to
devkit telemetry if it's installed and enabled. Unfortunately that means
that dispose is being prematurely called when that provider is torn
down. To help with this two things are done:

1. Make a flush method on ITelemetryReporter
2. Make when flush happens an implementation detail on the specific
providers.

In this case, rzls will flush after the language server host exits. In
other cases IDisposable will be used to flush telemetry (VS and OOP)
  • Loading branch information
ryzngard authored Oct 24, 2024
2 parents e5aa079 + f24259c commit e9199b6
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,9 @@ internal interface ITelemetryReporter
/// <param name="requestDuration">How long it took to handle the request</param>
/// <param name="result">The result of handling the request</param>
void ReportRequestTiming(string name, string? language, TimeSpan queuedDuration, TimeSpan requestDuration, TelemetryResult result);

/// <summary>
/// Flushes any current session data being collected
/// </summary>
void Flush();
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,8 @@ public TelemetryScope TrackLspRequest(string lspMethodName, string lspServerName
public void ReportRequestTiming(string name, string? language, TimeSpan queuedDuration, TimeSpan requestDuration, TelemetryResult result)
{
}

public void Flush()
{
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Composition;
using Microsoft.AspNetCore.Razor.Telemetry;
using Microsoft.VisualStudio.Razor.Telemetry;
Expand All @@ -9,10 +10,10 @@
namespace Microsoft.CodeAnalysis.Remote.Razor.Telemetry;

[Export(typeof(ITelemetryReporter)), Shared]
internal class OutOfProcessTelemetryReporter : TelemetryReporter
internal class OutOfProcessTelemetryReporter() : TelemetryReporter(TelemetryService.DefaultSession), IDisposable
{
public OutOfProcessTelemetryReporter()
: base(TelemetryService.DefaultSession)
public void Dispose()
{
Flush();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

namespace Microsoft.VisualStudio.Razor.Telemetry;

internal abstract partial class TelemetryReporter : ITelemetryReporter, IDisposable
internal abstract partial class TelemetryReporter : ITelemetryReporter
{
private const string CodeAnalysisNamespace = nameof(Microsoft) + "." + nameof(CodeAnalysis);
private const string AspNetCoreNamespace = nameof(Microsoft) + "." + nameof(AspNetCore);
Expand Down Expand Up @@ -51,9 +51,9 @@ protected TelemetryReporter(TelemetrySession? telemetrySession = null)
public virtual bool IsEnabled => _manager?.Session.IsOptedIn ?? false;
#endif

public void Dispose()
public void Flush()
{
_manager?.Dispose();
_manager?.Flush();
}

public void ReportEvent(string name, Severity severity)
Expand Down Expand Up @@ -215,7 +215,7 @@ public virtual void ReportMetric(TelemetryInstrumentEvent metricEvent)

protected void SetSession(TelemetrySession session)
{
_manager?.Dispose();
_manager?.Flush();
_manager = TelemetrySessionManager.Create(this, session);
}

Expand Down Expand Up @@ -439,7 +439,7 @@ private static bool IsInOwnedNamespace(string declaringTypeName)
declaringTypeName.StartsWith(AspNetCoreNamespace) ||
declaringTypeName.StartsWith(MicrosoftVSRazorNamespace);

private sealed class TelemetrySessionManager : IDisposable
private sealed class TelemetrySessionManager
{
/// <summary>
/// Store request counters in a concurrent dictionary as non-mutating LSP requests can
Expand All @@ -464,11 +464,6 @@ public static TelemetrySessionManager Create(TelemetryReporter telemetryReporter

public TelemetrySession Session { get; }

public void Dispose()
{
Flush();
}

public void Flush()
{
_aggregatingManager.Flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,14 @@
namespace Microsoft.VisualStudio.Razor.Telemetry;

[Export(typeof(ITelemetryReporter))]
internal class VSTelemetryReporter : TelemetryReporter
[method:ImportingConstructor]
internal class VSTelemetryReporter(ILoggerFactory loggerFactory) : TelemetryReporter(TelemetryService.DefaultSession), IDisposable
{
private readonly ILogger _logger;
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<VSTelemetryReporter>();

[ImportingConstructor]
public VSTelemetryReporter(ILoggerFactory loggerFactory)
// Get the DefaultSession for telemetry. This is set by VS with
// TelemetryService.SetDefaultSession and provides the correct
// appinsights keys etc
: base(TelemetryService.DefaultSession)
public void Dispose()
{
_logger = loggerFactory.GetOrCreateLogger<VSTelemetryReporter>();
Flush();
}

protected override bool HandleException(Exception exception, string? message, params object?[] @params)
Expand Down
9 changes: 8 additions & 1 deletion src/Razor/src/rzls/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,14 @@ public static async Task Main(string[] args)

loggerFactory.GetOrCreateLogger("RZLS").LogInformation($"Razor Language Server started successfully.");

await host.WaitForExitAsync().ConfigureAwait(true);
try
{
await host.WaitForExitAsync().ConfigureAwait(true);
}
finally
{
devKitTelemetryReporter?.Flush();
}
}

private static async Task<ITelemetryReporter?> TryGetTelemetryReporterAsync(string telemetryLevel, string sessionId, string telemetryExtensionPath)
Expand Down

0 comments on commit e9199b6

Please sign in to comment.