From 896e15a1773f9ab03f65eee55c76fca9941ab5c5 Mon Sep 17 00:00:00 2001 From: Sourabh Shirhatti Date: Fri, 29 May 2020 15:45:35 -0700 Subject: [PATCH 1/8] Revert "Hoist activity fields to the logging scope (#11211)" This reverts commit f7a2d3c26cfd821d0740fdf5f5ac26845fcd33d0. --- .../src/Internal/ActivityExtensions.cs | 43 ------------------- .../Internal/HostingApplicationDiagnostics.cs | 2 +- .../src/Internal/HostingLoggerExtensions.cs | 30 ++++--------- 3 files changed, 10 insertions(+), 65 deletions(-) delete mode 100644 src/Hosting/Hosting/src/Internal/ActivityExtensions.cs diff --git a/src/Hosting/Hosting/src/Internal/ActivityExtensions.cs b/src/Hosting/Hosting/src/Internal/ActivityExtensions.cs deleted file mode 100644 index fa23a5ca12b0..000000000000 --- a/src/Hosting/Hosting/src/Internal/ActivityExtensions.cs +++ /dev/null @@ -1,43 +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.Diagnostics; - -namespace Microsoft.AspNetCore.Hosting -{ - /// - /// Helpers for getting the right values from Activity no matter the format (w3c or hierarchical) - /// - internal static class ActivityExtensions - { - public static string GetSpanId(this Activity activity) - { - return activity.IdFormat switch - { - ActivityIdFormat.Hierarchical => activity.Id, - ActivityIdFormat.W3C => activity.SpanId.ToHexString(), - _ => null, - } ?? string.Empty; - } - - public static string GetTraceId(this Activity activity) - { - return activity.IdFormat switch - { - ActivityIdFormat.Hierarchical => activity.RootId, - ActivityIdFormat.W3C => activity.TraceId.ToHexString(), - _ => null, - } ?? string.Empty; - } - - public static string GetParentId(this Activity activity) - { - return activity.IdFormat switch - { - ActivityIdFormat.Hierarchical => activity.ParentId, - ActivityIdFormat.W3C => activity.ParentSpanId.ToHexString(), - _ => null, - } ?? string.Empty; - } - } -} diff --git a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs index 41b93ac083c3..9ccf5d0de6c7 100644 --- a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs @@ -69,7 +69,7 @@ public void BeginRequest(HttpContext httpContext, HostingApplication.Context con // Scope may be relevant for a different level of logging, so we always create it // see: https://github.com/aspnet/Hosting/pull/944 // Scope can be null if logging is not on. - context.Scope = _logger.RequestScope(httpContext, context.Activity); + context.Scope = _logger.RequestScope(httpContext, context.Activity.Id); if (_logger.IsEnabled(LogLevel.Information)) { diff --git a/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs b/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs index e5fb4fd0c6e6..60bedd68b97e 100644 --- a/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs +++ b/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs @@ -4,7 +4,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Diagnostics; using System.Globalization; using System.Reflection; using Microsoft.AspNetCore.Http; @@ -14,9 +13,9 @@ namespace Microsoft.AspNetCore.Hosting { internal static class HostingLoggerExtensions { - public static IDisposable RequestScope(this ILogger logger, HttpContext httpContext, Activity activity) + public static IDisposable RequestScope(this ILogger logger, HttpContext httpContext, string activityId) { - return logger.BeginScope(new HostingLogScope(httpContext, activity)); + return logger.BeginScope(new HostingLogScope(httpContext, activityId)); } public static void ApplicationError(this ILogger logger, Exception exception) @@ -97,7 +96,7 @@ private class HostingLogScope : IReadOnlyList> { private readonly string _path; private readonly string _traceIdentifier; - private readonly Activity _activity; + private readonly string _activityId; private string _cachedToString; @@ -105,7 +104,7 @@ public int Count { get { - return 5; + return 3; } } @@ -123,29 +122,20 @@ public KeyValuePair this[int index] } else if (index == 2) { - return new KeyValuePair("SpanId", _activity.GetSpanId()); - } - else if (index == 3) - { - return new KeyValuePair("TraceId", _activity.GetTraceId()); - } - else if (index == 4) - { - return new KeyValuePair("ParentId", _activity.GetParentId()); + return new KeyValuePair("ActivityId", _activityId); } throw new ArgumentOutOfRangeException(nameof(index)); } } - public HostingLogScope(HttpContext httpContext, Activity activity) + public HostingLogScope(HttpContext httpContext, string activityId) { _traceIdentifier = httpContext.TraceIdentifier; _path = (httpContext.Request.PathBase.HasValue ? httpContext.Request.PathBase + httpContext.Request.Path : httpContext.Request.Path).ToString(); - - _activity = activity; + _activityId = activityId; } public override string ToString() @@ -154,12 +144,10 @@ public override string ToString() { _cachedToString = string.Format( CultureInfo.InvariantCulture, - "RequestPath:{0} RequestId:{1}, SpanId:{2}, TraceId:{3}, ParentId:{4}", + "RequestPath:{0} RequestId:{1}, ActivityId:{2}", _path, _traceIdentifier, - _activity.GetSpanId(), - _activity.GetTraceId(), - _activity.GetParentId()); + _activityId); } return _cachedToString; From 970bf1947e749be5f3e8c6c9c297d2f7e6db2e16 Mon Sep 17 00:00:00 2001 From: Sourabh Shirhatti Date: Fri, 29 May 2020 16:39:42 -0700 Subject: [PATCH 2/8] Remove tests with Activity --- .../HostingApplicationDiagnosticsTests.cs | 44 ------------------- 1 file changed, 44 deletions(-) diff --git a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs index 08e3832d7ede..3ee8fbb34662 100644 --- a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs +++ b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs @@ -40,50 +40,6 @@ public void CreateContextWithDisabledLoggerDoesNotCreateActivity() Assert.Null(Activity.Current); } - [Fact] - public void CreateContextWithEnabledLoggerCreatesActivityAndSetsActivityInScope() - { - // Arrange - var logger = new LoggerWithScopes(isEnabled: true); - var hostingApplication = CreateApplication(out var features, logger: logger); - - // Act - var context = hostingApplication.CreateContext(features); - - Assert.Single(logger.Scopes); - var pairs = ((IReadOnlyList>)logger.Scopes[0]).ToDictionary(p => p.Key, p => p.Value); - Assert.Equal(Activity.Current.Id, pairs["SpanId"].ToString()); - Assert.Equal(Activity.Current.RootId, pairs["TraceId"].ToString()); - Assert.Equal(string.Empty, pairs["ParentId"]?.ToString()); - } - - [Fact] - public void CreateContextWithEnabledLoggerAndRequestIdCreatesActivityAndSetsActivityInScope() - { - // Arrange - - // Generate an id we can use for the request id header (in the correct format) - var activity = new Activity("IncomingRequest"); - activity.Start(); - var id = activity.Id; - activity.Stop(); - - var logger = new LoggerWithScopes(isEnabled: true); - var hostingApplication = CreateApplication(out var features, logger: logger, configure: context => - { - context.Request.Headers["Request-Id"] = id; - }); - - // Act - var context = hostingApplication.CreateContext(features); - - Assert.Single(logger.Scopes); - var pairs = ((IReadOnlyList>)logger.Scopes[0]).ToDictionary(p => p.Key, p => p.Value); - Assert.Equal(Activity.Current.Id, pairs["SpanId"].ToString()); - Assert.Equal(Activity.Current.RootId, pairs["TraceId"].ToString()); - Assert.Equal(id, pairs["ParentId"].ToString()); - } - [Fact] public void ActivityStopDoesNotFireIfNoListenerAttachedForStart() { From e24a383d9f92663cd9ccea854b4b935d64a6309c Mon Sep 17 00:00:00 2001 From: Sourabh Shirhatti Date: Fri, 29 May 2020 16:48:33 -0700 Subject: [PATCH 3/8] Remove ActivityId from HostingLogScope --- .../src/Internal/HostingLoggerExtensions.cs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs b/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs index 60bedd68b97e..013c8eb3a4bf 100644 --- a/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs +++ b/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs @@ -15,7 +15,7 @@ internal static class HostingLoggerExtensions { public static IDisposable RequestScope(this ILogger logger, HttpContext httpContext, string activityId) { - return logger.BeginScope(new HostingLogScope(httpContext, activityId)); + return logger.BeginScope(new HostingLogScope(httpContext)); } public static void ApplicationError(this ILogger logger, Exception exception) @@ -96,7 +96,6 @@ private class HostingLogScope : IReadOnlyList> { private readonly string _path; private readonly string _traceIdentifier; - private readonly string _activityId; private string _cachedToString; @@ -104,7 +103,7 @@ public int Count { get { - return 3; + return 2; } } @@ -120,22 +119,17 @@ public KeyValuePair this[int index] { return new KeyValuePair("RequestPath", _path); } - else if (index == 2) - { - return new KeyValuePair("ActivityId", _activityId); - } throw new ArgumentOutOfRangeException(nameof(index)); } } - public HostingLogScope(HttpContext httpContext, string activityId) + public HostingLogScope(HttpContext httpContext) { _traceIdentifier = httpContext.TraceIdentifier; _path = (httpContext.Request.PathBase.HasValue ? httpContext.Request.PathBase + httpContext.Request.Path : httpContext.Request.Path).ToString(); - _activityId = activityId; } public override string ToString() @@ -144,10 +138,9 @@ public override string ToString() { _cachedToString = string.Format( CultureInfo.InvariantCulture, - "RequestPath:{0} RequestId:{1}, ActivityId:{2}", + "RequestPath:{0} RequestId:{1}", _path, - _traceIdentifier, - _activityId); + _traceIdentifier); } return _cachedToString; From 8cceca9662a6f26cfec65d9bcbb933ff013ed002 Mon Sep 17 00:00:00 2001 From: Sourabh Shirhatti Date: Mon, 1 Jun 2020 09:10:11 -0700 Subject: [PATCH 4/8] Enable propogation in CreateDefaultBuilder --- src/DefaultBuilder/src/WebHost.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/DefaultBuilder/src/WebHost.cs b/src/DefaultBuilder/src/WebHost.cs index e8f00fa602ef..752fc325af70 100644 --- a/src/DefaultBuilder/src/WebHost.cs +++ b/src/DefaultBuilder/src/WebHost.cs @@ -191,6 +191,12 @@ public static IWebHostBuilder CreateDefaultBuilder(string[] args) }) .ConfigureLogging((hostingContext, logging) => { + logging.Configure(options => + { + options.ActivityTrackingOptions = ActivityTrackingOptions.SpanId + | ActivityTrackingOptions.TraceId + | ActivityTrackingOptions.ParentId; + }); logging.AddConfiguration(hostingContext.Configuration.GetSection("Logging")); logging.AddConsole(); logging.AddDebug(); From fecd74d3495c4c8ee93edc7ae9aab17c7d738d1f Mon Sep 17 00:00:00 2001 From: Sourabh Shirhatti Date: Mon, 1 Jun 2020 09:17:08 -0700 Subject: [PATCH 5/8] Clean up --- .../Hosting/src/Internal/HostingApplicationDiagnostics.cs | 2 +- src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs index 9ccf5d0de6c7..41156eb7d6e4 100644 --- a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs @@ -69,7 +69,7 @@ public void BeginRequest(HttpContext httpContext, HostingApplication.Context con // Scope may be relevant for a different level of logging, so we always create it // see: https://github.com/aspnet/Hosting/pull/944 // Scope can be null if logging is not on. - context.Scope = _logger.RequestScope(httpContext, context.Activity.Id); + context.Scope = _logger.RequestScope(httpContext); if (_logger.IsEnabled(LogLevel.Information)) { diff --git a/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs b/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs index 013c8eb3a4bf..4f8c1e6e7ec2 100644 --- a/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs +++ b/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs @@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Hosting { internal static class HostingLoggerExtensions { - public static IDisposable RequestScope(this ILogger logger, HttpContext httpContext, string activityId) + public static IDisposable RequestScope(this ILogger logger, HttpContext httpContext) { return logger.BeginScope(new HostingLogScope(httpContext)); } From a60668371b59fd13c072cf4cc6b0fbe74cb35a1b Mon Sep 17 00:00:00 2001 From: Sourabh Shirhatti Date: Wed, 3 Jun 2020 13:41:26 -0700 Subject: [PATCH 6/8] s/logging/loggingBuilder --- src/DefaultBuilder/src/WebHost.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/DefaultBuilder/src/WebHost.cs b/src/DefaultBuilder/src/WebHost.cs index 752fc325af70..2dda5979f84d 100644 --- a/src/DefaultBuilder/src/WebHost.cs +++ b/src/DefaultBuilder/src/WebHost.cs @@ -189,18 +189,18 @@ public static IWebHostBuilder CreateDefaultBuilder(string[] args) config.AddCommandLine(args); } }) - .ConfigureLogging((hostingContext, logging) => + .ConfigureLogging((hostingContext, loggingBuilder) => { - logging.Configure(options => + loggingBuilder.Configure(options => { options.ActivityTrackingOptions = ActivityTrackingOptions.SpanId | ActivityTrackingOptions.TraceId | ActivityTrackingOptions.ParentId; }); - logging.AddConfiguration(hostingContext.Configuration.GetSection("Logging")); - logging.AddConsole(); - logging.AddDebug(); - logging.AddEventSourceLogger(); + loggingBuilder.AddConfiguration(hostingContext.Configuration.GetSection("Logging")); + loggingBuilder.AddConsole(); + loggingBuilder.AddDebug(); + loggingBuilder.AddEventSourceLogger(); }). UseDefaultServiceProvider((context, options) => { From dbe1b7bbe82cd38cd511f028d1e699729e3f736f Mon Sep 17 00:00:00 2001 From: Sourabh Shirhatti Date: Tue, 9 Jun 2020 14:32:43 -0700 Subject: [PATCH 7/8] Enable Activity propogation for generichost --- src/DefaultBuilder/src/WebHost.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/DefaultBuilder/src/WebHost.cs b/src/DefaultBuilder/src/WebHost.cs index 2dda5979f84d..801e9f83168b 100644 --- a/src/DefaultBuilder/src/WebHost.cs +++ b/src/DefaultBuilder/src/WebHost.cs @@ -221,6 +221,15 @@ internal static void ConfigureWebDefaults(IWebHostBuilder builder) StaticWebAssetsLoader.UseStaticWebAssets(ctx.HostingEnvironment, ctx.Configuration); } }); + builder.ConfigureLogging(loggingBuilder => + { + loggingBuilder.Configure(options => + { + options.ActivityTrackingOptions = ActivityTrackingOptions.SpanId + | ActivityTrackingOptions.TraceId + | ActivityTrackingOptions.ParentId; + }); + }); builder.UseKestrel((builderContext, options) => { options.Configure(builderContext.Configuration.GetSection("Kestrel")); From 797bdd03604bfa7f66242a83b4d19e7574054e40 Mon Sep 17 00:00:00 2001 From: Sourabh Shirhatti Date: Mon, 15 Jun 2020 02:11:58 -0700 Subject: [PATCH 8/8] Replace with runtime/pull/37892 --- src/DefaultBuilder/src/WebHost.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/DefaultBuilder/src/WebHost.cs b/src/DefaultBuilder/src/WebHost.cs index 801e9f83168b..2dda5979f84d 100644 --- a/src/DefaultBuilder/src/WebHost.cs +++ b/src/DefaultBuilder/src/WebHost.cs @@ -221,15 +221,6 @@ internal static void ConfigureWebDefaults(IWebHostBuilder builder) StaticWebAssetsLoader.UseStaticWebAssets(ctx.HostingEnvironment, ctx.Configuration); } }); - builder.ConfigureLogging(loggingBuilder => - { - loggingBuilder.Configure(options => - { - options.ActivityTrackingOptions = ActivityTrackingOptions.SpanId - | ActivityTrackingOptions.TraceId - | ActivityTrackingOptions.ParentId; - }); - }); builder.UseKestrel((builderContext, options) => { options.Configure(builderContext.Configuration.GetSection("Kestrel"));