Skip to content

Remove default logging scope from ASP.NET Core #44873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/Hosting/Hosting/src/Internal/HostingApplication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -153,7 +152,6 @@ public void Reset()
{
// Not resetting HttpContext here as we pool it on the Context

Scope = null;
Activity = null;
StartLog = null;

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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<KeyValuePair<string, object>>
{
private readonly string _path;
private readonly string _traceIdentifier;

private string? _cachedToString;

public int Count => 2;

public KeyValuePair<string, object> this[int index]
{
get
{
if (index == 0)
{
return new KeyValuePair<string, object>("RequestId", _traceIdentifier);
}
else if (index == 1)
{
return new KeyValuePair<string, object>("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<KeyValuePair<string, object>> GetEnumerator()
{
for (var i = 0; i < Count; ++i)
{
yield return this[i];
}
}

IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
}
}
}
22 changes: 0 additions & 22 deletions src/Hosting/Hosting/test/WebHostTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<IHttpRequestIdentifierFeature>().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()
{
Expand Down