From 0bc04d3dc88775254b825202353baf2c7e74feb0 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sun, 23 Jul 2023 09:04:56 -0400 Subject: [PATCH] Stop creating the default logging scope in hosting - We correlate logs with Activities and more information can be added to them (like the trace identifier) using activity listeners or custom middleware. --- .../src/Internal/HostingApplication.cs | 2 - .../Internal/HostingApplicationDiagnostics.cs | 80 ------------------- src/Hosting/Hosting/test/WebHostTests.cs | 22 ----- 3 files changed, 104 deletions(-) diff --git a/src/Hosting/Hosting/src/Internal/HostingApplication.cs b/src/Hosting/Hosting/src/Internal/HostingApplication.cs index aae3b7d1cd90..f0cd289f3122 100644 --- a/src/Hosting/Hosting/src/Internal/HostingApplication.cs +++ b/src/Hosting/Hosting/src/Internal/HostingApplication.cs @@ -121,7 +121,6 @@ public void DisposeContext(Context context, Exception? exception) internal sealed class Context { public HttpContext? HttpContext { get; set; } - public IDisposable? Scope { get; set; } public Activity? Activity { get => HttpActivityFeature?.Activity; @@ -153,7 +152,6 @@ public void Reset() { // Not resetting HttpContext here as we pool it on the Context - Scope = null; Activity = null; StartLog = null; diff --git a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs index c5658692b0dd..e73aceac46c6 100644 --- a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs @@ -1,10 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Globalization; using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -97,11 +95,6 @@ public void BeginRequest(HttpContext httpContext, HostingApplication.Context con // To avoid allocation, return a null scope if the logger is not on at least to some degree. if (loggingEnabled) { - // 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 = Log.RequestScope(_logger, httpContext); - if (_logger.IsEnabled(LogLevel.Information)) { if (startTimestamp == 0) @@ -215,9 +208,6 @@ public void RequestEnd(HttpContext httpContext, Exception? exception, HostingApp _eventSource.RequestFailed(); } } - - // Logging Scope is finshed with - context.Scope?.Dispose(); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -506,74 +496,4 @@ private void StopActivity(Activity activity, HttpContext httpContext) WriteDiagnosticEvent(_diagnosticListener, ActivityStopKey, httpContext); activity.Stop(); // Resets Activity.Current (we want this after the Write) } - - private static class Log - { - public static IDisposable? RequestScope(ILogger logger, HttpContext httpContext) - { - return logger.BeginScope(new HostingLogScope(httpContext)); - } - - private sealed class HostingLogScope : IReadOnlyList> - { - private readonly string _path; - private readonly string _traceIdentifier; - - private string? _cachedToString; - - public int Count => 2; - - public KeyValuePair this[int index] - { - get - { - if (index == 0) - { - return new KeyValuePair("RequestId", _traceIdentifier); - } - else if (index == 1) - { - return new KeyValuePair("RequestPath", _path); - } - - throw new ArgumentOutOfRangeException(nameof(index)); - } - } - - public HostingLogScope(HttpContext httpContext) - { - _traceIdentifier = httpContext.TraceIdentifier; - _path = (httpContext.Request.PathBase.HasValue - ? httpContext.Request.PathBase + httpContext.Request.Path - : httpContext.Request.Path).ToString(); - } - - public override string ToString() - { - if (_cachedToString == null) - { - _cachedToString = string.Format( - CultureInfo.InvariantCulture, - "RequestPath:{0} RequestId:{1}", - _path, - _traceIdentifier); - } - - return _cachedToString; - } - - public IEnumerator> GetEnumerator() - { - for (var i = 0; i < Count; ++i) - { - yield return this[i]; - } - } - - IEnumerator IEnumerable.GetEnumerator() - { - return GetEnumerator(); - } - } - } } diff --git a/src/Hosting/Hosting/test/WebHostTests.cs b/src/Hosting/Hosting/test/WebHostTests.cs index 70b528a1e4e3..ed32e5874f38 100644 --- a/src/Hosting/Hosting/test/WebHostTests.cs +++ b/src/Hosting/Hosting/test/WebHostTests.cs @@ -868,28 +868,6 @@ public async Task IsEnvironment_Extension_Is_Case_Insensitive() } } - [Fact] - public async Task WebHost_CreatesDefaultRequestIdentifierFeature_IfNotPresent() - { - // Arrange - var requestDelegate = new RequestDelegate(httpContext => - { - // Assert - Assert.NotNull(httpContext); - var featuresTraceIdentifier = httpContext.Features.Get().TraceIdentifier; - Assert.False(string.IsNullOrWhiteSpace(httpContext.TraceIdentifier)); - Assert.Same(httpContext.TraceIdentifier, featuresTraceIdentifier); - - return Task.CompletedTask; - }); - - using (var host = CreateHost(requestDelegate)) - { - // Act - await host.StartAsync(); - } - } - [Fact] public async Task WebHost_DoesNot_CreateDefaultRequestIdentifierFeature_IfPresent() {