From e7d244b23b18b414bd42a9ad304c10e93b0a5041 Mon Sep 17 00:00:00 2001 From: amadeuszl Date: Tue, 11 Mar 2025 17:25:00 +0100 Subject: [PATCH 1/4] Add options to log unsupported routes --- .../Logging/HttpLoggingRedactionInterceptor.cs | 6 ++++++ .../Logging/LoggingRedactionOptions.cs | 5 +++++ .../Logging/AcceptanceTests.cs | 6 +++--- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/HttpLoggingRedactionInterceptor.cs b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/HttpLoggingRedactionInterceptor.cs index 050f0bb092d..0ebd099d41c 100644 --- a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/HttpLoggingRedactionInterceptor.cs +++ b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/HttpLoggingRedactionInterceptor.cs @@ -33,6 +33,7 @@ internal sealed class HttpLoggingRedactionInterceptor : IHttpLoggingInterceptor private readonly HeaderReader _responseHeadersReader; private readonly string[] _excludePathStartsWith; private readonly FrozenDictionary _parametersToRedactMap; + private readonly bool _reportUnmatchedRoutes; public HttpLoggingRedactionInterceptor( IOptions options, @@ -59,6 +60,7 @@ public HttpLoggingRedactionInterceptor( _responseHeadersReader = new(optionsValue.ResponseHeadersDataClasses, redactorProvider, HttpLoggingTagNames.ResponseHeaderPrefix); _excludePathStartsWith = optionsValue.ExcludePathStartsWith.ToArray(); + _reportUnmatchedRoutes = optionsValue.ReportUnmatchedRoutes; } public ValueTask OnRequestAsync(HttpLoggingInterceptorContext logContext) @@ -115,6 +117,10 @@ public ValueTask OnRequestAsync(HttpLoggingInterceptorContext logContext) } } } + else if (_reportUnmatchedRoutes) + { + path = context.Request.Path.ToString(); + } } else if (request.Path.HasValue) { diff --git a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs index 0f6f1324e97..f848ef941b3 100644 --- a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs +++ b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs @@ -100,6 +100,11 @@ public class LoggingRedactionOptions #pragma warning disable CA2227 // Collection properties should be read only public ISet ExcludePathStartsWith { get; set; } = new HashSet(StringComparer.OrdinalIgnoreCase); #pragma warning restore CA2227 // Collection properties should be read only + + /// + /// Gets or sets a value indicating whether to report unmatched routes. + /// + public bool ReportUnmatchedRoutes { get; set; } = false; } #endif diff --git a/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs b/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs index 869b0716e17..19c15b0bad4 100644 --- a/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs +++ b/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs @@ -224,13 +224,13 @@ await RunAsync( x.MediaTypeOptions.Clear(); x.MediaTypeOptions.AddText("text/*"); x.LoggingFields |= HttpLoggingFields.RequestBody; - }).AddHttpLoggingRedaction(), + }).AddHttpLoggingRedaction(options => options.ReportUnmatchedRoutes = true), async (logCollector, client) => { const string Content = "Client: hello!"; using var content = new StringContent(Content, null, requestContentType); - using var response = await client.PostAsync("/", content).ConfigureAwait(false); + using var response = await client.PostAsync("/myroute/123", content).ConfigureAwait(false); Assert.True(response.IsSuccessStatusCode); await WaitForLogRecordsAsync(logCollector, _defaultLogTimeout); @@ -247,7 +247,7 @@ await RunAsync( Assert.DoesNotContain(state, x => x.Key.StartsWith(HttpLoggingTagNames.RequestHeaderPrefix)); Assert.DoesNotContain(state, x => x.Key.StartsWith(HttpLoggingTagNames.ResponseHeaderPrefix)); Assert.Single(state, x => x.Key == HttpLoggingTagNames.Host && !string.IsNullOrEmpty(x.Value)); - Assert.Single(state, x => x.Key == HttpLoggingTagNames.Path && x.Value == TelemetryConstants.Unknown); + Assert.Single(state, x => x.Key == HttpLoggingTagNames.Path && x.Value == "/myroute/123"); Assert.Single(state, x => x.Key == HttpLoggingTagNames.StatusCode && x.Value == responseStatus); Assert.Single(state, x => x.Key == HttpLoggingTagNames.Method && x.Value == HttpMethod.Post.ToString()); Assert.Single(state, x => x.Key == HttpLoggingTagNames.Duration && From f2e8adee321e5de161a25dc6728c42cfd0853c78 Mon Sep 17 00:00:00 2001 From: amadeuszl Date: Wed, 12 Mar 2025 11:31:03 +0100 Subject: [PATCH 2/4] Refactor --- .../HttpLoggingRedactionInterceptor.cs | 6 +-- .../Logging/LoggingRedactionOptions.cs | 8 +++- .../Logging/AcceptanceTests.cs | 47 ++++++++++++++++++- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/HttpLoggingRedactionInterceptor.cs b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/HttpLoggingRedactionInterceptor.cs index 0ebd099d41c..31a253f68db 100644 --- a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/HttpLoggingRedactionInterceptor.cs +++ b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/HttpLoggingRedactionInterceptor.cs @@ -33,7 +33,7 @@ internal sealed class HttpLoggingRedactionInterceptor : IHttpLoggingInterceptor private readonly HeaderReader _responseHeadersReader; private readonly string[] _excludePathStartsWith; private readonly FrozenDictionary _parametersToRedactMap; - private readonly bool _reportUnmatchedRoutes; + private readonly bool _includeUnmatchedRoutes; public HttpLoggingRedactionInterceptor( IOptions options, @@ -60,7 +60,7 @@ public HttpLoggingRedactionInterceptor( _responseHeadersReader = new(optionsValue.ResponseHeadersDataClasses, redactorProvider, HttpLoggingTagNames.ResponseHeaderPrefix); _excludePathStartsWith = optionsValue.ExcludePathStartsWith.ToArray(); - _reportUnmatchedRoutes = optionsValue.ReportUnmatchedRoutes; + _includeUnmatchedRoutes = optionsValue.IncludeUnmatchedRoutes; } public ValueTask OnRequestAsync(HttpLoggingInterceptorContext logContext) @@ -117,7 +117,7 @@ public ValueTask OnRequestAsync(HttpLoggingInterceptorContext logContext) } } } - else if (_reportUnmatchedRoutes) + else if (_includeUnmatchedRoutes) { path = context.Request.Path.ToString(); } diff --git a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs index f848ef941b3..51f8d77aaf6 100644 --- a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs +++ b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs @@ -104,7 +104,13 @@ public class LoggingRedactionOptions /// /// Gets or sets a value indicating whether to report unmatched routes. /// - public bool ReportUnmatchedRoutes { get; set; } = false; + /// + /// If set to true, instead of logging unknown value for path attribute it will log whole path of routes not identified by ASP.NET Routing. + /// + /// Defaults to . +#pragma warning disable CA1805 // Do not initialize unnecessarily + public bool IncludeUnmatchedRoutes { get; set; } = false; +#pragma warning restore CA1805 // Do not initialize unnecessarily } #endif diff --git a/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs b/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs index 19c15b0bad4..6df17895cac 100644 --- a/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs +++ b/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs @@ -224,7 +224,7 @@ await RunAsync( x.MediaTypeOptions.Clear(); x.MediaTypeOptions.AddText("text/*"); x.LoggingFields |= HttpLoggingFields.RequestBody; - }).AddHttpLoggingRedaction(options => options.ReportUnmatchedRoutes = true), + }).AddHttpLoggingRedaction(), async (logCollector, client) => { const string Content = "Client: hello!"; @@ -247,7 +247,7 @@ await RunAsync( Assert.DoesNotContain(state, x => x.Key.StartsWith(HttpLoggingTagNames.RequestHeaderPrefix)); Assert.DoesNotContain(state, x => x.Key.StartsWith(HttpLoggingTagNames.ResponseHeaderPrefix)); Assert.Single(state, x => x.Key == HttpLoggingTagNames.Host && !string.IsNullOrEmpty(x.Value)); - Assert.Single(state, x => x.Key == HttpLoggingTagNames.Path && x.Value == "/myroute/123"); + Assert.Single(state, x => x.Key == HttpLoggingTagNames.Path && x.Value == TelemetryConstants.Unknown); Assert.Single(state, x => x.Key == HttpLoggingTagNames.StatusCode && x.Value == responseStatus); Assert.Single(state, x => x.Key == HttpLoggingTagNames.Method && x.Value == HttpMethod.Post.ToString()); Assert.Single(state, x => x.Key == HttpLoggingTagNames.Duration && @@ -267,6 +267,49 @@ await RunAsync( }); } + [Fact] + public async Task HttpLogging_WhenIncludeUnmatchedRoutes_LogRequestPath() + { + await RunAsync( + LogLevel.Information, + services => services.AddHttpLogging(x => + { + x.MediaTypeOptions.Clear(); + x.MediaTypeOptions.AddText("text/*"); + x.LoggingFields |= HttpLoggingFields.RequestBody; + }).AddHttpLoggingRedaction(options => options.IncludeUnmatchedRoutes = true), + async (logCollector, client) => + { + const string Content = "Client: hello!"; + + using var content = new StringContent(Content, null, MediaTypeNames.Text.Html); + using var response = await client.PostAsync("/myroute/123", content).ConfigureAwait(false); + Assert.True(response.IsSuccessStatusCode); + + await WaitForLogRecordsAsync(logCollector, _defaultLogTimeout); + + Assert.Equal(1, logCollector.Count); + Assert.Null(logCollector.LatestRecord.Exception); + Assert.Equal(LogLevel.Information, logCollector.LatestRecord.Level); + Assert.Equal(LoggingCategory, logCollector.LatestRecord.Category); + + var responseStatus = ((int)response.StatusCode).ToInvariantString(); + var state = logCollector.LatestRecord.StructuredState!; + + Assert.DoesNotContain(state, x => x.Key == HttpLoggingTagNames.ResponseBody); + Assert.DoesNotContain(state, x => x.Key.StartsWith(HttpLoggingTagNames.RequestHeaderPrefix)); + Assert.DoesNotContain(state, x => x.Key.StartsWith(HttpLoggingTagNames.ResponseHeaderPrefix)); + Assert.Single(state, x => x.Key == HttpLoggingTagNames.Host && !string.IsNullOrEmpty(x.Value)); + Assert.Single(state, x => x.Key == HttpLoggingTagNames.Path && x.Value == "/myroute/123"); + Assert.Single(state, x => x.Key == HttpLoggingTagNames.StatusCode && x.Value == responseStatus); + Assert.Single(state, x => x.Key == HttpLoggingTagNames.Method && x.Value == HttpMethod.Post.ToString()); + Assert.Single(state, x => x.Key == HttpLoggingTagNames.Duration && + x.Value != null && + int.Parse(x.Value, CultureInfo.InvariantCulture) == SlashRouteProcessingTimeMs); + }); + } + + [Fact] public async Task HttpLogging_WhenLogLevelInfo_LogRequestStart() { From 0d6f752495367d8fe0e0221fc3cdd5c98aae92c4 Mon Sep 17 00:00:00 2001 From: amadeuszl Date: Thu, 13 Mar 2025 16:45:33 +0100 Subject: [PATCH 3/4] Refactor --- .../Logging/LoggingRedactionOptions.cs | 2 +- .../Logging/AcceptanceTests.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs index 51f8d77aaf6..3f3540184fe 100644 --- a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs +++ b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs @@ -107,7 +107,7 @@ public class LoggingRedactionOptions /// /// If set to true, instead of logging unknown value for path attribute it will log whole path of routes not identified by ASP.NET Routing. /// - /// Defaults to . + /// Defaults to . #pragma warning disable CA1805 // Do not initialize unnecessarily public bool IncludeUnmatchedRoutes { get; set; } = false; #pragma warning restore CA1805 // Do not initialize unnecessarily diff --git a/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs b/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs index 6df17895cac..01a962f2f90 100644 --- a/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs +++ b/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs @@ -309,7 +309,6 @@ await RunAsync( }); } - [Fact] public async Task HttpLogging_WhenLogLevelInfo_LogRequestStart() { From ad7a81475a6c87f2e623fdcff0b8bebfbb20fd75 Mon Sep 17 00:00:00 2001 From: amadeuszl Date: Fri, 14 Mar 2025 08:24:50 +0100 Subject: [PATCH 4/4] Refactor --- .../Logging/LoggingRedactionOptions.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs index 3f3540184fe..d2cb34ac547 100644 --- a/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs +++ b/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs @@ -108,9 +108,7 @@ public class LoggingRedactionOptions /// If set to true, instead of logging unknown value for path attribute it will log whole path of routes not identified by ASP.NET Routing. /// /// Defaults to . -#pragma warning disable CA1805 // Do not initialize unnecessarily - public bool IncludeUnmatchedRoutes { get; set; } = false; -#pragma warning restore CA1805 // Do not initialize unnecessarily + public bool IncludeUnmatchedRoutes { get; set; } } #endif