From 894a3d9d7a31110484023cc8d132eaea3640c001 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Fri, 2 Jun 2017 08:33:20 -0700 Subject: [PATCH 1/2] React to logging DI changes (#56) --- .../Startup.cs | 8 +-- .../ApplicationInsightsLoggerConfiguration.cs | 72 ------------------- .../ApplicationInsightsLoggerStartupFilter.cs | 25 ++----- .../ApplicationInsightsStartupLoader.cs | 44 +++++++++++- .../AppServicesWebHostBuilderExtensions.cs | 2 +- .../LoggingTest.cs | 51 ++----------- ...AppServicesWebHostBuilderExtensionsTest.cs | 3 +- 7 files changed, 58 insertions(+), 147 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsLoggerConfiguration.cs diff --git a/sample/ApplicationInsightsHostingStartupSample/Startup.cs b/sample/ApplicationInsightsHostingStartupSample/Startup.cs index 2231d5e14c..f34a884340 100644 --- a/sample/ApplicationInsightsHostingStartupSample/Startup.cs +++ b/sample/ApplicationInsightsHostingStartupSample/Startup.cs @@ -79,13 +79,9 @@ namespace IISSample .Build(); var host = new WebHostBuilder() - .ConfigureLogging((hostingContext, factory) => + .ConfigureLogging((hostingContext, builder) => { - if (hostingContext.Configuration["WIRE_LOGGING_CONFIGURATION"]?.ToLowerInvariant() != "false") - { - factory.UseConfiguration(hostingContext.Configuration.GetSection("Logging")); - } - factory.AddConsole(); + builder.AddConsole(); }) .UseKestrel() .UseStartup() diff --git a/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsLoggerConfiguration.cs b/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsLoggerConfiguration.cs deleted file mode 100644 index f34dfc8671..0000000000 --- a/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsLoggerConfiguration.cs +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.IO; -using System.Linq; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.Logging; - -namespace Microsoft.AspNetCore.ApplicationInsights.HostingStartup -{ - internal class ApplicationInsightsLoggerConfiguration - { - private const string ApplicationInsightsLoggerFactory = "Microsoft.ApplicationInsights.AspNetCore.Logging.ApplicationInsightsLoggerProvider"; - private const string ApplicationInsightsLoggerLevelSection = "Logging:" + ApplicationInsightsLoggerFactory + ":LogLevel"; - private const string ApplicationInsightsSettingsFile = "ApplicationInsights.settings.json"; - - private static readonly KeyValuePair[] _defaultLoggingLevels = { - new KeyValuePair("Microsoft", LogLevel.Warning), - new KeyValuePair("System", LogLevel.Warning), - new KeyValuePair(null, LogLevel.Information) - }; - - public static void ConfigureLogging(IConfigurationBuilder configurationBuilder) - { - // Skip adding default rules when debugger is attached - // we want to send all events to VS - if (!Debugger.IsAttached) - { - configurationBuilder.AddInMemoryCollection(GetDefaultLoggingSettings()); - } - - var home = Environment.GetEnvironmentVariable("HOME"); - if (!string.IsNullOrEmpty(home)) - { - var settingsFile = Path.Combine(home, "site", "diagnostics", ApplicationInsightsSettingsFile); - configurationBuilder.AddJsonFile(settingsFile, optional: true); - } - } - - public static bool ApplyDefaultFilter(string name, LogLevel level) - { - foreach (var pair in _defaultLoggingLevels) - { - // Default is null - if (pair.Key == null || name.StartsWith(pair.Key, StringComparison.Ordinal)) - { - return level >= pair.Value; - } - } - - return false; - } - - public static bool HasLoggingConfigured(IConfiguration configuration) - { - return configuration?.GetSection(ApplicationInsightsLoggerLevelSection) != null; - } - - private static KeyValuePair[] GetDefaultLoggingSettings() - { - return _defaultLoggingLevels.Select(pair => - { - var key = pair.Key ?? "Default"; - var optionsKey = $"{ApplicationInsightsLoggerLevelSection}:{key}"; - return new KeyValuePair(optionsKey, pair.Value.ToString()); - }).ToArray(); - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsLoggerStartupFilter.cs b/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsLoggerStartupFilter.cs index f84a020861..47eb9a2549 100644 --- a/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsLoggerStartupFilter.cs +++ b/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsLoggerStartupFilter.cs @@ -21,25 +21,12 @@ namespace Microsoft.AspNetCore.ApplicationInsights.HostingStartup var loggerEnabled = true; Action disableCallback = () => loggerEnabled = false; - if (loggerFactory is LoggerFactory stronglyTypedLoggerFactory && - ApplicationInsightsLoggerConfiguration.HasLoggingConfigured(stronglyTypedLoggerFactory.Configuration)) - { - // We detected that logger settings got to LoggerFactory configuration and - // defaults would be applied - loggerFactory.AddApplicationInsights( - builder.ApplicationServices, - (s, level) => loggerEnabled, - disableCallback); - } - else - { - // It's not AspNetCore LoggerFactory or configuration was not wired - // just add a logger with default settings - loggerFactory.AddApplicationInsights( - builder.ApplicationServices, - (s, level) => loggerEnabled && ApplicationInsightsLoggerConfiguration.ApplyDefaultFilter(s, level), - disableCallback); - } + // We detected that logger settings got to LoggerFactory configuration and + // defaults would be applied + loggerFactory.AddApplicationInsights( + builder.ApplicationServices, + (s, level) => loggerEnabled, + disableCallback); next(builder); }; diff --git a/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsStartupLoader.cs b/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsStartupLoader.cs index 75357e1de4..e2d4d4d199 100644 --- a/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsStartupLoader.cs +++ b/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsStartupLoader.cs @@ -1,9 +1,17 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Razor.TagHelpers; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; [assembly: HostingStartup(typeof(Microsoft.AspNetCore.ApplicationInsights.HostingStartup.ApplicationInsightsHostingStartup))] @@ -17,6 +25,7 @@ namespace Microsoft.AspNetCore.ApplicationInsights.HostingStartup /// public class ApplicationInsightsHostingStartup : IHostingStartup { + private const string ApplicationInsightsSettingsFile = "ApplicationInsights.settings.json"; /// /// Calls UseApplicationInsights @@ -24,7 +33,6 @@ namespace Microsoft.AspNetCore.ApplicationInsights.HostingStartup /// public void Configure(IWebHostBuilder builder) { - builder.ConfigureAppConfiguration((context, configurationBuilder) => ApplicationInsightsLoggerConfiguration.ConfigureLogging(configurationBuilder)); builder.UseApplicationInsights(); builder.ConfigureServices(InitializeServices); @@ -37,8 +45,42 @@ namespace Microsoft.AspNetCore.ApplicationInsights.HostingStartup /// The associated with the application. private void InitializeServices(IServiceCollection services) { + services.AddSingleton, DefaultApplicationInsightsLoggerFilters>(); services.AddSingleton(); services.AddSingleton(); + + var home = Environment.GetEnvironmentVariable("HOME"); + if (!string.IsNullOrEmpty(home)) + { + var settingsFile = Path.Combine(home, "site", "diagnostics", ApplicationInsightsSettingsFile); + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddJsonFile(settingsFile, optional: true); + services.AddLogging(builder => builder.AddConfiguration(configurationBuilder.Build().GetSection("Logging"), replace: true)); + } + } + } + + internal class DefaultApplicationInsightsLoggerFilters : IConfigureOptions + { + private const string ApplicationInsightsLoggerFactory = "Microsoft.ApplicationInsights.AspNetCore.Logging.ApplicationInsightsLoggerProvider"; + + private static readonly KeyValuePair[] _defaultLoggingLevels = { + new KeyValuePair("Microsoft", LogLevel.Warning), + new KeyValuePair("System", LogLevel.Warning), + new KeyValuePair(null, LogLevel.Information) + }; + + public void Configure(LoggerFilterOptions options) + { + foreach (var pair in _defaultLoggingLevels) + { + options.Rules.Add(new LoggerFilterRule( + ApplicationInsightsLoggerFactory, + pair.Key, + null, + (type, name, level) => Debugger.IsAttached || level >= pair.Value + )); + } } } } diff --git a/src/Microsoft.AspNetCore.AzureAppServicesIntegration/AppServicesWebHostBuilderExtensions.cs b/src/Microsoft.AspNetCore.AzureAppServicesIntegration/AppServicesWebHostBuilderExtensions.cs index 6bad6976c8..09a51bf56d 100644 --- a/src/Microsoft.AspNetCore.AzureAppServicesIntegration/AppServicesWebHostBuilderExtensions.cs +++ b/src/Microsoft.AspNetCore.AzureAppServicesIntegration/AppServicesWebHostBuilderExtensions.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Hosting throw new ArgumentNullException(nameof(hostBuilder)); } #pragma warning disable 618 - hostBuilder.ConfigureLogging(loggerFactory => loggerFactory.AddAzureWebAppDiagnostics()); + hostBuilder.ConfigureLogging(builder => builder.AddAzureWebAppDiagnostics()); #pragma warning restore 618 return hostBuilder; } diff --git a/test/ApplicationInsights.HostingStartup.Tests/LoggingTest.cs b/test/ApplicationInsights.HostingStartup.Tests/LoggingTest.cs index 8ea857bb06..336149aec9 100644 --- a/test/ApplicationInsights.HostingStartup.Tests/LoggingTest.cs +++ b/test/ApplicationInsights.HostingStartup.Tests/LoggingTest.cs @@ -22,25 +22,16 @@ namespace ApplicationInsightsJavaScriptSnippetTest [InlineData(ApplicationType.Standalone)] public async Task DefaultAILogFiltersApplied(ApplicationType applicationType) { - var responseText = await RunRequest(applicationType, "DefaultLogging", true); + var responseText = await RunRequest(applicationType, "DefaultLogging"); AssertDefaultLogs(responseText); } - [Theory] - [InlineData(ApplicationType.Portable)] - [InlineData(ApplicationType.Standalone)] - public async Task DefaultAILogFiltersAppliedWithoutConfiguration(ApplicationType applicationType) - { - var responseText = await RunRequest(applicationType, "DefaultLogging", false); - AssertDefaultNoConfigurationLogs(responseText); - } - [Theory] [InlineData(ApplicationType.Portable)] [InlineData(ApplicationType.Standalone)] public async Task CustomAILogFiltersApplied(ApplicationType applicationType) { - var responseText = await RunRequest(applicationType, "CustomLogging", true); + var responseText = await RunRequest(applicationType, "CustomLogging"); AssertCustomLogs(responseText); } @@ -75,37 +66,6 @@ namespace ApplicationInsightsJavaScriptSnippetTest Assert.Contains("Specific trace log", responseText); } - private static void AssertDefaultNoConfigurationLogs(string responseText) - { - // Enabled by default - Assert.Contains("System warning log", responseText); - // Disabled by default - Assert.DoesNotContain("System information log", responseText); - // Disabled by default - Assert.DoesNotContain("System trace log", responseText); - - // Enabled by default - Assert.Contains("Microsoft warning log", responseText); - // Disabled by default - Assert.DoesNotContain("Microsoft information log", responseText); - // Disabled by default - Assert.DoesNotContain("Microsoft trace log", responseText); - - // Enabled by default - Assert.Contains("Custom warning log", responseText); - // Enabled by default - Assert.Contains("Custom information log", responseText); - // Disabled by default - Assert.DoesNotContain("Custom trace log", responseText); - - // Enabled by default - Assert.Contains("Specific warning log", responseText); - // Enabled by default - Assert.Contains("Specific information log", responseText); - // Disabled by default - Assert.DoesNotContain("Specific trace log", responseText); - } - private static void AssertCustomLogs(string responseText) { // Custom logger allows only namespaces with 'o' in the name @@ -130,7 +90,7 @@ namespace ApplicationInsightsJavaScriptSnippetTest Assert.DoesNotContain("Specific trace log", responseText); } - private async Task RunRequest(ApplicationType applicationType, string environment, bool wireConfiguration) + private async Task RunRequest(ApplicationType applicationType, string environment) { string responseText; var testName = $"ApplicationInsightsLoggingTest_{applicationType}"; @@ -153,10 +113,7 @@ namespace ApplicationInsightsJavaScriptSnippetTest "Microsoft.AspNetCore.ApplicationInsights.HostingStartup"), new KeyValuePair( "HOME", - Path.Combine(GetApplicationPath(), "home")), - new KeyValuePair( - "ASPNETCORE_WIRE_LOGGING_CONFIGURATION", - wireConfiguration.ToString()), + Path.Combine(GetApplicationPath(), "home")) }, }; diff --git a/test/Microsoft.AspNetCore.AzureAppServicesIntegration.Tests/AppServicesWebHostBuilderExtensionsTest.cs b/test/Microsoft.AspNetCore.AzureAppServicesIntegration.Tests/AppServicesWebHostBuilderExtensionsTest.cs index 7ce1abd03a..d3aa47bba5 100644 --- a/test/Microsoft.AspNetCore.AzureAppServicesIntegration.Tests/AppServicesWebHostBuilderExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.AzureAppServicesIntegration.Tests/AppServicesWebHostBuilderExtensionsTest.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Moq; using Xunit; @@ -17,7 +18,7 @@ namespace Microsoft.AspNetCore.Hosting.Azure.AppServices.Tests mock.Object.UseAzureAppServices(); - mock.Verify(builder => builder.ConfigureLogging(It.IsNotNull>()), Times.Once); + mock.Verify(builder => builder.ConfigureServices(It.IsNotNull>()), Times.Once); } } } From c381d8715afe1e8b17bbc501539693b5964431d6 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Fri, 2 Jun 2017 09:37:18 -0700 Subject: [PATCH 2/2] React to AddConfiguration overload change --- .../ApplicationInsightsStartupLoader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsStartupLoader.cs b/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsStartupLoader.cs index e2d4d4d199..6211ae3610 100644 --- a/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsStartupLoader.cs +++ b/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsStartupLoader.cs @@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.ApplicationInsights.HostingStartup var settingsFile = Path.Combine(home, "site", "diagnostics", ApplicationInsightsSettingsFile); var configurationBuilder = new ConfigurationBuilder(); configurationBuilder.AddJsonFile(settingsFile, optional: true); - services.AddLogging(builder => builder.AddConfiguration(configurationBuilder.Build().GetSection("Logging"), replace: true)); + services.AddLogging(builder => builder.AddConfiguration(configurationBuilder.Build().GetSection("Logging"))); } } }