diff --git a/release_notes.md b/release_notes.md index 1dfa7e41cd..17651b9002 100644 --- a/release_notes.md +++ b/release_notes.md @@ -5,3 +5,4 @@ --> - Update Python Worker Version to [4.40.2](https://github.com/Azure/azure-functions-python-worker/releases/tag/4.40.2) - Add JitTrace Files for v4.1044 +- Send `TelemetryHealthCheckPublisher` logs to ScriptHost `ILogger` (#11398) diff --git a/src/WebJobs.Script.WebHost/DependencyInjection/ServiceProviderExtensions.cs b/src/WebJobs.Script.WebHost/DependencyInjection/ServiceProviderExtensions.cs index 1b2f4e5ddb..7b55cca891 100644 --- a/src/WebJobs.Script.WebHost/DependencyInjection/ServiceProviderExtensions.cs +++ b/src/WebJobs.Script.WebHost/DependencyInjection/ServiceProviderExtensions.cs @@ -7,7 +7,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -20,13 +19,15 @@ public static class ServiceProviderExtensions /// /// Things we don't want to copy down to child containers because... /// - private static readonly HashSet ChildContainerIgnoredTypes = new() - { + private static readonly HashSet ChildContainerIgnoredTypes = + [ typeof(IStartupFilter), // This would re-add middlewares to the host pipeline typeof(IManagedHostedService), // These shouldn't be instantiated twice typeof(IHostedService), // These shouldn't be instantiated twice - typeof(ILoggerProvider), // These shouldn't be instantiated twice - }; + typeof(ILoggerProvider), // These shouldn't be instantiated twice + typeof(ILoggerFactory), // WebHost has a keyed implementation which will fail propagation. ScriptHost registers its own anyways. + typeof(ILogger<>), // Same reason as ILoggerFactory. + ]; /// /// Creates a child container. diff --git a/src/WebJobs.Script.WebHost/Diagnostics/ILoggingBuilderExtensions.cs b/src/WebJobs.Script.WebHost/Diagnostics/ILoggingBuilderExtensions.cs index 811ccc8854..f5539db523 100644 --- a/src/WebJobs.Script.WebHost/Diagnostics/ILoggingBuilderExtensions.cs +++ b/src/WebJobs.Script.WebHost/Diagnostics/ILoggingBuilderExtensions.cs @@ -1,19 +1,23 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. See License.txt in the project root for license information. using Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics; using Microsoft.Extensions.DependencyInjection; +#nullable enable + namespace Microsoft.Extensions.Logging { public static class ILoggingBuilderExtensions { - public static void AddWebJobsSystem(this ILoggingBuilder builder) where T : SystemLoggerProvider + public static ILoggingBuilder AddWebJobsSystem(this ILoggingBuilder builder) + where T : SystemLoggerProvider { builder.Services.AddSingleton(); // Log all logs to SystemLogger builder.AddDefaultWebJobsFilters(LogLevel.Trace); + return builder; } } } diff --git a/src/WebJobs.Script.WebHost/Program.cs b/src/WebJobs.Script.WebHost/Program.cs index b1f9f2dd93..91d3ad6e17 100644 --- a/src/WebJobs.Script.WebHost/Program.cs +++ b/src/WebJobs.Script.WebHost/Program.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. See License.txt in the project root for license information. using System; @@ -88,6 +88,7 @@ public static IWebHostBuilder CreateWebHostBuilder(string[] args = null) loggingBuilder.AddDefaultWebJobsFilters(); loggingBuilder.AddWebJobsSystem(); + loggingBuilder.AddForwardingLogger(); loggingBuilder.Services.AddSingleton(); loggingBuilder.Services.AddSingleton(s => s.GetRequiredService()); loggingBuilder.Services.AddSingleton(); diff --git a/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs b/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs index 4ccee105d1..17ad0cfc55 100644 --- a/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs +++ b/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs @@ -247,6 +247,7 @@ public static void AddWebJobsScriptHost(this IServiceCollection services, IConfi services.AddAzureStorageProviders(); // Add health checks + services.AddMetrics(); services.AddHealthChecks().AddWebJobsScriptHealthChecks(); } diff --git a/src/WebJobs.Script/Diagnostics/ForwardingLogger.cs b/src/WebJobs.Script/Diagnostics/ForwardingLogger.cs new file mode 100644 index 0000000000..90ffc9fcfb --- /dev/null +++ b/src/WebJobs.Script/Diagnostics/ForwardingLogger.cs @@ -0,0 +1,110 @@ +// 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.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Azure.WebJobs.Script; +using Microsoft.Extensions.DependencyInjection; + +#nullable enable + +namespace Microsoft.Extensions.Logging +{ + internal class ForwardingLogger : ILogger + { + // The service key to use for dependency injection to get forwarding loggers. + public const string ServiceKey = "Forwarding"; + + private readonly string _categoryName; + private readonly ILogger _fallback; + private readonly IScriptHostManager _manager; + + // We use weak references so as to not keep a ScriptHost alive after it shuts down. + private readonly WeakReference _current = new(null!); + private readonly WeakReference _services = new(null!); + + public ForwardingLogger(string categoryName, ILogger inner, IScriptHostManager manager) + { + ArgumentNullException.ThrowIfNull(inner); + ArgumentNullException.ThrowIfNull(manager); + _categoryName = categoryName; + _fallback = inner; + _manager = manager; + } + + private ILogger Current + { + get + { + if (TryGetCurrentLogger(out ILogger? logger)) + { + return logger; + } + + // No current ScriptHost logger, or the ScriptHost is gone. Use the fallback WebHost logger. + return _fallback; + } + } + + public IDisposable? BeginScope(TState state) + where TState : notnull + => Current.BeginScope(state); + + public bool IsEnabled(LogLevel logLevel) => Current.IsEnabled(logLevel); + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + => Current.Log(logLevel, eventId, state, exception, formatter); + + private bool TryGetCurrentLogger([NotNullWhen(true)] out ILogger? logger) + { + if (IsLoggerCurrent(out logger)) + { + return true; + } + else if (_manager.Services is { } services) + { + logger = services.GetRequiredService().CreateLogger(_categoryName); + _services.SetTarget(services); + _current.SetTarget(logger); + return true; + } + + logger = null; + return false; + } + + private bool IsLoggerCurrent([NotNullWhen(true)] out ILogger? logger) + { + // First check if the last IServiceProvider we used is still active. + if (_services.TryGetTarget(out IServiceProvider? services) + && ReferenceEquals(services, _manager.Services)) + { + // Service provider is still correct, so our logger is current. + return _current.TryGetTarget(out logger); + } + + logger = null; + return false; + } + } + + [DebuggerDisplay("{_logger}")] + internal class ForwardingLogger : ILogger + { + private readonly ILogger _logger; + + public ForwardingLogger([ForwardingLogger] ILoggerFactory factory) + { + ArgumentNullException.ThrowIfNull(factory); + _logger = factory.CreateLogger(); + } + + IDisposable? ILogger.BeginScope(TState state) => _logger.BeginScope(state); + + bool ILogger.IsEnabled(LogLevel logLevel) => _logger.IsEnabled(logLevel); + + void ILogger.Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => + _logger.Log(logLevel, eventId, state, exception, formatter); + } +} diff --git a/src/WebJobs.Script/Diagnostics/ForwardingLoggerAttribute.cs b/src/WebJobs.Script/Diagnostics/ForwardingLoggerAttribute.cs new file mode 100644 index 0000000000..b2451e88e7 --- /dev/null +++ b/src/WebJobs.Script/Diagnostics/ForwardingLoggerAttribute.cs @@ -0,0 +1,14 @@ +// 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 Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.Extensions.Logging +{ + [AttributeUsage(AttributeTargets.Parameter)] + internal class ForwardingLoggerAttribute() + : FromKeyedServicesAttribute(ForwardingLogger.ServiceKey) + { + } +} diff --git a/src/WebJobs.Script/Diagnostics/ForwardingLoggerFactory.cs b/src/WebJobs.Script/Diagnostics/ForwardingLoggerFactory.cs new file mode 100644 index 0000000000..cad747e378 --- /dev/null +++ b/src/WebJobs.Script/Diagnostics/ForwardingLoggerFactory.cs @@ -0,0 +1,68 @@ +// 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.Collections.Concurrent; +using System.Diagnostics; +using Microsoft.Azure.WebJobs.Script; + +#nullable enable + +namespace Microsoft.Extensions.Logging +{ + /// + /// A logger factory that creates loggers which track the current active ScriptHost (if any), falling + /// back to the WebHost logger if no ScriptHost is active. + /// + [DebuggerDisplay(@"InnerFactory = \{ {_inner} \}, ScriptHostState = {_manager.State}")] + public sealed class ForwardingLoggerFactory : ILoggerFactory + { + private readonly ConcurrentDictionary _loggers = new(StringComparer.Ordinal); + private readonly ILoggerFactory _inner; + private readonly IScriptHostManager _manager; + private readonly object _sync = new(); + + private bool _disposed; + + public ForwardingLoggerFactory(ILoggerFactory inner, IScriptHostManager manager) + { + ArgumentNullException.ThrowIfNull(inner); + ArgumentNullException.ThrowIfNull(manager); + _inner = inner; + _manager = manager; + } + + /// + public void AddProvider(ILoggerProvider provider) + => throw new NotSupportedException( + $"{nameof(ILoggerProvider)} can not be added to the {nameof(ForwardingLoggerFactory)}."); + + /// + public ILogger CreateLogger(string categoryName) + { + ObjectDisposedException.ThrowIf(_disposed, this); + + if (!_loggers.TryGetValue(categoryName, out ForwardingLogger? logger)) + { + lock (_sync) + { + if (!_loggers.TryGetValue(categoryName, out logger)) + { + ILogger innerLogger = _inner.CreateLogger(categoryName); + logger = new ForwardingLogger(categoryName, innerLogger, _manager); + _loggers[categoryName] = logger; + } + } + } + + return logger; + } + + /// + public void Dispose() + { + // this is just to block further logger creation. + _disposed = true; + } + } +} diff --git a/src/WebJobs.Script/Diagnostics/HealthChecks/HealthCheckExtensions.cs b/src/WebJobs.Script/Diagnostics/HealthChecks/HealthCheckExtensions.cs index 8e0b8866dd..745aab55c9 100644 --- a/src/WebJobs.Script/Diagnostics/HealthChecks/HealthCheckExtensions.cs +++ b/src/WebJobs.Script/Diagnostics/HealthChecks/HealthCheckExtensions.cs @@ -6,6 +6,7 @@ using System.Linq; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Logging; namespace Microsoft.Azure.WebJobs.Script.Diagnostics.HealthChecks { @@ -51,8 +52,8 @@ static void RegisterPublisher(IServiceCollection services, string tag) }); } - builder.Services.AddLogging(); builder.Services.AddMetrics(); + builder.Services.AddLogging(b => b.AddForwardingLogger()); builder.Services.AddSingleton(); RegisterPublisher(builder.Services, null); // always register the default publisher diff --git a/src/WebJobs.Script/Diagnostics/HealthChecks/TelemetryHealthCheckPublisher.cs b/src/WebJobs.Script/Diagnostics/HealthChecks/TelemetryHealthCheckPublisher.cs index 9ad1c86d36..1ea88852a8 100644 --- a/src/WebJobs.Script/Diagnostics/HealthChecks/TelemetryHealthCheckPublisher.cs +++ b/src/WebJobs.Script/Diagnostics/HealthChecks/TelemetryHealthCheckPublisher.cs @@ -21,7 +21,7 @@ public partial class TelemetryHealthCheckPublisher : IHealthCheckPublisher public TelemetryHealthCheckPublisher( HealthCheckMetrics metrics, TelemetryHealthCheckPublisherOptions options, - ILogger logger) + [ForwardingLogger] ILogger logger) { _metrics = metrics ?? throw new ArgumentNullException(nameof(metrics)); _options = options ?? throw new ArgumentNullException(nameof(options)); diff --git a/src/WebJobs.Script/Extensions/ScriptLoggingBuilderExtensions.cs b/src/WebJobs.Script/Extensions/ScriptLoggingBuilderExtensions.cs index c72c77b8da..9639feff1f 100644 --- a/src/WebJobs.Script/Extensions/ScriptLoggingBuilderExtensions.cs +++ b/src/WebJobs.Script/Extensions/ScriptLoggingBuilderExtensions.cs @@ -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.Collections.Concurrent; using System.Linq; using Microsoft.AspNetCore.Hosting; @@ -8,13 +9,25 @@ using Microsoft.Azure.WebJobs.Script.Configuration; using Microsoft.Azure.WebJobs.Script.Workers; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; namespace Microsoft.Extensions.Logging { public static class ScriptLoggingBuilderExtensions { - private static ConcurrentDictionary _filteredCategoryCache = new ConcurrentDictionary(); + private static readonly ConcurrentDictionary _filteredCategoryCache = new(); + + public static ILoggingBuilder AddForwardingLogger(this ILoggingBuilder builder) + { + ArgumentNullException.ThrowIfNull(builder); + + builder.Services.TryAddKeyedSingleton( + ForwardingLogger.ServiceKey); + builder.Services.TryAddKeyedSingleton( + typeof(ILogger<>), ForwardingLogger.ServiceKey, typeof(ForwardingLogger<>)); + return builder; + } public static ILoggingBuilder AddDefaultWebJobsFilters(this ILoggingBuilder builder) { @@ -23,7 +36,8 @@ public static ILoggingBuilder AddDefaultWebJobsFilters(this ILoggingBuilder buil return builder; } - public static ILoggingBuilder AddDefaultWebJobsFilters(this ILoggingBuilder builder, LogLevel level) where T : ILoggerProvider + public static ILoggingBuilder AddDefaultWebJobsFilters(this ILoggingBuilder builder, LogLevel level) + where T : ILoggerProvider { builder.AddFilter(null, LogLevel.None); builder.AddFilter((c, l) => Filter(c, l, level)); @@ -37,7 +51,9 @@ internal static bool Filter(string category, LogLevel actualLevel, LogLevel minL private static bool IsFiltered(string category) { - return _filteredCategoryCache.GetOrAdd(category, static cat => ScriptConstants.SystemLogCategoryPrefixes.Any(p => cat.StartsWith(p))); + return _filteredCategoryCache.GetOrAdd( + category, + static cat => ScriptConstants.SystemLogCategoryPrefixes.Any(p => cat.StartsWith(p))); } public static void AddConsoleIfEnabled(this ILoggingBuilder builder, HostBuilderContext context) @@ -45,12 +61,14 @@ public static void AddConsoleIfEnabled(this ILoggingBuilder builder, HostBuilder builder.AddConsoleIfEnabled(context.HostingEnvironment.IsDevelopment(), context.Configuration); } - private static void AddConsoleIfEnabled(this ILoggingBuilder builder, bool isDevelopment, IConfiguration configuration) + private static void AddConsoleIfEnabled( + this ILoggingBuilder builder, bool isDevelopment, IConfiguration configuration) { // console logging defaults to false, except for self host bool enableConsole = isDevelopment; - string consolePath = ConfigurationPath.Combine(ConfigurationSectionNames.JobHost, "Logging", "Console", "IsEnabled"); + string consolePath = ConfigurationPath.Combine( + ConfigurationSectionNames.JobHost, "Logging", "Console", "IsEnabled"); IConfigurationSection configSection = configuration.GetSection(consolePath); if (configSection.Exists()) diff --git a/src/WebJobs.Script/Host/IScriptHostManager.cs b/src/WebJobs.Script/Host/IScriptHostManager.cs index 582368970e..81ded40ca6 100644 --- a/src/WebJobs.Script/Host/IScriptHostManager.cs +++ b/src/WebJobs.Script/Host/IScriptHostManager.cs @@ -5,6 +5,8 @@ using System.Threading; using System.Threading.Tasks; +#nullable enable + namespace Microsoft.Azure.WebJobs.Script { public interface IScriptHostManager @@ -24,7 +26,12 @@ public interface IScriptHostManager /// /// Gets the last host that has occurred. /// - Exception LastError { get; } + Exception? LastError { get; } + + /// + /// Gets the current for the active Script Host. + /// + IServiceProvider? Services { get; } /// /// Restarts the current Script Job Host. diff --git a/test/WebJobs.Script.Tests.Shared/TestHelpers.cs b/test/WebJobs.Script.Tests.Shared/TestHelpers.cs index 7711c8dc80..cd8e84d8dd 100644 --- a/test/WebJobs.Script.Tests.Shared/TestHelpers.cs +++ b/test/WebJobs.Script.Tests.Shared/TestHelpers.cs @@ -654,6 +654,8 @@ event EventHandler IScriptHostManager.HostInitializing Exception IScriptHostManager.LastError => throw new NotImplementedException(); + IServiceProvider IScriptHostManager.Services => this; + public void OnActiveHostChanged() { ActiveHostChanged?.Invoke(this, new ActiveHostChangedEventArgs(null, null)); diff --git a/test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerAttributeTests.cs b/test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerAttributeTests.cs new file mode 100644 index 0000000000..2ad679fb1c --- /dev/null +++ b/test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerAttributeTests.cs @@ -0,0 +1,44 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using AwesomeAssertions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace Microsoft.Azure.WebJobs.Script.Tests.Diagnostics +{ + public class ForwardingLoggerAttributeTests + { + [Fact] + public void Key_IsCorrect() + { + ForwardingLoggerAttribute attribute = new(); + + attribute.Key.Should().BeOfType() + .Which.Should().NotBeNullOrWhiteSpace() + .And.Be(ForwardingLogger.ServiceKey); + } + + [Fact] + public void Import_GetsService() + { + object nonKeyed = new(); + object keyed = new(); + + ServiceCollection services = new(); + services.AddSingleton(nonKeyed); + services.AddKeyedSingleton(ForwardingLogger.ServiceKey, keyed); + services.AddSingleton(); + + TestClass test = services.BuildServiceProvider().GetRequiredService(); + + test.Instance.Should().BeSameAs(keyed); + } + + private class TestClass([ForwardingLogger] object instance) + { + public object Instance => instance; + } + } +} diff --git a/test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerFactoryTests.cs b/test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerFactoryTests.cs new file mode 100644 index 0000000000..b4d72c89ae --- /dev/null +++ b/test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerFactoryTests.cs @@ -0,0 +1,86 @@ +// 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 AwesomeAssertions; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Microsoft.Azure.WebJobs.Script.Tests.Diagnostics +{ + public class ForwardingLoggerFactoryTests + { + private readonly Mock _mockInnerFactory; + private readonly Mock _mockManager; + private readonly Mock _mockInnerLogger; + private readonly ForwardingLoggerFactory _factory; + + public ForwardingLoggerFactoryTests() + { + _mockInnerFactory = new Mock(); + _mockManager = new Mock(); + _mockInnerLogger = new Mock(); + _factory = new ForwardingLoggerFactory(_mockInnerFactory.Object, _mockManager.Object); + } + + [Fact] + public void Constructor_WithNullInnerFactory_ThrowsArgumentNullException() + { + TestHelpers.Act(() => new ForwardingLoggerFactory(null, Mock.Of())) + .Should().Throw() + .WithParameterName("inner"); + } + + [Fact] + public void Constructor_WithNullManager_ThrowsArgumentNullException() + { + TestHelpers.Act(() => new ForwardingLoggerFactory(Mock.Of(), null)) + .Should().Throw() + .WithParameterName("manager"); + } + + [Fact] + public void AddProvider_ThrowsNotSupportedException() + { + TestHelpers.Act(() => _factory.AddProvider(Mock.Of())) + .Should().Throw(); + } + + [Fact] + public void CreateLogger_ReturnsLoggerInstance() + { + string categoryName = "TestCategory"; + _mockInnerFactory.Setup(f => f.CreateLogger(categoryName)).Returns(_mockInnerLogger.Object); + + ILogger result = _factory.CreateLogger(categoryName); + + result.Should().NotBeNull(); + result.Should().BeOfType(); + _mockInnerFactory.Verify(f => f.CreateLogger(categoryName), Times.Once); + } + + [Fact] + public void CreateLogger_Caching_ReturnsSameInstance() + { + string categoryName = "TestCategory"; + _mockInnerFactory.Setup(f => f.CreateLogger(categoryName)).Returns(_mockInnerLogger.Object); + + ILogger result1 = _factory.CreateLogger(categoryName); + ILogger result2 = _factory.CreateLogger(categoryName); + + result1.Should().NotBeNull(); + result1.Should().BeOfType(); + result2.Should().BeSameAs(result1); + _mockInnerFactory.Verify(f => f.CreateLogger(categoryName), Times.Once); + } + + [Fact] + public void Dispose_BlocksCreation() + { + _factory.Dispose(); + TestHelpers.Act(() => _factory.CreateLogger("TestCategory")) + .Should().Throw(); + } + } +} \ No newline at end of file diff --git a/test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerTests.cs b/test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerTests.cs new file mode 100644 index 0000000000..e18655f1a2 --- /dev/null +++ b/test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerTests.cs @@ -0,0 +1,205 @@ +// 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.Threading; +using System.Threading.Tasks; +using AwesomeAssertions; +using AwesomeAssertions.Execution; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; +using Moq; +using Xunit; + +namespace Microsoft.Azure.WebJobs.Script.Tests.Diagnostics +{ + public class ForwardingLoggerTests + { + private const string CategoryName = "TestCategory"; + + private readonly MockScriptHostManager _manager = new(); + private readonly ILogger _webHostLogger; + private readonly ILoggerFactory _webHostLoggerFactory; + private readonly FakeLogCollector _webHostLogCollector; + + public ForwardingLoggerTests() + { + ServiceCollection services = new(); + services.AddFakeLogging(); + IServiceProvider provider = services.BuildServiceProvider(); + _webHostLoggerFactory = provider.GetRequiredService(); + _webHostLogger = _webHostLoggerFactory.CreateLogger(CategoryName); + _webHostLogCollector = provider.GetFakeLogCollector(); + } + + [Fact] + public void Constructor_WithNullInner_ThrowsArgumentNullException() + { + TestHelpers.Act(() => new ForwardingLogger(CategoryName, null!, Mock.Of())) + .Should().Throw() + .WithParameterName("inner"); + } + + [Fact] + public void Constructor_WithNullManager_ThrowsArgumentNullException() + { + TestHelpers.Act(() => new ForwardingLogger(CategoryName, Mock.Of(), null!)) + .Should().Throw() + .WithParameterName("manager"); + } + + [Fact] + public void Log_WithoutScriptHostServices_UsesFallbackLogger() + { + ForwardingLogger forwardingLogger = new(CategoryName, _webHostLogger, _manager); + + forwardingLogger.LogInformation("Web host message"); + + VerifyLog(_webHostLogCollector, LogLevel.Information, "Web host message"); + } + + [Fact] + public void Log_WithScriptHostServices_UsesScriptHostLogger() + { + _manager.Services = CreateProvider(out FakeLogCollector collector); + + ForwardingLogger forwardingLogger = new(CategoryName, _webHostLogger, _manager); + + forwardingLogger.LogWarning("Script host message"); + + _webHostLogCollector.Count.Should().Be(0); + VerifyLog(collector, LogLevel.Warning, "Script host message"); + } + + [Fact] + public void Log_WithChangedScriptHostServices_UpdatesToNewLogger() + { + _manager.Services = CreateProvider(out FakeLogCollector collector); + ForwardingLogger forwardingLogger = new(CategoryName, _webHostLogger, _manager); + + forwardingLogger.LogWarning("Script host message"); + + _webHostLogCollector.Count.Should().Be(0); + VerifyLog(collector, LogLevel.Warning, "Script host message"); + + _manager.Services = CreateProvider(out FakeLogCollector newCollector); + + forwardingLogger.LogError("New script host message"); + + // should not have changed. + _webHostLogCollector.Count.Should().Be(0); + collector.Count.Should().Be(1); + VerifyLog(newCollector, LogLevel.Error, "New script host message"); + } + + [Fact] + public void Log_WithChangedScriptHostServices_ReturnsToFallback() + { + _manager.Services = CreateProvider(out FakeLogCollector collector); + ForwardingLogger forwardingLogger = new(CategoryName, _webHostLogger, _manager); + + forwardingLogger.LogWarning("Script host message"); + + _webHostLogCollector.Count.Should().Be(0); + VerifyLog(collector, LogLevel.Warning, "Script host message"); + + _manager.Services = null; + + forwardingLogger.LogError("New web host message"); + + // should not have changed. + _webHostLogCollector.Count.Should().Be(1); + collector.Count.Should().Be(1); + VerifyLog(_webHostLogCollector, LogLevel.Error, "New web host message"); + } + + [Fact] + public void LoggerT_Constructor_WithNullFactory_ThrowsArgumentNullException() + { + TestHelpers.Act(() => new ForwardingLogger(null!)) + .Should().Throw() + .WithParameterName("factory"); + } + + [Fact] + public void LoggerT_Log_LogsToInner() + { + ForwardingLogger logger = new(_webHostLoggerFactory); + logger.LogCritical("Test message"); + + VerifyLog( + _webHostLogCollector, + LogLevel.Critical, + "Test message", + "object"); + } + + [Fact] + public void EndToEnd_CreatesForwardingLogger() + { + _manager.Services = CreateProvider(out FakeLogCollector collector); + ServiceCollection services = new(); + services.AddSingleton(_manager); + services.AddLogging(b => b.AddForwardingLogger().AddFakeLogging()); + services.AddSingleton(); + + IServiceProvider provider = services.BuildServiceProvider(); + + TestClass test = provider.GetRequiredService(); + test.Logger.LogInformation("Test message"); + + provider.GetFakeLogCollector().Count.Should().Be(0); + + string expectedCategory = typeof(TestClass).FullName.Replace('+', '.'); + VerifyLog(collector, LogLevel.Information, "Test message", expectedCategory); + } + + private static IServiceProvider CreateProvider(out FakeLogCollector collector) + { + ServiceCollection services = new(); + services.AddFakeLogging(); + IServiceProvider serviceProvider = services.BuildServiceProvider(); + collector = serviceProvider.GetFakeLogCollector(); + return serviceProvider; + } + + private static void VerifyLog( + FakeLogCollector collector, LogLevel level, string message, string category = CategoryName) + { + collector.Count.Should().Be(1); + FakeLogRecord record = collector.LatestRecord; + using AssertionScope scope = new("Verifying log record"); + record.Category.Should().Be(category); + record.Level.Should().Be(level); + record.Message.Should().Be(message); + } + + private class MockScriptHostManager : IScriptHostManager + { +#pragma warning disable CS0067 // The event is never used + public event EventHandler HostInitializing; + + public event EventHandler ActiveHostChanged; +#pragma warning restore CS0067 // The event is never used + + public IServiceProvider Services { get; set; } + + public ScriptHostState State => Services is null ? ScriptHostState.Default : ScriptHostState.Running; + + public Exception LastError { get; } + + public void Dispose() { } + + public Task RestartHostAsync(string reason, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + } + + private class TestClass([ForwardingLogger] ILogger logger) + { + public ILogger Logger => logger; + } + } +} diff --git a/test/WebJobs.Script.Tests/Diagnostics/HealthChecks/HealthCheckExtensionsTests.cs b/test/WebJobs.Script.Tests/Diagnostics/HealthChecks/HealthCheckExtensionsTests.cs index 8a0656f446..017dbd366c 100644 --- a/test/WebJobs.Script.Tests/Diagnostics/HealthChecks/HealthCheckExtensionsTests.cs +++ b/test/WebJobs.Script.Tests/Diagnostics/HealthChecks/HealthCheckExtensionsTests.cs @@ -9,6 +9,7 @@ using Microsoft.Azure.WebJobs.Script.Diagnostics.HealthChecks; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Logging; using Moq; using Xunit; @@ -248,6 +249,9 @@ private static void VerifyPublishers(IServiceCollection services, params string[ services.Where(x => x.ServiceType == typeof(IHealthCheckPublisher)).Should().HaveCount(tags.Length) .And.AllSatisfy(x => x.Lifetime.Should().Be(ServiceLifetime.Singleton)); + services.AddLogging(b => b.AddForwardingLogger()); + services.AddMetrics(); + services.AddSingleton(Mock.Of()); ServiceProvider provider = services.BuildServiceProvider(); IEnumerable publishers = provider.GetServices(); diff --git a/test/WebJobs.Script.Tests/Extensions/ScriptLoggingBuilderExtensionsTests.cs b/test/WebJobs.Script.Tests/Extensions/ScriptLoggingBuilderExtensionsTests.cs new file mode 100644 index 0000000000..ccc95fed99 --- /dev/null +++ b/test/WebJobs.Script.Tests/Extensions/ScriptLoggingBuilderExtensionsTests.cs @@ -0,0 +1,47 @@ +// 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 AwesomeAssertions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace Microsoft.Azure.WebJobs.Script.Tests.Extensions +{ + public class ScriptLoggingBuilderExtensionsTests + { + [Fact] + public void AddForwardingLogger_NullBuilder_Throws() + { + TestHelpers.Act(() => ScriptLoggingBuilderExtensions.AddForwardingLogger(null!)) + .Should().Throw() + .WithParameterName("builder"); + } + + [Fact] + public void AddForwardingLogger_AddsServices() + { + ServiceCollection services = new(); + services.AddLogging(b => b.AddForwardingLogger()); + + services.Should().ContainSingle( + s => s.IsKeyedService && s.KeyedImplementationType == typeof(ForwardingLoggerFactory)) + .Which.Should().Satisfy(sd => + { + sd.ServiceType.Should().Be(typeof(ILoggerFactory)); + sd.Lifetime.Should().Be(ServiceLifetime.Singleton); + sd.ServiceKey.Should().Be(ForwardingLogger.ServiceKey); + }); + + services.Should().ContainSingle( + s => s.IsKeyedService && s.KeyedImplementationType == typeof(ForwardingLogger<>)) + .Which.Should().Satisfy(sd => + { + sd.ServiceType.Should().Be(typeof(ILogger<>)); + sd.Lifetime.Should().Be(ServiceLifetime.Singleton); + sd.ServiceKey.Should().Be(ForwardingLogger.ServiceKey); + }); + } + } +} diff --git a/test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs b/test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs index 3e695891bc..08d61958f5 100644 --- a/test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs +++ b/test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs @@ -257,6 +257,8 @@ public TestScriptHostManager(IMetricsLogger metricsLogger) public Exception LastError => throw new NotImplementedException(); + public IServiceProvider Services => this; + public object GetService(Type serviceType) { if (serviceType == typeof(IMetricsLogger))