Rework startup topic handling and reenable output to logging framework (#14243)

Jellyfin.Server/Migrations/JellyfinMigrationService.cs

@@ -47,7 +47,7 @@ internal class JellyfinMigrationService
     public JellyfinMigrationService(
         IDbContextFactory<JellyfinDbContext> dbContextFactory,
         ILoggerFactory loggerFactory,
-        IStartupLogger startupLogger,
+        IStartupLogger<JellyfinMigrationService> startupLogger,
         IApplicationPaths applicationPaths,
         IBackupService? backupService = null,
         IJellyfinDatabaseProvider? jellyfinDatabaseProvider = null)

Jellyfin.Server/Migrations/Routines/MigrateKeyframeData.cs

@@ -35,7 +35,7 @@ public class MigrateKeyframeData : IDatabaseMigrationRoutine
     /// <param name="appPaths">Instance of the <see cref="IApplicationPaths"/> interface.</param>
     /// <param name="dbProvider">The EFCore db factory.</param>
     public MigrateKeyframeData(
-        IStartupLogger startupLogger,
+        IStartupLogger<MigrateKeyframeData> startupLogger,
         IApplicationPaths appPaths,
         IDbContextFactory<JellyfinDbContext> dbProvider)
     {

Jellyfin.Server/Migrations/Routines/MigrateLibraryDb.cs

@@ -48,7 +48,7 @@ internal class MigrateLibraryDb : IDatabaseMigrationRoutine
     /// <param name="paths">The server application paths.</param>
     /// <param name="jellyfinDatabaseProvider">The database provider for special access.</param>
     public MigrateLibraryDb(
-        IStartupLogger startupLogger,
+        IStartupLogger<MigrateLibraryDb> startupLogger,
         IDbContextFactory<JellyfinDbContext> provider,
         IServerApplicationPaths paths,
         IJellyfinDatabaseProvider jellyfinDatabaseProvider)

Jellyfin.Server/Migrations/Routines/MigrateLibraryDbCompatibilityCheck.cs

@@ -26,7 +26,7 @@ public class MigrateLibraryDbCompatibilityCheck : IAsyncMigrationRoutine
     /// </summary>
     /// <param name="startupLogger">The startup logger.</param>
     /// <param name="paths">The Path service.</param>
-    public MigrateLibraryDbCompatibilityCheck(IStartupLogger startupLogger, IServerApplicationPaths paths)
+    public MigrateLibraryDbCompatibilityCheck(IStartupLogger<MigrateLibraryDbCompatibilityCheck> startupLogger, IServerApplicationPaths paths)
     {
         _logger = startupLogger;
         _paths = paths;

Jellyfin.Server/Migrations/Routines/MigrateRatingLevels.cs

@@ -23,7 +23,7 @@ internal class MigrateRatingLevels : IDatabaseMigrationRoutine
 
     public MigrateRatingLevels(
         IDbContextFactory<JellyfinDbContext> provider,
-        IStartupLogger logger,
+        IStartupLogger<MigrateRatingLevels> logger,
         ILocalizationManager localizationManager)
     {
         _provider = provider;

Jellyfin.Server/Migrations/Routines/MoveExtractedFiles.cs

@@ -47,7 +47,7 @@ public class MoveExtractedFiles : IAsyncMigrationRoutine
     public MoveExtractedFiles(
         IApplicationPaths appPaths,
         ILogger<MoveExtractedFiles> logger,
-        IStartupLogger startupLogger,
+        IStartupLogger<MoveExtractedFiles> startupLogger,
         IPathManager pathManager,
         IFileSystem fileSystem,
         IDbContextFactory<JellyfinDbContext> dbProvider)

Jellyfin.Server/Migrations/Routines/MoveTrickplayFiles.cs

@@ -37,7 +37,7 @@ public class MoveTrickplayFiles : IMigrationRoutine
         ITrickplayManager trickplayManager,
         IFileSystem fileSystem,
         ILibraryManager libraryManager,
-        IStartupLogger logger)
+        IStartupLogger<MoveTrickplayFiles> logger)
     {
         _trickplayManager = trickplayManager;
         _fileSystem = fileSystem;

Jellyfin.Server/Migrations/Stages/CodeMigration.cs

@@ -5,6 +5,7 @@ using System.Threading.Tasks;
 using Jellyfin.Server.ServerSetupApp;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Logging;
 
 namespace Jellyfin.Server.Migrations.Stages;
 
@@ -21,11 +22,13 @@ internal class CodeMigration(Type migrationType, JellyfinMigrationAttribute meta
         return Metadata.Order.ToString("yyyyMMddHHmmsss", CultureInfo.InvariantCulture) + "_" + Metadata.Name!;
     }
 
-    private ServiceCollection MigrationServices(IServiceProvider serviceProvider, IStartupLogger logger)
+    private IServiceCollection MigrationServices(IServiceProvider serviceProvider, IStartupLogger logger)
     {
-        var childServiceCollection = new ServiceCollection();
+        var childServiceCollection = new ServiceCollection()
-        childServiceCollection.AddSingleton(serviceProvider);
+            .AddSingleton(serviceProvider)
-        childServiceCollection.AddSingleton(logger);
+            .AddSingleton(logger)
+            .AddSingleton(typeof(IStartupLogger<>), typeof(NestedStartupLogger<>))
+            .AddSingleton<StartupLogTopic>(logger.Topic!);
 
         foreach (ServiceDescriptor service in serviceProvider.GetRequiredService<IServiceCollection>())
         {
@@ -78,4 +81,11 @@ internal class CodeMigration(Type migrationType, JellyfinMigrationAttribute meta
             throw new InvalidOperationException($"The type {MigrationType} does not implement either IMigrationRoutine or IAsyncMigrationRoutine and is not a valid migration type");
         }
     }
+
+    private class NestedStartupLogger<TCategory> : StartupLogger<TCategory>, IStartupLogger<TCategory>
+    {
+        public NestedStartupLogger(ILogger logger, StartupLogTopic topic) : base(logger, topic)
+        {
+        }
+    }
 }

Jellyfin.Server/Program.cs

@@ -60,7 +60,7 @@ namespace Jellyfin.Server
         private static long _startTimestamp;
         private static ILogger _logger = NullLogger.Instance;
         private static bool _restartOnShutdown;
-        private static IStartupLogger? _migrationLogger;
+        private static IStartupLogger<JellyfinMigrationService>? _migrationLogger;
         private static string? _restoreFromBackup;
 
         /// <summary>
@@ -103,6 +103,7 @@ namespace Jellyfin.Server
             _setupServer = new SetupServer(static () => _jellyfinHost?.Services?.GetService<INetworkManager>(), appPaths, static () => _appHost, _loggerFactory, startupConfig);
             await _setupServer.RunAsync().ConfigureAwait(false);
             _logger = _loggerFactory.CreateLogger("Main");
+            StartupLogger.Logger = new StartupLogger(_logger);
 
             // Use the logging framework for uncaught exceptions instead of std error
             AppDomain.CurrentDomain.UnhandledException += (_, e)
@@ -178,7 +179,9 @@ namespace Jellyfin.Server
                     })
                     .ConfigureAppConfiguration(config => config.ConfigureAppConfiguration(options, appPaths, startupConfig))
                     .UseSerilog()
-                    .ConfigureServices(e => e.AddTransient<IStartupLogger, StartupLogger>().AddSingleton<IServiceCollection>(e))
+                    .ConfigureServices(e => e
+                        .RegisterStartupLogger()
+                        .AddSingleton<IServiceCollection>(e))
                     .Build();
 
                 // Re-use the host service provider in the app host since ASP.NET doesn't allow a custom service collection.
@@ -268,7 +271,7 @@ namespace Jellyfin.Server
         /// <returns>A task.</returns>
         public static async Task ApplyStartupMigrationAsync(ServerApplicationPaths appPaths, IConfiguration startupConfig)
         {
-            _migrationLogger = StartupLogger.Logger.BeginGroup($"Migration Service");
+            _migrationLogger = StartupLogger.Logger.BeginGroup<JellyfinMigrationService>($"Migration Service");
             var startupConfigurationManager = new ServerConfigurationManager(appPaths, _loggerFactory, new MyXmlSerializer());
             startupConfigurationManager.AddParts([new DatabaseConfigurationFactory()]);
             var migrationStartupServiceProvider = new ServiceCollection()
@@ -276,7 +279,7 @@ namespace Jellyfin.Server
                 .AddJellyfinDbContext(startupConfigurationManager, startupConfig)
                 .AddSingleton<IApplicationPaths>(appPaths)
                 .AddSingleton<ServerApplicationPaths>(appPaths)
-                .AddSingleton<IStartupLogger>(_migrationLogger);
+                .RegisterStartupLogger();
 
             migrationStartupServiceProvider.AddSingleton(migrationStartupServiceProvider);
             var startupService = migrationStartupServiceProvider.BuildServiceProvider();

Jellyfin.Server/ServerSetupApp/IStartupLogger.cs

@@ -1,5 +1,4 @@
 using System;
-using Morestachio.Helper.Logging;
 using ILogger = Microsoft.Extensions.Logging.ILogger;
 
 namespace Jellyfin.Server.ServerSetupApp;
@@ -9,6 +8,11 @@ namespace Jellyfin.Server.ServerSetupApp;
 /// </summary>
 public interface IStartupLogger : ILogger
 {
+    /// <summary>
+    /// Gets the topic this logger is assigned to.
+    /// </summary>
+    StartupLogTopic? Topic { get; }
+
     /// <summary>
     /// Adds another logger instance to this logger for combined logging.
     /// </summary>
@@ -22,4 +26,41 @@ public interface IStartupLogger : ILogger
     /// <param name="logEntry">Defines the log message that introduces the new group.</param>
     /// <returns>A new logger that can write to the group.</returns>
     IStartupLogger BeginGroup(FormattableString logEntry);
+
+    /// <summary>
+    /// Adds another logger instance to this logger for combined logging.
+    /// </summary>
+    /// <param name="logger">Other logger to rely messages to.</param>
+    /// <returns>A combined logger.</returns>
+    /// <typeparam name="TCategory">The logger cateogry.</typeparam>
+    IStartupLogger<TCategory> With<TCategory>(ILogger logger);
+
+    /// <summary>
+    /// Opens a new Group logger within the parent logger.
+    /// </summary>
+    /// <param name="logEntry">Defines the log message that introduces the new group.</param>
+    /// <returns>A new logger that can write to the group.</returns>
+    /// <typeparam name="TCategory">The logger cateogry.</typeparam>
+    IStartupLogger<TCategory> BeginGroup<TCategory>(FormattableString logEntry);
+}
+
+/// <summary>
+/// Defines a logger that can be injected via DI to get a startup logger initialised with an logger framework connected <see cref="ILogger"/>.
+/// </summary>
+/// <typeparam name="TCategory">The logger cateogry.</typeparam>
+public interface IStartupLogger<TCategory> : IStartupLogger
+{
+    /// <summary>
+    /// Adds another logger instance to this logger for combined logging.
+    /// </summary>
+    /// <param name="logger">Other logger to rely messages to.</param>
+    /// <returns>A combined logger.</returns>
+    new IStartupLogger<TCategory> With(ILogger logger);
+
+    /// <summary>
+    /// Opens a new Group logger within the parent logger.
+    /// </summary>
+    /// <param name="logEntry">Defines the log message that introduces the new group.</param>
+    /// <returns>A new logger that can write to the group.</returns>
+    new IStartupLogger<TCategory> BeginGroup(FormattableString logEntry);
 }

Jellyfin.Server/ServerSetupApp/SetupServer.cs

@@ -71,7 +71,7 @@ public sealed class SetupServer : IDisposable
         _configurationManager.RegisterConfiguration<NetworkConfigurationFactory>();
     }
 
-    internal static ConcurrentQueue<StartupLogEntry>? LogQueue { get; set; } = new();
+    internal static ConcurrentQueue<StartupLogTopic>? LogQueue { get; set; } = new();
 
     /// <summary>
     /// Gets a value indicating whether Startup server is currently running.
@@ -88,12 +88,12 @@ public sealed class SetupServer : IDisposable
         _startupUiRenderer = (await ParserOptionsBuilder.New()
             .WithTemplate(fileTemplate)
             .WithFormatter(
-                (StartupLogEntry logEntry, IEnumerable<StartupLogEntry> children) =>
+                (StartupLogTopic logEntry, IEnumerable<StartupLogTopic> children) =>
                 {
                     if (children.Any())
                     {
                         var maxLevel = logEntry.LogLevel;
-                        var stack = new Stack<StartupLogEntry>(children);
+                        var stack = new Stack<StartupLogTopic>(children);
 
                         while (maxLevel != LogLevel.Error && stack.Count > 0 && (logEntry = stack.Pop()) != null) // error is the highest inherted error level.
                         {
@@ -362,15 +362,4 @@ public sealed class SetupServer : IDisposable
             });
         }
     }
-
-    internal class StartupLogEntry
-    {
-        public LogLevel LogLevel { get; set; }
-
-        public string? Content { get; set; }
-
-        public DateTimeOffset DateOfCreation { get; set; }
-
-        public List<StartupLogEntry> Children { get; set; } = [];
-    }
 }

Jellyfin.Server/ServerSetupApp/StartupLogTopic.cs

@@ -0,0 +1,31 @@
+using System;
+using System.Collections.ObjectModel;
+using Microsoft.Extensions.Logging;
+
+namespace Jellyfin.Server.ServerSetupApp;
+
+/// <summary>
+/// Defines a topic for the Startup UI.
+/// </summary>
+public class StartupLogTopic
+{
+    /// <summary>
+    /// Gets or Sets the LogLevel.
+    /// </summary>
+    public LogLevel LogLevel { get; set; }
+
+    /// <summary>
+    /// Gets or Sets the descriptor for the topic.
+    /// </summary>
+    public string? Content { get; set; }
+
+    /// <summary>
+    /// Gets or sets the time the topic was created.
+    /// </summary>
+    public DateTimeOffset DateOfCreation { get; set; }
+
+    /// <summary>
+    /// Gets the child items of this topic.
+    /// </summary>
+    public Collection<StartupLogTopic> Children { get; } = [];
+}

Jellyfin.Server/ServerSetupApp/StartupLogger.cs

@@ -1,56 +1,86 @@
 using System;
-using System.Collections.Generic;
 using System.Globalization;
-using System.Linq;
-using Jellyfin.Server.Migrations.Routines;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
 
 namespace Jellyfin.Server.ServerSetupApp;
 
 /// <inheritdoc/>
 public class StartupLogger : IStartupLogger
 {
-    private readonly SetupServer.StartupLogEntry? _groupEntry;
+    private readonly StartupLogTopic? _topic;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="StartupLogger"/> class.
     /// </summary>
-    public StartupLogger()
+    /// <param name="logger">The underlying base logger.</param>
+    public StartupLogger(ILogger logger)
     {
-        Loggers = [];
+        BaseLogger = logger;
     }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="StartupLogger"/> class.
     /// </summary>
-    private StartupLogger(SetupServer.StartupLogEntry? groupEntry) : this()
+    /// <param name="logger">The underlying base logger.</param>
+    /// <param name="topic">The group for this logger.</param>
+    internal StartupLogger(ILogger logger, StartupLogTopic? topic) : this(logger)
     {
-        _groupEntry = groupEntry;
+        _topic = topic;
     }
 
-    internal static IStartupLogger Logger { get; } = new StartupLogger();
+    internal static IStartupLogger Logger { get; set; } = new StartupLogger(NullLogger.Instance);
 
-    private List<ILogger> Loggers { get; set; }
+    /// <inheritdoc/>
+    public StartupLogTopic? Topic => _topic;
+
+    /// <summary>
+    /// Gets or Sets the underlying base logger.
+    /// </summary>
+    protected ILogger BaseLogger { get; set; }
 
     /// <inheritdoc/>
     public IStartupLogger BeginGroup(FormattableString logEntry)
     {
-        var startupEntry = new SetupServer.StartupLogEntry()
+        return new StartupLogger(BaseLogger, AddToTopic(logEntry));
+    }
+
+    /// <inheritdoc/>
+    public IStartupLogger With(ILogger logger)
+    {
+        return new StartupLogger(logger, Topic);
+    }
+
+    /// <inheritdoc/>
+    public IStartupLogger<TCategory> With<TCategory>(ILogger logger)
+    {
+        return new StartupLogger<TCategory>(logger, Topic);
+    }
+
+    /// <inheritdoc/>
+    public IStartupLogger<TCategory> BeginGroup<TCategory>(FormattableString logEntry)
+    {
+        return new StartupLogger<TCategory>(BaseLogger, AddToTopic(logEntry));
+    }
+
+    private StartupLogTopic AddToTopic(FormattableString logEntry)
+    {
+        var startupEntry = new StartupLogTopic()
         {
             Content = logEntry.ToString(CultureInfo.InvariantCulture),
             DateOfCreation = DateTimeOffset.Now
         };
 
-        if (_groupEntry is null)
+        if (Topic is null)
         {
             SetupServer.LogQueue?.Enqueue(startupEntry);
         }
         else
         {
-            _groupEntry.Children.Add(startupEntry);
+            Topic.Children.Add(startupEntry);
         }
 
-        return new StartupLogger(startupEntry);
+        return startupEntry;
     }
 
     /// <inheritdoc/>
@@ -69,34 +99,26 @@ public class StartupLogger : IStartupLogger
     /// <inheritdoc/>
     public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
     {
-        foreach (var item in Loggers.Where(e => e.IsEnabled(logLevel)))
+        if (BaseLogger.IsEnabled(logLevel))
         {
-            item.Log(logLevel, eventId, state, exception, formatter);
+            // if enabled allow the base logger also to receive the message
+            BaseLogger.Log(logLevel, eventId, state, exception, formatter);
         }
 
-        var startupEntry = new SetupServer.StartupLogEntry()
+        var startupEntry = new StartupLogTopic()
         {
             LogLevel = logLevel,
             Content = formatter(state, exception),
             DateOfCreation = DateTimeOffset.Now
         };
 
-        if (_groupEntry is null)
+        if (Topic is null)
         {
             SetupServer.LogQueue?.Enqueue(startupEntry);
         }
         else
         {
-            _groupEntry.Children.Add(startupEntry);
+            Topic.Children.Add(startupEntry);
         }
     }
-
-    /// <inheritdoc/>
-    public IStartupLogger With(ILogger logger)
-    {
-        return new StartupLogger(_groupEntry)
-        {
-            Loggers = [.. Loggers, logger]
-        };
-    }
 }

Jellyfin.Server/ServerSetupApp/StartupLoggerExtensions.cs

@@ -0,0 +1,18 @@
+using System;
+using System.Globalization;
+using System.Linq;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
+
+namespace Jellyfin.Server.ServerSetupApp;
+
+internal static class StartupLoggerExtensions
+{
+    public static IServiceCollection RegisterStartupLogger(this IServiceCollection services)
+    {
+        return services
+            .AddTransient<IStartupLogger, StartupLogger<Startup>>()
+            .AddTransient(typeof(IStartupLogger<>), typeof(StartupLogger<>));
+    }
+}

Jellyfin.Server/ServerSetupApp/StartupLoggerOfCategory.cs

@@ -0,0 +1,56 @@
+using System;
+using System.Globalization;
+using Microsoft.Extensions.Logging;
+
+namespace Jellyfin.Server.ServerSetupApp;
+
+/// <summary>
+/// Startup logger for usage with DI that utilises an underlying logger from the DI.
+/// </summary>
+/// <typeparam name="TCategory">The category of the underlying logger.</typeparam>
+#pragma warning disable SA1649 // File name should match first type name
+public class StartupLogger<TCategory> : StartupLogger, IStartupLogger<TCategory>
+#pragma warning restore SA1649 // File name should match first type name
+{
+    /// <summary>
+    /// Initializes a new instance of the <see cref="StartupLogger{TCategory}"/> class.
+    /// </summary>
+    /// <param name="logger">The injected base logger.</param>
+    public StartupLogger(ILogger<TCategory> logger) : base(logger)
+    {
+    }
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="StartupLogger{TCategory}"/> class.
+    /// </summary>
+    /// <param name="logger">The underlying base logger.</param>
+    /// <param name="groupEntry">The group for this logger.</param>
+    internal StartupLogger(ILogger logger, StartupLogTopic? groupEntry) : base(logger, groupEntry)
+    {
+    }
+
+    IStartupLogger<TCategory> IStartupLogger<TCategory>.BeginGroup(FormattableString logEntry)
+    {
+        var startupEntry = new StartupLogTopic()
+        {
+            Content = logEntry.ToString(CultureInfo.InvariantCulture),
+            DateOfCreation = DateTimeOffset.Now
+        };
+
+        if (Topic is null)
+        {
+            SetupServer.LogQueue?.Enqueue(startupEntry);
+        }
+        else
+        {
+            Topic.Children.Add(startupEntry);
+        }
+
+        return new StartupLogger<TCategory>(BaseLogger, startupEntry);
+    }
+
+    IStartupLogger<TCategory> IStartupLogger<TCategory>.With(ILogger logger)
+    {
+        return new StartupLogger<TCategory>(logger, Topic);
+    }
+}

tests/Jellyfin.Server.Integration.Tests/JellyfinApplicationFactory.cs

@@ -98,7 +98,10 @@ namespace Jellyfin.Server.Integration.Tests
                         .AddEnvironmentVariables("JELLYFIN_")
                         .AddInMemoryCollection(commandLineOpts.ConvertToConfig());
                 })
-                .ConfigureServices(e => e.AddSingleton<IStartupLogger, NullStartupLogger>().AddSingleton(e));
+                .ConfigureServices(e => e
+                    .AddSingleton<IStartupLogger, NullStartupLogger<object>>()
+                    .AddTransient(typeof(IStartupLogger<>), typeof(NullStartupLogger<>))
+                    .AddSingleton(e));
         }
 
         /// <inheritdoc/>
@@ -132,13 +135,20 @@ namespace Jellyfin.Server.Integration.Tests
             base.Dispose(disposing);
         }
 
-        private sealed class NullStartupLogger : IStartupLogger
+        private sealed class NullStartupLogger<TCategory> : IStartupLogger<TCategory>
         {
+            public StartupLogTopic? Topic => throw new NotImplementedException();
+
             public IStartupLogger BeginGroup(FormattableString logEntry)
             {
                 return this;
             }
 
+            public IStartupLogger<TCategory1> BeginGroup<TCategory1>(FormattableString logEntry)
+            {
+                return new NullStartupLogger<TCategory1>();
+            }
+
             public IDisposable? BeginScope<TState>(TState state)
                 where TState : notnull
             {
@@ -160,10 +170,25 @@ namespace Jellyfin.Server.Integration.Tests
                 return this;
             }
 
+            public IStartupLogger<TCategory1> With<TCategory1>(Microsoft.Extensions.Logging.ILogger logger)
+            {
+                return new NullStartupLogger<TCategory1>();
+            }
+
+            IStartupLogger<TCategory> IStartupLogger<TCategory>.BeginGroup(FormattableString logEntry)
+            {
+                return new NullStartupLogger<TCategory>();
+            }
+
             IStartupLogger IStartupLogger.With(Microsoft.Extensions.Logging.ILogger logger)
             {
                 return this;
             }
+
+            IStartupLogger<TCategory> IStartupLogger<TCategory>.With(Microsoft.Extensions.Logging.ILogger logger)
+            {
+                return this;
+            }
         }
     }
 }