Closed
Show file tree
Hide file tree
Changes from all commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Failed to load files.
Original file line numberDiff line numberDiff line change
Expand Up@@ -132,7 +132,6 @@ extends:
**/Nerdbank.Streams.dll;
**/Newtonsoft.Json.dll;
**/OmniSharp.Extensions*.dll;
**/Serilog*.dll;
**/System.Reactive.dll;
- task: ArchiveFiles@2
displayName: Zip signed artifacts
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -219,7 +219,6 @@ protected override void BeginProcessing()
protected override void EndProcessing()
{
_logger.Log(PsesLogLevel.Diagnostic, "Beginning EndProcessing block");

try
{
// First try to remove PSReadLine to decomplicate startup
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,9 +15,8 @@ namespace Microsoft.PowerShell.EditorServices.Hosting
/// User-facing log level for editor services configuration.
/// </summary>
/// <remarks>
/// The underlying values of this enum attempt to align to both
/// <see cref="Microsoft.Extensions.Logging.LogLevel" /> and
/// <see cref="Serilog.Events.LogEventLevel" />.
/// The underlying values of this enum attempt to align to
/// <see cref="Microsoft.Extensions.Logging.LogLevel" />
/// </remarks>
public enum PsesLogLevel
{
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -42,7 +42,7 @@ public EditorServicesRunner(
_config = config;
_sessionFileWriter = sessionFileWriter;
// NOTE: This factory helps to isolate `Microsoft.Extensions.Logging/DependencyInjection`.
_serverFactory = EditorServicesServerFactory.Create(_config.LogPath, (int)_config.LogLevel, logger);
_serverFactory = new(logger);
_alreadySubscribedDebug = false;
_loggersToUnsubscribe = loggersToUnsubscribe;
}
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,81 +2,30 @@
// Licensed under the MIT License.

using System;
using System.Diagnostics;
using System.IO;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.PowerShell.EditorServices.Logging;
using Microsoft.PowerShell.EditorServices.Server;
using Serilog;
using Serilog.Events;
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
using Microsoft.PowerShell.EditorServices.Services.Extension;

#if DEBUG
using Serilog.Debugging;
#endif
// The HostLogger type isn't directly referenced from this assembly, however it uses a common IObservable interface and this alias helps make it more clear the purpose. We can use Microsoft.Extensions.Logging from this point because the ALC should be loaded, but we need to only expose the IObservable to the Hosting assembly so it doesn't try to load MEL before the ALC is ready.
using HostLogger = System.IObservable<(int logLevel, string message)>;

namespace Microsoft.PowerShell.EditorServices.Hosting
{
/// <summary>
/// Factory class for hiding dependencies of Editor Services.
/// Factory for creating the LSP server and debug server instances.
/// </summary>
/// <remarks>
/// Dependency injection and logging are wrapped by factory methods on this class so that the
/// host assembly can construct the LSP and debug servers without directly depending on <see
/// cref="Microsoft.Extensions.Logging"/> and <see
/// cref="Microsoft.Extensions.DependencyInjection"/>.
/// </remarks>
internal sealed class EditorServicesServerFactory : IDisposable
{
private readonly HostLogger _hostLogger;

/// <summary>
/// Create a new Editor Services factory. This method will instantiate logging.
/// Creates a loggerfactory for this instance
/// </summary>
/// <remarks>
/// <para>
/// This can only be called once because it sets global state (the logger) and that call is
/// in <see cref="Hosting.EditorServicesRunner" />.
/// </para>
/// <para>
/// TODO: Why is this a static function wrapping a constructor instead of just a
/// constructor? In the end it returns an instance (albeit a "singleton").
/// </para>
/// </remarks>
/// <param name="logDirectoryPath">The path of the log file to use.</param>
/// <param name="minimumLogLevel">The minimum log level to use.</param>
/// <param name="hostLogger">The host logger?</param>
public static EditorServicesServerFactory Create(string logDirectoryPath, int minimumLogLevel, IObservable<(int logLevel, string message)> hostLogger)
{
// NOTE: Ignore the suggestion to use Environment.ProcessId as it doesn't work for
// .NET 4.6.2 (for Windows PowerShell), and this won't be caught in CI.
int currentPID = Process.GetCurrentProcess().Id;
string logPath = Path.Combine(logDirectoryPath, $"PowerShellEditorServices-{currentPID}.log");
Log.Logger = new LoggerConfiguration()
.Enrich.FromLogContext()
.WriteTo.Async(config => config.File(logPath))
.MinimumLevel.Is((LogEventLevel)minimumLogLevel)
.CreateLogger();

#if DEBUG
SelfLog.Enable(msg => Debug.WriteLine(msg));
#endif

LoggerFactory loggerFactory = new();
loggerFactory.AddSerilog();

// Hook up logging from the host so that its recorded in the log file
hostLogger.Subscribe(new HostLoggerAdapter(loggerFactory));

return new EditorServicesServerFactory(loggerFactory);
}

// TODO: Can we somehow refactor this member so the language and debug servers can be
// instantiated using their constructors instead of tying them to this factory with `Create`
// methods?
private readonly ILoggerFactory _loggerFactory;

private EditorServicesServerFactory(ILoggerFactory loggerFactory) => _loggerFactory = loggerFactory;
/// <param name="hostLogger">The hostLogger that will be provided to the language services for logging handoff</param>
internal EditorServicesServerFactory(HostLogger hostLogger) => _hostLogger = hostLogger;

/// <summary>
/// Create the LSP server.
Expand All@@ -92,7 +41,7 @@ public static EditorServicesServerFactory Create(string logDirectoryPath, int mi
public PsesLanguageServer CreateLanguageServer(
Stream inputStream,
Stream outputStream,
HostStartupInfo hostStartupInfo) => new(_loggerFactory, inputStream, outputStream, hostStartupInfo);
HostStartupInfo hostStartupInfo) => new(_hostLogger, inputStream, outputStream, hostStartupInfo);

/// <summary>
/// Create the debug server given a language server instance.
Expand All@@ -110,7 +59,7 @@ public PsesDebugServer CreateDebugServerWithLanguageServer(
PsesLanguageServer languageServer)
{
return new PsesDebugServer(
_loggerFactory,
_hostLogger,
inputStream,
outputStream,
languageServer.LanguageServer.Services);
Expand All@@ -132,7 +81,7 @@ public PsesDebugServer RecreateDebugServer(
PsesDebugServer debugServer)
{
return new PsesDebugServer(
_loggerFactory,
_hostLogger,
inputStream,
outputStream,
debugServer.ServiceProvider);
Expand All@@ -153,7 +102,6 @@ public PsesDebugServer CreateDebugServerForTempSession(
ServiceProvider serviceProvider = new ServiceCollection()
.AddLogging(builder => builder
.ClearProviders()
.AddSerilog()
.SetMinimumLevel(LogLevel.Trace)) // TODO: Why randomly set to trace?
.AddSingleton<ILanguageServerFacade>(_ => null)
// TODO: Why add these for a debug server?!
Expand All@@ -171,25 +119,14 @@ public PsesDebugServer CreateDebugServerForTempSession(
serviceProvider.GetService<ExtensionService>();

return new PsesDebugServer(
_loggerFactory,
_hostLogger,
inputStream,
outputStream,
serviceProvider,
isTemp: true);
}

/// <summary>
/// TODO: This class probably should not be <see cref="IDisposable"/> as the primary
/// intention of that interface is to provide cleanup of unmanaged resources, which the
/// logger certainly is not. Nor is this class used with a <see langword="using"/>. Instead,
/// this class should call <see cref="Log.CloseAndFlush()"/> in a finalizer. This
/// could potentially even be done with <see
/// cref="SerilogLoggerFactoryExtensions.AddSerilog"</> by passing <c>dispose=true</c>.
/// </summary>
public void Dispose()
{
Log.CloseAndFlush();
_loggerFactory.Dispose();
}
// TODO: Clean up host logger? Shouldn't matter since we start a new process after shutdown.
public void Dispose() { }
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -107,9 +107,8 @@ public sealed class HostStartupInfo
/// The minimum log level of log events to be logged.
/// </summary>
/// <remarks>
/// This is cast to all of <see cref="Hosting.PsesLogLevel"/>, <see
/// cref="Microsoft.Extensions.Logging.LogLevel"/>, and <see
/// cref="Serilog.Events.LogEventLevel"/>, hence it is an <c>int</c>.
/// This primitive maps to <see cref="Hosting.PsesLogLevel"/> and <see
/// cref="Microsoft.Extensions.Logging.LogLevel"/>
/// </remarks>
public int LogLevel { get; }

Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,23 +9,16 @@ namespace Microsoft.PowerShell.EditorServices.Logging
/// <summary>
/// Adapter class to allow logging events sent by the host to be recorded by PSES' logging infrastructure.
/// </summary>
internal class HostLoggerAdapter : IObserver<(int logLevel, string message)>
internal class HostLoggerAdapter(ILogger logger) : IObserver<(int logLevel, string message)>
{
private readonly ILogger _logger;
public void OnError(Exception error) => logger.LogError(error, "Error in host logger");

/// <summary>
/// Create a new host logger adapter.
/// </summary>
/// <param name="loggerFactory">Factory to create logger instances with.</param>
public HostLoggerAdapter(ILoggerFactory loggerFactory) => _logger = loggerFactory.CreateLogger("HostLogs");
public void OnNext((int logLevel, string message) value) => logger.Log((LogLevel)value.logLevel, value.message);

public void OnCompleted()
{
// Nothing to do; we simply don't send more log messages
}

public void OnError(Exception error) => _logger.LogError(error, "Error in host logger");

public void OnNext((int logLevel, string message) value) => _logger.Log((LogLevel)value.logLevel, value.message);
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -36,10 +36,6 @@
<PackageReference Include="Microsoft.Extensions.FileSystemGlobbing" />
<PackageReference Include="Microsoft.Extensions.Logging" />
<PackageReference Include="PowerShellStandard.Library" />
<PackageReference Include="Serilog" />
<PackageReference Include="Serilog.Extensions.Logging" />
<PackageReference Include="Serilog.Sinks.Async" />
<PackageReference Include="Serilog.Sinks.File" />
<PackageReference Include="System.IO.Pipes.AccessControl" />
<PackageReference Include="System.Security.Principal" />
<PackageReference Include="System.Security.Principal.Windows" />
Expand Down
Loading