Enable ILogger by default (#853)

Enable ILogger by default when enabling application insights monitoring.
microsoft · Mar 29, 2019 · 04ec946 · 04ec946
1 parent 27181c9
commit 04ec946
Show file tree

Hide file tree

Showing 16 changed files with 253 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # Changelog
 
 ## Version 2.7.0-beta3
+- [Enables Microsoft.Extensions.Logging.ApplicationInsights.ApplicationInsightsLoggerProvider by default. If ApplicationInsightsLoggerProvider was enabled previously using ILoggerFactory extension method, please remove it to prevent duplicate logs.](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/854)
 - [Remove reference to Microsoft.Extensions.DiagnosticAdapter and use DiagnosticSource subscription APIs directly](https://github.com/Microsoft/ApplicationInsights-aspnetcore/pull/852) 
 - [Fix: NullReferenceException in ApplicationInsightsLogger.Log when exception contains a Data entry with a null value](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/848)
 

diff --git a/Readme.md b/Readme.md
@@ -3,6 +3,10 @@
 - [Microsoft.ApplicationInsights.AspNetCore](https://www.nuget.org/packages/Microsoft.ApplicationInsights.AspNetCore/)
 [![Nuget](https://img.shields.io/nuget/vpre/Microsoft.ApplicationInsights.AspNetCore.svg)](https://nuget.org/packages/Microsoft.ApplicationInsights.AspNetCore)
 
+Windows: [![Build Status](https://mseng.visualstudio.com/AppInsights/_apis/build/status/ChuckNorris/AI_ASPNETCore_Develop?branchName=develop)](https://mseng.visualstudio.com/AppInsights/_build/latest?definitionId=3717&branchName=develop)
+
+Linux :[![Build Status](https://mseng.visualstudio.com/AppInsights/_apis/build/status/ChuckNorris/AI-AspNetCoreSDK-develop-linux?branchName=develop)](https://mseng.visualstudio.com/AppInsights/_build/latest?definitionId=6273&branchName=develop)
+
 
 Microsoft Application Insights for ASP.NET Core applications
 =============================================================

diff --git a/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.cs b/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.cs
@@ -22,8 +22,10 @@
     using Microsoft.AspNetCore.Http;
     using Microsoft.Extensions.Configuration;
     using Microsoft.Extensions.Configuration.Memory;
+    using Microsoft.Extensions.DependencyInjection;
     using Microsoft.Extensions.DependencyInjection.Extensions;
     using Microsoft.Extensions.Options;
+    using Microsoft.Extensions.Logging;
 
     /// <summary>
     /// Extension methods for <see cref="IServiceCollection"/> that allow adding Application Insights services to application.
@@ -190,8 +192,6 @@ public static IServiceCollection AddApplicationInsightsTelemetry(this IServiceCo
 
                 services.AddSingleton<TelemetryClient>();
 
-                services.AddSingleton<ApplicationInsightsDebugLogger, ApplicationInsightsDebugLogger>();
-
                 services
                     .TryAddSingleton<IConfigureOptions<ApplicationInsightsServiceOptions>,
                         DefaultApplicationInsightsServiceConfigureOptions>();
@@ -201,12 +201,38 @@ public static IServiceCollection AddApplicationInsightsTelemetry(this IServiceCo
                 // that requires IOptions infrastructure to run and initialize
                 services.AddSingleton<IStartupFilter, ApplicationInsightsStartupFilter>();
 
-                services.AddSingleton<JavaScriptSnippet>();
-                services.AddSingleton<ApplicationInsightsLoggerEvents>();
+                services.AddSingleton<JavaScriptSnippet>();                
 
                 services.AddOptions();
                 services.AddSingleton<IOptions<TelemetryConfiguration>, TelemetryConfigurationOptions>();
                 services.AddSingleton<IConfigureOptions<TelemetryConfiguration>, TelemetryConfigurationOptionsSetup>();
+
+                // NetStandard2.0 has a package reference to Microsoft.Extensions.Logging.ApplicationInsights, and
+                // enables ApplicationInsightsLoggerProvider by default.                
+#if NETSTANDARD2_0
+                services.AddLogging(loggingBuilder =>
+                {
+                     loggingBuilder.AddApplicationInsights();
+
+                    // The default behavior is to capture only logs above Warning level from all categories.
+                    // This can achieved with this code level filter -> loggingBuilder.AddFilter<Microsoft.Extensions.Logging.ApplicationInsights.ApplicationInsightsLoggerProvider>("",LogLevel.Warning);
+                    // However, this will make it impossible to override this behavior from Configuration like below using appsettings.json:
+                    //"ApplicationInsights": {
+                    // "LogLevel": {
+                    // "": "Error"
+                    // }
+                    // },
+                    // The reason is as both rules will match the filter, the last one added wins.
+                    // To ensure that the default filter is in the beginning of filter rules, so that user override from Configuration will always win, 
+                    // we add code filter rule to the 0th position as below.
+
+                    loggingBuilder.Services.Configure<LoggerFilterOptions>
+                    (options => options.Rules.Insert(0,
+                        new LoggerFilterRule(
+                            "Microsoft.Extensions.Logging.ApplicationInsights.ApplicationInsightsLoggerProvider", null,
+                            LogLevel.Warning, null)));
+                });                                
+#endif
             }
 
             return services;
@@ -406,8 +432,8 @@ internal static void AddTelemetryConfiguration(IConfiguration config,
 
         private static bool IsApplicationInsightsAdded(IServiceCollection services)
         {
-            // We treat ApplicationInsightsDebugLogger as a marker that AI services were added to service collection
-            return services.Any(service => service.ServiceType == typeof(ApplicationInsightsDebugLogger));
+            // We treat TelemetryClient as a marker that AI services were added to service collection
+            return services.Any(service => service.ServiceType == typeof(TelemetryClient));
         }
     }
 }
diff --git a/...nInsights.AspNetCore/Logging/Implementation/ApplicationInsightsLoggerFactoryExtensions.cs b/...nInsights.AspNetCore/Logging/Implementation/ApplicationInsightsLoggerFactoryExtensions.cs
@@ -2,7 +2,7 @@
 {
     using System;
     using ApplicationInsights;
-    using ApplicationInsights.AspNetCore.Logging;
+    using Microsoft.ApplicationInsights;
     using Microsoft.Extensions.DependencyInjection;
     using Microsoft.Extensions.Options;
 
@@ -16,7 +16,7 @@ public static class ApplicationInsightsLoggerFactoryExtensions
         /// </summary>
         /// <param name="factory"></param>
         /// <param name="serviceProvider">The instance of <see cref="IServiceProvider"/> to use for service resolution.</param>
-        [Obsolete("Use Microsoft.Extensions.Logging.ApplicationInsightsLoggingBuilderExtensions.AddApplicationInsights from Microsoft.Extensions.Logging.ApplicationInsights package")]
+        [Obsolete("ApplicationInsightsLoggerProvider is now enabled by default when enabling ApplicationInsights monitoring using UseApplicationInsights extension method on IWebHostBuilder or AddApplicationInsightsTelemetry extension method on IServiceCollection. From 2.7.0-beta3 onwards, calling this method will result in double logging and filters applied will not get applied. If interested in using just logging provider, then please use Microsoft.Extensions.Logging.ApplicationInsightsLoggingBuilderExtensions.AddApplicationInsights from Microsoft.Extensions.Logging.ApplicationInsights package. Read more https://aka.ms/ApplicationInsightsILoggerFaq")]
         public static ILoggerFactory AddApplicationInsights(this ILoggerFactory factory, IServiceProvider serviceProvider)
         {
             return factory.AddApplicationInsights(serviceProvider, LogLevel.Warning);
@@ -28,7 +28,7 @@ public static ILoggerFactory AddApplicationInsights(this ILoggerFactory factory,
         /// <param name="factory"></param>
         /// <param name="serviceProvider">The instance of <see cref="IServiceProvider"/> to use for service resolution.</param>
         /// <param name="minLevel">The minimum <see cref="LogLevel"/> to be logged</param>
-        [Obsolete("Use Microsoft.Extensions.Logging.ApplicationInsightsLoggingBuilderExtensions.AddApplicationInsights from Microsoft.Extensions.Logging.ApplicationInsights package")]
+        [Obsolete("ApplicationInsightsLoggerProvider is now enabled by default when enabling ApplicationInsights monitoring using UseApplicationInsights extension method on IWebHostBuilder or AddApplicationInsightsTelemetry extension method on IServiceCollection. From 2.7.0-beta3 onwards, calling this method will result in double logging and filters applied will not get applied. If interested in using just logging provider, then please use Microsoft.Extensions.Logging.ApplicationInsightsLoggingBuilderExtensions.AddApplicationInsights from Microsoft.Extensions.Logging.ApplicationInsights package. Read more https://aka.ms/ApplicationInsightsILoggerFaq")]
         public static ILoggerFactory AddApplicationInsights(
             this ILoggerFactory factory,
             IServiceProvider serviceProvider,
@@ -44,7 +44,7 @@ public static ILoggerFactory AddApplicationInsights(
         /// <param name="factory"></param>
         /// <param name="filter"></param>
         /// <param name="serviceProvider">The instance of <see cref="IServiceProvider"/> to use for service resolution.</param>
-        [Obsolete("Use Microsoft.Extensions.Logging.ApplicationInsightsLoggingBuilderExtensions.AddApplicationInsights from Microsoft.Extensions.Logging.ApplicationInsights package")]
+        [Obsolete("ApplicationInsightsLoggerProvider is now enabled by default when enabling ApplicationInsights monitoring using UseApplicationInsights extension method on IWebHostBuilder or AddApplicationInsightsTelemetry extension method on IServiceCollection. From 2.7.0-beta3 onwards, calling this method will result in double logging and filters applied will not get applied. If interested in using just logging provider, then please use Microsoft.Extensions.Logging.ApplicationInsightsLoggingBuilderExtensions.AddApplicationInsights from Microsoft.Extensions.Logging.ApplicationInsights package. Read more https://aka.ms/ApplicationInsightsILoggerFaq")]
         public static ILoggerFactory AddApplicationInsights(
             this ILoggerFactory factory,
             IServiceProvider serviceProvider,
@@ -60,20 +60,21 @@ public static ILoggerFactory AddApplicationInsights(
         /// <param name="filter"></param>
         /// <param name="serviceProvider">The instance of <see cref="IServiceProvider"/> to use for service resolution.</param>
         /// <param name="loggerAddedCallback">The callback that gets executed when another ApplicationInsights logger is added.</param>
-        [Obsolete("Use Microsoft.Extensions.Logging.ApplicationInsightsLoggingBuilderExtensions.AddApplicationInsights from Microsoft.Extensions.Logging.ApplicationInsights package")]
+        [Obsolete("ApplicationInsightsLoggerProvider is now enabled by default when enabling ApplicationInsights monitoring using UseApplicationInsights extension method on IWebHostBuilder or AddApplicationInsightsTelemetry extension method on IServiceCollection. From 2.7.0-beta3 onwards, calling this method will result in double logging and filters applied will not get applied. If interested in using just logging provider, then please use Microsoft.Extensions.Logging.ApplicationInsightsLoggingBuilderExtensions.AddApplicationInsights from Microsoft.Extensions.Logging.ApplicationInsights package. Read more https://aka.ms/ApplicationInsightsILoggerFaq")]
         public static ILoggerFactory AddApplicationInsights(
             this ILoggerFactory factory,
             IServiceProvider serviceProvider,
             Func<string, LogLevel, bool> filter,
             Action loggerAddedCallback)
-        {
+        {            
+
             var client = serviceProvider.GetService<TelemetryClient>();
-            var debugLoggerControl = serviceProvider.GetService<ApplicationInsightsLoggerEvents>();
-            var options = serviceProvider.GetService<IOptions<ApplicationInsightsLoggerOptions>>();
+            var debugLoggerControl = serviceProvider.GetService<Microsoft.ApplicationInsights.AspNetCore.Logging.ApplicationInsightsLoggerEvents>();
+            var options = serviceProvider.GetService<IOptions<Microsoft.ApplicationInsights.AspNetCore.Logging.ApplicationInsightsLoggerOptions>>();
 
             if (options == null)
             {
-                options = Options.Create(new ApplicationInsightsLoggerOptions());
+                options = Options.Create(new Microsoft.ApplicationInsights.AspNetCore.Logging.ApplicationInsightsLoggerOptions());
             }
 
             if (debugLoggerControl != null)
@@ -85,8 +86,8 @@ public static ILoggerFactory AddApplicationInsights(
                     debugLoggerControl.LoggerAdded += loggerAddedCallback;
                 }
             }
-
-            factory.AddProvider(new ApplicationInsightsLoggerProvider(client, filter, options));
+            factory.AddProvider(new Microsoft.ApplicationInsights.AspNetCore.Logging.ApplicationInsightsLoggerProvider(client, filter, options));
+                         
             return factory;
         }
     }

diff --git a/...pplicationInsights.AspNetCore/Logging/Implementation/ApplicationInsightsLoggerProvider.cs b/...pplicationInsights.AspNetCore/Logging/Implementation/ApplicationInsightsLoggerProvider.cs
@@ -7,7 +7,10 @@
     /// <summary>
     /// <see cref="ILoggerProvider"/> implementation that creates returns instances of <see cref="ApplicationInsightsLogger"/>
     /// </summary>
+#if !NETSTANDARD2_0
+    // For NETSTANDARD2.0 We take dependency on Microsoft.Extensions.Logging.ApplicationInsights which has ApplicationInsightsProvider having the same ProviderAlias and don't want to clash with this ProviderAlias.
     [ProviderAlias("ApplicationInsights")]
+#endif
     internal class ApplicationInsightsLoggerProvider : ILoggerProvider
     {
         private readonly TelemetryClient telemetryClient;

diff --git a/src/Microsoft.ApplicationInsights.AspNetCore/Microsoft.ApplicationInsights.AspNetCore.csproj b/src/Microsoft.ApplicationInsights.AspNetCore/Microsoft.ApplicationInsights.AspNetCore.csproj
@@ -86,17 +86,18 @@
     <PackageReference Include="Microsoft.ApplicationInsights.PerfCounterCollector" Version="2.10.0-beta2" />
     <PackageReference Include="Microsoft.ApplicationInsights.WindowsServer" Version="2.10.0-beta2" />
     <PackageReference Include="Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel" Version="2.10.0-beta2" />
-    <PackageReference Include="Microsoft.AspNetCore.Hosting" Version="1.0.2" />
-    <PackageReference Include="Microsoft.Extensions.Configuration" Version="1.0.2" />
-    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="1.0.2" />
-    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="1.0.2" />
+    <PackageReference Include="Microsoft.AspNetCore.Hosting" Version="1.0.2" />           
     <PackageReference Include="System.Text.Encodings.Web" Version="4.3.1" />
   </ItemGroup>
 
-  <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' ">
-  <PackageReference Include="Microsoft.Extensions.Configuration" Version="2.0.0" />
-  <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="2.0.0" />
-  <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="2.0.0" />
+  <ItemGroup Condition=" '$(TargetFramework)' != 'netstandard2.0' ">
+    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="1.0.2" />
+    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="1.0.2" />    
+  </ItemGroup>
+
+  <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' ">  
+  <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="2.1.0" />  
+  <PackageReference Include="Microsoft.Extensions.Logging.ApplicationInsights" Version="2.9.1" />
   <PackageReference Include="System.Text.Encodings.Web" Version="4.3.1" />
   </ItemGroup>
 

diff --git a/...icrosoft.ApplicationInsights.AspNetCore/TelemetryInitializers/TelemetryInitializerBase.cs b/...icrosoft.ApplicationInsights.AspNetCore/TelemetryInitializers/TelemetryInitializerBase.cs
@@ -13,11 +13,13 @@ public abstract class TelemetryInitializerBase : ITelemetryInitializer
     {
         private readonly IHttpContextAccessor httpContextAccessor;
 
+        /// <inheritdoc />
         public TelemetryInitializerBase(IHttpContextAccessor httpContextAccessor)
         {
             this.httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor));
         }
 
+        /// <inheritdoc />
         public void Initialize(ITelemetry telemetry)
         {
             var context = this.httpContextAccessor.HttpContext;

diff --git a/...ft.ApplicationInsights.AspNetCore/TelemetryInitializers/WebSessionTelemetryInitializer.cs b/...ft.ApplicationInsights.AspNetCore/TelemetryInitializers/WebSessionTelemetryInitializer.cs
@@ -9,6 +9,7 @@ public class WebSessionTelemetryInitializer : TelemetryInitializerBase
     {
         private const string WebSessionCookieName = "ai_session";
 
+        /// <inheritdoc />
         public WebSessionTelemetryInitializer(IHttpContextAccessor httpContextAccessor)
              : base(httpContextAccessor)
         {

diff --git a/...osoft.ApplicationInsights.AspNetCore/TelemetryInitializers/WebUserTelemetryInitializer.cs b/...osoft.ApplicationInsights.AspNetCore/TelemetryInitializers/WebUserTelemetryInitializer.cs
@@ -5,15 +5,18 @@
     using Microsoft.ApplicationInsights.DataContracts;
     using Microsoft.AspNetCore.Http;
 
+    /// <inheritdoc />
     public class WebUserTelemetryInitializer : TelemetryInitializerBase
     {
         private const string WebUserCookieName = "ai_user";
 
+        /// <inheritdoc />
         public WebUserTelemetryInitializer(IHttpContextAccessor httpContextAccessor)
              : base(httpContextAccessor)
         {
         }
 
+        /// <inheritdoc />
         protected override void OnInitializeTelemetry(HttpContext platformContext, RequestTelemetry requestTelemetry, ITelemetry telemetry)
         {
             if (!string.IsNullOrEmpty(telemetry.Context.User.Id))

diff --git a/test/FunctionalTestUtils20/InProcessServer.cs b/test/FunctionalTestUtils20/InProcessServer.cs
@@ -40,15 +40,17 @@ public class InProcessServer : IDisposable
 
         private TelemetryHttpListenerObservable listener;
         private readonly Func<IWebHostBuilder, IWebHostBuilder> configureHost;
+        private readonly Action<IServiceCollection> configureServices;
 
-        public InProcessServer(string assemblyName, ITestOutputHelper output, Func<IWebHostBuilder, IWebHostBuilder> configureHost = null)
+        public InProcessServer(string assemblyName, ITestOutputHelper output, Func<IWebHostBuilder, IWebHostBuilder> configureHost = null, Action<IServiceCollection> configureServices = null)
         {
             this.output = output;
 
             // localhost instead of machine name, as its not possible to get machine name when running non windows.
             var machineName = "localhost";
             this.url = "http://" + machineName + ":" + random.Next(5000, 14000).ToString();
             this.configureHost = configureHost;
+            this.configureServices = configureServices;
             this.httpListenerConnectionString = LauchApplicationAndStartListener(assemblyName);
         }
 
@@ -155,7 +157,12 @@ private string Start(string assemblyName)
             {
                 builder = this.configureHost(builder);
             }
-
+
+            if (this.configureServices != null)
+            {
+                builder.ConfigureServices(services => this.configureServices(services));
+            }
+
             this.hostingEngine = builder.Build();
             this.hostingEngine.Start();
 

diff --git a/test/MVCFramework20.FunctionalTests/MVCFramework20.FunctionalTests20.csproj b/test/MVCFramework20.FunctionalTests/MVCFramework20.FunctionalTests20.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <VersionPrefix>2.0.0</VersionPrefix>
@@ -56,10 +56,10 @@
     <PackageReference Include="Microsoft.AspNetCore.Server.Kestrel" Version="2.0.0" />
     <PackageReference Include="Microsoft.AspNetCore.StaticFiles" Version="2.0.0" />
     <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="2.0.0" />
-    <PackageReference Include="Microsoft.Extensions.Configuration.FileExtensions" Version="2.0.0" />
-    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="2.0.0" />
+    <PackageReference Include="Microsoft.Extensions.Configuration.FileExtensions" Version="2.1.0" />
+    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="2.1.0" />
     <PackageReference Include="Microsoft.Extensions.Configuration.UserSecrets" Version="2.0.0" />
-    <PackageReference Include="Microsoft.Extensions.Logging" Version="2.0.0" />
+    <PackageReference Include="Microsoft.Extensions.Logging" Version="2.1.0" />
     <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="2.0.0" />
     <PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="2.0.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.7.0" />