From afc4f6e49c3fc5bffe4479fe20ec2ab2126f3918 Mon Sep 17 00:00:00 2001 From: Jared Goodwin Date: Thu, 27 Jul 2023 11:17:37 -0700 Subject: [PATCH] Fix/improve log streaming from agent. --- Agent/Program.cs | 9 ++- Agent/Services/AgentHubConnection.cs | 65 ++++++-------------- Agent/Services/FileLogsManager.cs | 85 +++++++++++++++++++++++++++ Shared/Services/FileLogger.cs | 82 ++------------------------ Shared/Services/FileLoggerDefaults.cs | 56 ++++++++++++++++++ 5 files changed, 170 insertions(+), 127 deletions(-) create mode 100644 Agent/Services/FileLogsManager.cs create mode 100644 Shared/Services/FileLoggerDefaults.cs diff --git a/Agent/Program.cs b/Agent/Program.cs index 7afc93ee5..d6a8b3eb1 100644 --- a/Agent/Program.cs +++ b/Agent/Program.cs @@ -17,6 +17,7 @@ using Microsoft.Extensions.Hosting; using System.Linq; using Microsoft.Win32; +using System.Reflection; namespace Remotely.Agent; @@ -47,7 +48,8 @@ public static async Task Main(string[] args) catch (Exception ex) { var version = AppVersionHelper.GetAppVersion(); - var logger = new FileLogger("Remotely_Agent", version, "Main"); + var componentName = Assembly.GetExecutingAssembly().GetName().Name; + var logger = new FileLogger($"{componentName}", version, "Main"); logger.LogError(ex, "Error during agent startup."); throw; } @@ -86,10 +88,10 @@ private static void RegisterServices(IServiceCollection services) { builder.AddConsole().AddDebug(); var version = AppVersionHelper.GetAppVersion(); - builder.AddProvider(new FileLoggerProvider("Remotely_Agent", version)); + var componentName = Assembly.GetExecutingAssembly().GetName().Name; + builder.AddProvider(new FileLoggerProvider($"{componentName}", version)); }); - // TODO: All these should be registered as interfaces. services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); @@ -102,6 +104,7 @@ private static void RegisterServices(IServiceCollection services) services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddSingleton(); if (OperatingSystem.IsWindows()) { diff --git a/Agent/Services/AgentHubConnection.cs b/Agent/Services/AgentHubConnection.cs index e68a2683a..2e759d1d3 100644 --- a/Agent/Services/AgentHubConnection.cs +++ b/Agent/Services/AgentHubConnection.cs @@ -39,7 +39,7 @@ public class AgentHubConnection : IAgentHubConnection, IDisposable private readonly IHttpClientFactory _httpFactory; private readonly IWakeOnLanService _wakeOnLanService; private readonly ILogger _logger; - private readonly IEnumerable _loggerProviders; + private readonly IFileLogsManager _fileLogsManager; private readonly IScriptExecutor _scriptExecutor; private readonly IUninstaller _uninstaller; private readonly IUpdater _updater; @@ -48,7 +48,6 @@ public class AgentHubConnection : IAgentHubConnection, IDisposable private HubConnection? _hubConnection; private Timer? _heartbeatTimer; private bool _isServerVerified; - private FileLogger? _fileLogger; public AgentHubConnection( IConfigService configService, @@ -60,7 +59,7 @@ public AgentHubConnection( IDeviceInformationService deviceInfoService, IHttpClientFactory httpFactory, IWakeOnLanService wakeOnLanService, - IEnumerable loggerProviders, + IFileLogsManager fileLogsManager, ILogger logger) { _configService = configService; @@ -73,7 +72,7 @@ public AgentHubConnection( _httpFactory = httpFactory; _wakeOnLanService = wakeOnLanService; _logger = logger; - _loggerProviders = loggerProviders; + _fileLogsManager = fileLogsManager; } public bool IsConnected => _hubConnection?.State == HubConnectionState.Connected; @@ -303,14 +302,7 @@ private void RegisterMessageHandlers() _hubConnection.On("DeleteLogs", () => { - if (TryGetFileLogger(out var fileLogger)) - { - fileLogger.DeleteLogs(); - } - if (_fileLogger is FileLogger logger) - { - logger.DeleteLogs(); - } + _fileLogsManager.DeleteLogs(); }); @@ -373,26 +365,24 @@ private void RegisterMessageHandlers() _hubConnection.On("GetLogs", async (string senderConnectionId) => { - if (_fileLogger is not FileLogger logger) - { - await _hubConnection.InvokeAsync("SendLogs", "Logger is not of expected type.", senderConnectionId).ConfigureAwait(false); - return; - } - - var logBytes = await logger.ReadAllBytes(); - - if (!logBytes.Any()) + try { - var message = "There are no log entries written."; + if (!await _fileLogsManager.AnyLogsExist()) + { + var message = "There are no log entries written."; + await _hubConnection.InvokeAsync("SendLogs", message, senderConnectionId).ConfigureAwait(false); + return; + } - await _hubConnection.InvokeAsync("SendLogs", message, senderConnectionId).ConfigureAwait(false); - return; + await foreach (var chunk in _fileLogsManager.ReadAllBytes()) + { + var lines = Encoding.UTF8.GetString(chunk); + await _hubConnection.InvokeAsync("SendLogs", lines, senderConnectionId).ConfigureAwait(false); + } } - - for (var i = 0; i < logBytes.Length; i += 50_000) + catch (Exception ex) { - var chunk = Encoding.UTF8.GetString(logBytes.Skip(i).Take(50_000).ToArray()); - await _hubConnection.InvokeAsync("SendLogs", chunk, senderConnectionId).ConfigureAwait(false); + _logger.LogError(ex, "Error while retrieving logs."); } }); @@ -544,25 +534,6 @@ private void RegisterMessageHandlers() }); } - private bool TryGetFileLogger([NotNullWhen(true)] out FileLogger? fileLogger) - { - if (_fileLogger is null) - { - var logger = _loggerProviders - .OfType() - .FirstOrDefault() - ?.CreateLogger(nameof(AgentHubConnection)); - - if (logger is FileLogger loggerImpl) - { - _fileLogger = loggerImpl; - } - } - - fileLogger = _fileLogger; - return fileLogger is not null; - } - private async Task VerifyServer() { if (_connectionInfo is null || _hubConnection is null) diff --git a/Agent/Services/FileLogsManager.cs b/Agent/Services/FileLogsManager.cs new file mode 100644 index 000000000..a9684b631 --- /dev/null +++ b/Agent/Services/FileLogsManager.cs @@ -0,0 +1,85 @@ +using Microsoft.Extensions.Logging; +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Text; +using System.Threading.Tasks; + +namespace Remotely.Shared.Services; + +public interface IFileLogsManager +{ + Task AnyLogsExist(); + Task DeleteLogs(); + IAsyncEnumerable ReadAllBytes(); +} + +public class FileLogsManager : IFileLogsManager +{ + public async Task AnyLogsExist() + { + using var logLock = await FileLoggerDefaults.AcquireLock(); + + var componentName = Assembly.GetExecutingAssembly().GetName().Name; + var directory = Path.Combine(FileLoggerDefaults.LogsFolderPath, $"{componentName}"); + + if (Directory.Exists(directory)) + { + foreach (var file in Directory.GetFiles(directory)) + { + if (new FileInfo(file).Length > 0) + { + return true; + } + } + } + return false; + } + + public async Task DeleteLogs() + { + using var logLock = await FileLoggerDefaults.AcquireLock(); + + var componentName = Assembly.GetExecutingAssembly().GetName().Name; + var directory = Path.Combine(FileLoggerDefaults.LogsFolderPath, $"{componentName}"); + + if (Directory.Exists(directory)) + { + foreach (var file in Directory.GetFiles(directory)) + { + try + { + File.Delete(file); + } + catch { } + } + } + } + + public async IAsyncEnumerable ReadAllBytes() + { + using var logLock = await FileLoggerDefaults.AcquireLock(); + + var componentName = Assembly.GetExecutingAssembly().GetName().Name; + var directory = Path.Combine(FileLoggerDefaults.LogsFolderPath, $"{componentName}"); + + if (!Directory.Exists(directory)) + { + yield break; + } + + var files = Directory + .GetFiles(directory) + .OrderBy(File.GetCreationTime); + + foreach (var file in files) + { + foreach (var chunk in File.ReadAllBytes(file).Chunk(50_000)) + { + yield return File.ReadAllBytes(file); + } + } + } +} diff --git a/Shared/Services/FileLogger.cs b/Shared/Services/FileLogger.cs index 3cc376a97..4f4c10671 100644 --- a/Shared/Services/FileLogger.cs +++ b/Shared/Services/FileLogger.cs @@ -14,7 +14,6 @@ namespace Remotely.Shared.Services; public class FileLogger : ILogger { private static readonly ConcurrentStack _scopeStack = new(); - private static readonly SemaphoreSlim _writeLock = new(1, 1); private readonly string _categoryName; private readonly string _componentName; private readonly string _componentVersion; @@ -27,62 +26,15 @@ public FileLogger(string componentName, string componentVersion, string category _categoryName = categoryName; } - private static string LogsFolderPath - { - get - { - if (OperatingSystem.IsWindows()) - { - var logsPath = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), - "Remotely", - "Logs"); - - if (EnvironmentHelper.IsDebug) - { - logsPath += "_Debug"; - } - return logsPath; - } - - if (OperatingSystem.IsLinux()) - { - if (EnvironmentHelper.IsDebug) - { - return "/var/log/remotely_debug"; - } - return "/var/log/remotely"; - } - - throw new PlatformNotSupportedException(); - } - } - private string LogPath => Path.Combine(LogsFolderPath, _componentName, $"LogFile_{DateTime.Now:yyyy-MM-dd}.log"); + private string LogPath => FileLoggerDefaults.GetLogPath(_componentName); - public IDisposable? BeginScope(TState state) - where TState : notnull + public IDisposable? BeginScope(TState state) + where TState : notnull { _scopeStack.Push($"{state}"); return new NoopDisposable(); } - public void DeleteLogs() - { - try - { - _writeLock.Wait(); - - if (File.Exists(LogPath)) - { - File.Delete(LogPath); - } - } - catch { } - finally - { - _writeLock.Release(); - } - } public bool IsEnabled(LogLevel logLevel) { @@ -93,10 +45,9 @@ public bool IsEnabled(LogLevel logLevel) _ => false, }; } - - public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + public async void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) { - _writeLock.Wait(); + using var logLock = await FileLoggerDefaults.AcquireLock(); try { @@ -110,32 +61,9 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except { Console.WriteLine($"Error writing log entry: {ex.Message}"); } - finally - { - _writeLock.Release(); - } } - public async Task ReadAllBytes() - { - try - { - _writeLock.Wait(); - CheckLogFileExists(); - - return await File.ReadAllBytesAsync(LogPath); - } - catch (Exception ex) - { - this.LogError(ex, "Error while reading all bytes from logs."); - return Array.Empty(); - } - finally - { - _writeLock.Release(); - } - } private void CheckLogFileExists() { diff --git a/Shared/Services/FileLoggerDefaults.cs b/Shared/Services/FileLoggerDefaults.cs new file mode 100644 index 000000000..f4d68d274 --- /dev/null +++ b/Shared/Services/FileLoggerDefaults.cs @@ -0,0 +1,56 @@ +using Immense.RemoteControl.Shared.Primitives; +using Remotely.Shared.Utilities; +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace Remotely.Shared.Services; +public static class FileLoggerDefaults +{ + private static readonly SemaphoreSlim _logLock = new(1, 1); + + public static string LogsFolderPath + { + get + { + if (OperatingSystem.IsWindows()) + { + var logsPath = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + "Remotely", + "Logs"); + + if (EnvironmentHelper.IsDebug) + { + logsPath += "_Debug"; + } + return logsPath; + } + + if (OperatingSystem.IsLinux()) + { + if (EnvironmentHelper.IsDebug) + { + return "/var/log/remotely_debug"; + } + return "/var/log/remotely"; + } + + throw new PlatformNotSupportedException(); + } + } + + public static async Task AcquireLock(CancellationToken cancellationToken = default) + { + await _logLock.WaitAsync(cancellationToken); + return new CallbackDisposable(() => _logLock.Release()); + } + + public static string GetLogPath(string componentName) => + Path.Combine(LogsFolderPath, componentName, $"LogFile_{DateTime.Now:yyyy-MM-dd}.log"); + +}