LogLevel changed to Error and stack trace added for generic unknown e…

…xception within SDK (#946) Generic exception logging modified to include stack trace
microsoft · Jul 24, 2019 · da4c563 · da4c563
1 parent d864a98
commit da4c563
Show file tree

Hide file tree

Showing 6 changed files with 9 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 - [Obsolete extension methods on IWebHostBuilder in favor of AddApplicationInsights extension method on IServiceCollection.](https://github.com/microsoft/ApplicationInsights-aspnetcore/issues/919)
 - [Remove support for deprecated x-ms based correlation headers.](https://github.com/microsoft/ApplicationInsights-aspnetcore/issues/939)
 - [Uri for multiple hosts headers is set to "Multiple-Host".](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/862)
+- [LogLevel changed to Error and stack trace added for generic unknown exception within SDK.](https://github.com/microsoft/ApplicationInsights-aspnetcore/pull/946)
 
 ## Version 2.8.0-beta1
 - [Add EventCounter collection.](https://github.com/microsoft/ApplicationInsights-aspnetcore/issues/913)

diff --git a/...licationInsights.AspNetCore/Extensibility/Implementation/Tracing/AspNetCoreEventSource.cs b/...licationInsights.AspNetCore/Extensibility/Implementation/Tracing/AspNetCoreEventSource.cs
@@ -137,8 +137,8 @@ public void NotActiveListenerNoTracking(string evntName, string activityId, stri
             14,
             Keywords = Keywords.Diagnostics,
             Message = "An error has occured which may prevent application insights from functioning. Error message: '{0}' ",
-            Level = EventLevel.Warning)]
-        public void LogWarning(string errorMessage, string appDomainName = "Incorrect")
+            Level = EventLevel.Error)]
+        public void LogError(string errorMessage, string appDomainName = "Incorrect")
         {
             this.WriteEvent(14, errorMessage, this.ApplicationName);
         }

diff --git a/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.cs b/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.cs
@@ -18,6 +18,7 @@
     using Microsoft.ApplicationInsights.Extensibility.EventCounterCollector;
 #endif
     using Microsoft.ApplicationInsights.Extensibility.Implementation.ApplicationId;
+    using Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing;
     using Microsoft.ApplicationInsights.Extensibility.PerfCounterCollector;
     using Microsoft.ApplicationInsights.Extensibility.PerfCounterCollector.QuickPulse;
     using Microsoft.ApplicationInsights.WindowsServer;
@@ -287,7 +288,7 @@ public static IServiceCollection AddApplicationInsightsTelemetry(this IServiceCo
             }
             catch (Exception e)
             {
-                AspNetCoreEventSource.Instance.LogWarning(e.Message);
+                AspNetCoreEventSource.Instance.LogError(e.ToInvariantString());
                 return services;
             }
         }

diff --git a/...crosoft.ApplicationInsights.AspNetCore/Implementation/ApplicationInsightsStartupFilter.cs b/...crosoft.ApplicationInsights.AspNetCore/Implementation/ApplicationInsightsStartupFilter.cs
@@ -3,6 +3,7 @@
     using System;
     using Microsoft.ApplicationInsights.AspNetCore.Extensibility.Implementation.Tracing;
     using Microsoft.ApplicationInsights.Extensibility;
+    using Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing;
     using Microsoft.AspNetCore.Builder;
     using Microsoft.AspNetCore.Hosting;
     using Microsoft.Extensions.DependencyInjection;
@@ -35,7 +36,7 @@ public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
                 catch (Exception ex)
                 {
                     this.logger.LogWarning(0, ex, "Failed to resolve TelemetryConfiguration.");
-                    AspNetCoreEventSource.Instance.LogWarning(ex.Message);
+                    AspNetCoreEventSource.Instance.LogError(ex.ToInvariantString());
                 }
 
                 // Invoking next builder is not wrapped in try catch to ensure any exceptions gets propogated up.

diff --git a/...osoft.ApplicationInsights.AspNetCore/Implementation/TelemetryConfigurationOptionsSetup.cs b/...osoft.ApplicationInsights.AspNetCore/Implementation/TelemetryConfigurationOptionsSetup.cs
@@ -135,7 +135,7 @@ public void Configure(TelemetryConfiguration configuration)
             }
             catch (Exception ex)
             {
-                AspNetCoreEventSource.Instance.TelemetryConfigurationSetupFailure(ex.Message);
+                AspNetCoreEventSource.Instance.TelemetryConfigurationSetupFailure(ex.ToInvariantString());
             }
         }
 

diff --git a/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HostingDiagnosticListenerTest.cs b/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HostingDiagnosticListenerTest.cs
@@ -261,10 +261,9 @@ public void OnBeginRequestCreateNewActivityAndInitializeRequestTelemetry(bool is
         [Fact]
         public void OnBeginRequestCreateNewActivityAndInitializeRequestTelemetryFromRequestIdHeader()
         {
+            // This tests 1.XX scenario where SDK is responsible for reading Correlation-Context and populate Activity.Baggage
             HttpContext context = CreateContext(HttpRequestScheme, HttpRequestHost, "/Test", method: "POST");
             var requestId = Guid.NewGuid().ToString();
-            var standardRequestId = Guid.NewGuid().ToString();
-            var standardRequestRootId = Guid.NewGuid().ToString();
             context.Request.Headers[RequestResponseHeaders.RequestIdHeader] = requestId;
             context.Request.Headers[RequestResponseHeaders.CorrelationContextHeader] = "prop1=value1, prop2=value2";
 
@@ -280,9 +279,7 @@ public void OnBeginRequestCreateNewActivityAndInitializeRequestTelemetryFromRequ
                 Assert.NotNull(requestTelemetry);
                 Assert.Equal(requestTelemetry.Id, Activity.Current.Id);
                 Assert.Equal(requestTelemetry.Context.Operation.Id, Activity.Current.RootId);
-                Assert.NotEqual(requestTelemetry.Context.Operation.Id, standardRequestRootId);
                 Assert.Equal(requestTelemetry.Context.Operation.ParentId, requestId);
-                Assert.NotEqual(requestTelemetry.Context.Operation.ParentId, standardRequestId);
                 Assert.Equal("value1", requestTelemetry.Properties["prop1"]);
                 Assert.Equal("value2", requestTelemetry.Properties["prop2"]);
             }
@@ -358,15 +355,12 @@ public void OnEndRequestSetsRequestNameToMethodAndPath(bool isAspNetCore2)
         {
             HttpContext context = CreateContext(HttpRequestScheme, HttpRequestHost, "/Test", method: "GET");
             TelemetryConfiguration config = TelemetryConfiguration.CreateDefault();
-            config.ExperimentalFeatures.Add("conditionalAppId");
 
             using (var hostingListener = CreateHostingListener(isAspNetCore2, config))
             {
                 HandleRequestBegin(hostingListener, context, 0, isAspNetCore2);
 
                 Assert.NotNull(context.Features.Get<RequestTelemetry>());
-                Assert.Null(HttpHeadersUtilities.GetRequestContextKeyValue(context.Response.Headers,
-                        RequestResponseHeaders.RequestContextTargetKey));
 
                 HandleRequestEnd(hostingListener, context, 0, isAspNetCore2);
             }