From 47c63f7681c15d8791da8d898880500e1328b48a Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Wed, 27 Dec 2023 20:05:44 +0100 Subject: [PATCH 01/12] Support content headers in HttpClient logging --- .../MediumHttpClientLoggingBenchmark.cs | 16 ++--- .../SmallHttpClientLoggingBenchmark.cs | 16 ++--- .../HttpClientFactory.cs | 16 ++--- .../Program.cs | 2 +- .../Logging/Internal/HttpHeadersReader.cs | 32 +++++---- .../Logging/AcceptanceTests.cs | 1 + .../Logging/HttpHeadersReaderTest.cs | 70 ++++++++++++++++++- ...s.cs => TelemetryCommonExtensionsTests.cs} | 2 +- 8 files changed, 115 insertions(+), 40 deletions(-) rename test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/{TelemetryCommonExtensions2Tests.cs => TelemetryCommonExtensionsTests.cs} (98%) diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/MediumHttpClientLoggingBenchmark.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/MediumHttpClientLoggingBenchmark.cs index cd9b1604bf7..fea8ec17e53 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/MediumHttpClientLoggingBenchmark.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/MediumHttpClientLoggingBenchmark.cs @@ -16,28 +16,28 @@ public class MediumHttpClientLoggingBenchmark private const int ReadSizeLimit = 16384; private static HttpRequestMessage Request => new(HttpMethod.Post, "https://www.microsoft.com"); - private static readonly System.Net.Http.HttpClient _mediumNoLog + private static readonly HttpClient _mediumNoLog = HttpClientFactory.CreateWithoutLogging(DataFileName); - private static readonly System.Net.Http.HttpClient _mediumLogAll + private static readonly HttpClient _mediumLogAll = HttpClientFactory.CreateWithLoggingLogAll(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _mediumLogRequest + private static readonly HttpClient _mediumLogRequest = HttpClientFactory.CreateWithLoggingLogRequest(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _mediumLogResponse + private static readonly HttpClient _mediumLogResponse = HttpClientFactory.CreateWithLoggingLogResponse(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _mediumNoLogChunked + private static readonly HttpClient _mediumNoLogChunked = HttpClientFactory.CreateWithoutLogging_ChunkedEncoding(DataFileName); - private static readonly System.Net.Http.HttpClient _mediumLogAllChunked + private static readonly HttpClient _mediumLogAllChunked = HttpClientFactory.CreateWithLoggingLogAll_ChunkedEncoding(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _mediumLogRequestChunked + private static readonly HttpClient _mediumLogRequestChunked = HttpClientFactory.CreateWithLoggingLogRequest_ChunkedEncoding(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _mediumLogResponseChunked + private static readonly HttpClient _mediumLogResponseChunked = HttpClientFactory.CreateWithLoggingLogResponse_ChunkedEncoding(DataFileName, ReadSizeLimit); [Benchmark(Baseline = true)] diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/SmallHttpClientLoggingBenchmark.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/SmallHttpClientLoggingBenchmark.cs index e21d0852e60..1689ceada92 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/SmallHttpClientLoggingBenchmark.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/SmallHttpClientLoggingBenchmark.cs @@ -16,28 +16,28 @@ public class SmallHttpClientLoggingBenchmark private const int ReadSizeLimit = 8192; private static HttpRequestMessage Request => new(HttpMethod.Post, "https://www.microsoft.com"); - private static readonly System.Net.Http.HttpClient _smallNoLog + private static readonly HttpClient _smallNoLog = HttpClientFactory.CreateWithoutLogging(DataFileName); - private static readonly System.Net.Http.HttpClient _smallLogAll + private static readonly HttpClient _smallLogAll = HttpClientFactory.CreateWithLoggingLogAll(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _smallLogRequest + private static readonly HttpClient _smallLogRequest = HttpClientFactory.CreateWithLoggingLogRequest(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _smallLogResponse + private static readonly HttpClient _smallLogResponse = HttpClientFactory.CreateWithLoggingLogResponse(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _smallNoLogChunked + private static readonly HttpClient _smallNoLogChunked = HttpClientFactory.CreateWithoutLogging_ChunkedEncoding(DataFileName); - private static readonly System.Net.Http.HttpClient _smallLogAllChunked + private static readonly HttpClient _smallLogAllChunked = HttpClientFactory.CreateWithLoggingLogAll_ChunkedEncoding(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _smallLogRequestChunked + private static readonly HttpClient _smallLogRequestChunked = HttpClientFactory.CreateWithLoggingLogRequest_ChunkedEncoding(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _smallLogResponseChunked + private static readonly HttpClient _smallLogResponseChunked = HttpClientFactory.CreateWithLoggingLogResponse_ChunkedEncoding(DataFileName, ReadSizeLimit); [Benchmark(Baseline = true)] diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs index 8cef086ae76..ee858a66efa 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs @@ -11,7 +11,7 @@ namespace Microsoft.Extensions.Http.Logging.Bench; internal static class HttpClientFactory { - public static System.Net.Http.HttpClient CreateWithLoggingLogRequest(string fileName, int readLimit) + public static HttpClient CreateWithLoggingLogRequest(string fileName, int readLimit) { var services = new ServiceCollection(); @@ -35,7 +35,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogRequest(string file .CreateClient(nameof(fileName)); } - public static System.Net.Http.HttpClient CreateWithLoggingLogResponse(string fileName, int readLimit) + public static HttpClient CreateWithLoggingLogResponse(string fileName, int readLimit) { var services = new ServiceCollection(); @@ -58,7 +58,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogResponse(string fil .CreateClient(nameof(fileName)); } - public static System.Net.Http.HttpClient CreateWithLoggingLogAll(string fileName, int readLimit) + public static HttpClient CreateWithLoggingLogAll(string fileName, int readLimit) { var services = new ServiceCollection(); @@ -85,7 +85,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogAll(string fileName .CreateClient(nameof(fileName)); } - public static System.Net.Http.HttpClient CreateWithLoggingLogRequest_ChunkedEncoding(string fileName, int readLimit) + public static HttpClient CreateWithLoggingLogRequest_ChunkedEncoding(string fileName, int readLimit) { var services = new ServiceCollection(); @@ -108,7 +108,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogRequest_ChunkedEnco .CreateClient(nameof(fileName)); } - public static System.Net.Http.HttpClient CreateWithLoggingLogResponse_ChunkedEncoding(string fileName, int readLimit) + public static HttpClient CreateWithLoggingLogResponse_ChunkedEncoding(string fileName, int readLimit) { var services = new ServiceCollection(); @@ -131,7 +131,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogResponse_ChunkedEnc .CreateClient(nameof(fileName)); } - public static System.Net.Http.HttpClient CreateWithLoggingLogAll_ChunkedEncoding(string fileName, int readLimit) + public static HttpClient CreateWithLoggingLogAll_ChunkedEncoding(string fileName, int readLimit) { var services = new ServiceCollection(); @@ -158,7 +158,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogAll_ChunkedEncoding .CreateClient(nameof(fileName)); } - public static System.Net.Http.HttpClient CreateWithoutLogging(string fileName) + public static HttpClient CreateWithoutLogging(string fileName) => new ServiceCollection() .AddSingleton(_ => NoRemoteCallHandler.Create(fileName)) .AddHttpClient(nameof(fileName)) @@ -168,7 +168,7 @@ public static System.Net.Http.HttpClient CreateWithoutLogging(string fileName) .GetRequiredService() .CreateClient(nameof(fileName)); - public static System.Net.Http.HttpClient CreateWithoutLogging_ChunkedEncoding(string fileName) + public static HttpClient CreateWithoutLogging_ChunkedEncoding(string fileName) => new ServiceCollection() .AddSingleton(_ => NoRemoteCallNotSeekableHandler.Create(fileName)) .AddHttpClient(nameof(fileName)) diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Program.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Program.cs index 9566c263011..72c9a47de43 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Program.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Program.cs @@ -18,7 +18,7 @@ private static void Main(string[] args) var dontRequireSlnToRunBenchmarks = ManualConfig .Create(DefaultConfig.Instance) .AddJob(Job.MediumRun - .WithRuntime(CoreRuntime.Core50) + .WithRuntime(CoreRuntime.Core80) .WithGcServer(true) .WithJit(Jit.RyuJit) .WithPlatform(Platform.X64) diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs index 9044ef4ab02..fb5239c8047 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs @@ -15,8 +15,8 @@ namespace Microsoft.Extensions.Http.Logging.Internal; internal sealed class HttpHeadersReader : IHttpHeadersReader { - private readonly FrozenDictionary _requestHeaders; - private readonly FrozenDictionary _responseHeaders; + private readonly FrozenDictionary _requestHeadersToLog; + private readonly FrozenDictionary _responseHeadersToLog; private readonly IHttpHeadersRedactor _redactor; public HttpHeadersReader(IOptionsMonitor optionsMonitor, IHttpHeadersRedactor redactor, [ServiceKey] string? serviceKey = null) @@ -25,8 +25,8 @@ public HttpHeadersReader(IOptionsMonitor optionsMonitor, IHttpHe _redactor = redactor; - _requestHeaders = options.RequestHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); - _responseHeaders = options.ResponseHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); + _requestHeadersToLog = options.RequestHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); + _responseHeadersToLog = options.ResponseHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); } public void ReadRequestHeaders(HttpRequestMessage request, List>? destination) @@ -36,7 +36,11 @@ public void ReadRequestHeaders(HttpRequestMessage request, List>? destination) @@ -46,19 +50,21 @@ public void ReadResponseHeaders(HttpResponseMessage response, List headersToLog, List> destination) + // TODO: test whether this implementation is generally more optimal than the previous one + private void ReadHeaders(HttpHeaders headers, FrozenDictionary headersToLog, List> destination) { - foreach (var kvp in headersToLog) + foreach (var header in headers) { - var classification = kvp.Value; - var header = kvp.Key; - - if (requestHeaders.TryGetValues(header, out var values)) + if (headersToLog.TryGetValue(header.Key, out var classification)) { - destination.Add(new(header, _redactor.Redact(values, classification))); + destination.Add(new(header.Key, _redactor.Redact(header.Value, classification))); } } } diff --git a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/AcceptanceTests.cs b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/AcceptanceTests.cs index 603f7346d16..93e3cb8b340 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/AcceptanceTests.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/AcceptanceTests.cs @@ -563,6 +563,7 @@ public async Task AddDefaultHttpClientLogging_DisablesNetScope() .AddExtendedHttpClientLogging() .BlockRemoteCall() .BuildServiceProvider(); + var options = provider.GetRequiredService>().Get("test"); var client = provider.GetRequiredService().CreateClient("test"); using var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, _unreachableRequestUri); diff --git a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs index a32a649b62e..d7011726731 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Net.Http; +using System.Net.Mime; using FluentAssertions; using Microsoft.Extensions.Compliance.Classification; using Microsoft.Extensions.Compliance.Testing; @@ -59,7 +60,7 @@ public void HttpHeadersReader_WhenHeadersProvided_ReadsThem() { "Header3", FakeTaxonomy.PublicData }, { "Header4", FakeTaxonomy.PublicData }, { "hEaDeR7", FakeTaxonomy.PrivateData } - }, + } }; var headersReader = new HttpHeadersReader(options.ToOptionsMonitor(), mockHeadersRedactor.Object); @@ -86,6 +87,7 @@ public void HttpHeadersReader_WhenHeadersProvided_ReadsThem() new KeyValuePair("Header1", Redacted), new KeyValuePair("Header2", Redacted) }; + var expectedResponse = new[] { new KeyValuePair("Header3", "Value.3"), @@ -100,6 +102,72 @@ public void HttpHeadersReader_WhenHeadersProvided_ReadsThem() responseBuffer.Should().BeEquivalentTo(expectedResponse); } + [Fact] + public void HttpHeadersReader_WhenProvided_ReadsContentHeaders() + { + var mockHeadersRedactor = new Mock(); + mockHeadersRedactor.Setup(r => r.Redact(It.IsAny>(), FakeTaxonomy.PublicData)) + .Returns, DataClassification>((x, _) => string.Join(",", x)); + + var options = new LoggingOptions + { + RequestHeadersDataClasses = new Dictionary + { + { "Header1", FakeTaxonomy.PublicData }, + { "Content-Header1", FakeTaxonomy.PublicData }, + { "Content-Type", FakeTaxonomy.PublicData } + }, + ResponseHeadersDataClasses = new Dictionary + { + { "Header3", FakeTaxonomy.PublicData }, + { "Content-Header2", FakeTaxonomy.PublicData }, + { "Content-Length", FakeTaxonomy.PublicData } + } + }; + + var headersReader = new HttpHeadersReader(options.ToOptionsMonitor(), mockHeadersRedactor.Object); + + using var requestContent = new StringContent(string.Empty); + requestContent.Headers.ContentType = new(MediaTypeNames.Application.Soap); + requestContent.Headers.ContentLength = 42; + requestContent.Headers.Add("Content-Header1", "Value.8"); + + using var httpRequest = new HttpRequestMessage { Content = requestContent }; + httpRequest.Headers.Add("Header1", "Value.1"); + httpRequest.Headers.Add("Header2", "Value.2"); + + using var responseContent = new StringContent(string.Empty); + responseContent.Headers.ContentType = new(MediaTypeNames.Text.Html); + responseContent.Headers.ContentLength = 24; + responseContent.Headers.Add("Content-Header2", "Value.9"); + + using var httpResponse = new HttpResponseMessage { Content = responseContent }; + httpResponse.Headers.Add("Header3", "Value.3"); + httpResponse.Headers.Add("Header4", "Value.4"); + + var expectedRequest = new[] + { + new KeyValuePair("Header1", "Value.1"), + new KeyValuePair("Content-Header1", "Value.8"), + new KeyValuePair("Content-Type", MediaTypeNames.Application.Soap), + }; + + var expectedResponse = new[] + { + new KeyValuePair("Header3", "Value.3"), + new KeyValuePair("Content-Header2", "Value.9"), + new KeyValuePair("Content-Length", "24"), + }; + + List> requestBuffer = []; + headersReader.ReadRequestHeaders(httpRequest, requestBuffer); + requestBuffer.Should().BeEquivalentTo(expectedRequest); + + List> responseBuffer = []; + headersReader.ReadResponseHeaders(httpResponse, responseBuffer); + responseBuffer.Should().BeEquivalentTo(expectedResponse); + } + [Fact] public void HttpHeadersReader_WhenBufferIsNull_DoesNothing() { diff --git a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/TelemetryCommonExtensions2Tests.cs b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/TelemetryCommonExtensionsTests.cs similarity index 98% rename from test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/TelemetryCommonExtensions2Tests.cs rename to test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/TelemetryCommonExtensionsTests.cs index 85f7791eeb2..a7a404079b6 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/TelemetryCommonExtensions2Tests.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/TelemetryCommonExtensionsTests.cs @@ -8,7 +8,7 @@ namespace Microsoft.Extensions.Telemetry.Internal.Test; -public class TelemetryCommonExtensions2Tests +public class TelemetryCommonExtensionsTests { [Fact] public void GetDependencyName_DependencyNameMissing_ReturnsUnknown() From f61ab6b2c5907ff3241dfded0c24d342efa41f4b Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Thu, 28 Dec 2023 15:15:04 +0100 Subject: [PATCH 02/12] fix tests; add benchmark --- .../Benchmarks/HeadersReaderBenchmark.cs | 50 +++++++++++++++++++ .../HugeHttpCLientLoggingBenchmark.cs | 16 +++--- .../HttpClientFactory.cs | 8 +-- .../NoRemoteCallHandler.cs | 5 +- .../StaticOptionsMonitor.cs | 23 +++++++++ .../Logging/HttpHeadersReaderTest.cs | 12 ++--- 6 files changed, 92 insertions(+), 22 deletions(-) create mode 100644 bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs create mode 100644 bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/StaticOptionsMonitor.cs diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs new file mode 100644 index 00000000000..d2b2490aa4b --- /dev/null +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs @@ -0,0 +1,50 @@ +// 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.Generic; +using System.Net.Http; +using BenchmarkDotNet.Attributes; +using Microsoft.Extensions.Compliance.Redaction; +using Microsoft.Extensions.Compliance.Testing; +using Microsoft.Extensions.Http.Logging; +using Microsoft.Extensions.Http.Logging.Internal; +using Microsoft.Extensions.Telemetry.Internal; + +namespace Microsoft.Extensions.Http.Diagnostics.Bench.Benchmarks; + +public class HeadersReaderBenchmark +{ + private readonly HttpHeadersReader _headersReader; + private readonly List> _outputBuffer = new(capacity: 1024); + + [Params(0, 5, 15)] + public int HeadersCount { get; set; } + + [Params(0, 3, 5, 10)] + public int HeadersToLogCount { get; set; } + public HttpRequestMessage Request { get; } + + public HeadersReaderBenchmark() + { + Request = new HttpRequestMessage(HttpMethod.Post, "https://www.microsoft.com"); + for (var i = 0; i < HeadersCount; i++) + { + Request.Headers.Add($"Header{i}", $"Value{i}"); + } + + var options = new LoggingOptions(); + for (var i = 0; i < HeadersToLogCount; i++) + { + options.RequestHeadersDataClasses.Add($"Header{i}", FakeTaxonomy.PublicData); + } + + var redactor = new HttpHeadersRedactor(NullRedactorProvider.Instance); + _headersReader = new HttpHeadersReader(new StaticOptionsMonitor(options), redactor); + } + + [Benchmark] + public void HeadersReader() + { + _headersReader.ReadRequestHeaders(Request, _outputBuffer); + } +} diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HugeHttpCLientLoggingBenchmark.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HugeHttpCLientLoggingBenchmark.cs index b889a4e6e3b..edc19460f4b 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HugeHttpCLientLoggingBenchmark.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HugeHttpCLientLoggingBenchmark.cs @@ -16,28 +16,28 @@ public class HugeHttpClientLoggingBenchmark private const int ReadSizeLimit = 53248; private static HttpRequestMessage Request => new(HttpMethod.Post, "https://www.microsoft.com"); - private static readonly System.Net.Http.HttpClient _hugeNoLog + private static readonly HttpClient _hugeNoLog = HttpClientFactory.CreateWithoutLogging(DataFileName); - private static readonly System.Net.Http.HttpClient _hugeLogAll + private static readonly HttpClient _hugeLogAll = HttpClientFactory.CreateWithLoggingLogAll(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _hugeLogRequest + private static readonly HttpClient _hugeLogRequest = HttpClientFactory.CreateWithLoggingLogRequest(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _hugeLogResponse + private static readonly HttpClient _hugeLogResponse = HttpClientFactory.CreateWithLoggingLogResponse(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _hugeNoLogChunked + private static readonly HttpClient _hugeNoLogChunked = HttpClientFactory.CreateWithoutLogging_ChunkedEncoding(DataFileName); - private static readonly System.Net.Http.HttpClient _hugeLogAllChunked + private static readonly HttpClient _hugeLogAllChunked = HttpClientFactory.CreateWithLoggingLogAll_ChunkedEncoding(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _hugeLogRequestChunked + private static readonly HttpClient _hugeLogRequestChunked = HttpClientFactory.CreateWithLoggingLogRequest_ChunkedEncoding(DataFileName, ReadSizeLimit); - private static readonly System.Net.Http.HttpClient _hugeLogResponseChunked + private static readonly HttpClient _hugeLogResponseChunked = HttpClientFactory.CreateWithLoggingLogResponse_ChunkedEncoding(DataFileName, ReadSizeLimit); [Benchmark(Baseline = true)] diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs index ee858a66efa..4aac36f8ccd 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs @@ -25,7 +25,7 @@ public static HttpClient CreateWithLoggingLogRequest(string fileName, int readLi .AddExtendedHttpClientLogging(options => { options.BodySizeLimit = readLimit; - options.RequestBodyContentTypes.Add(new("application/json")); + options.RequestBodyContentTypes.Add("application/json"); options.RequestHeadersDataClasses.Add("Content-Type", FakeTaxonomy.PrivateData); }) .AddHttpMessageHandler() @@ -48,7 +48,7 @@ public static HttpClient CreateWithLoggingLogResponse(string fileName, int readL .AddExtendedHttpClientLogging(options => { options.BodySizeLimit = readLimit; - options.ResponseBodyContentTypes.Add(new("application/json")); + options.ResponseBodyContentTypes.Add("application/json"); options.ResponseHeadersDataClasses.Add("Content-Type", FakeTaxonomy.PrivateData); }) .AddHttpMessageHandler() @@ -72,10 +72,10 @@ public static HttpClient CreateWithLoggingLogAll(string fileName, int readLimit) { options.BodySizeLimit = readLimit; - options.RequestBodyContentTypes.Add(new("application/json")); + options.RequestBodyContentTypes.Add("application/json"); options.RequestHeadersDataClasses.Add("Content-Type", FakeTaxonomy.PrivateData); - options.ResponseBodyContentTypes.Add(new("application/json")); + options.ResponseBodyContentTypes.Add("application/json"); options.ResponseHeadersDataClasses.Add("Content-Type", FakeTaxonomy.PrivateData); }) .AddHttpMessageHandler() diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/NoRemoteCallHandler.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/NoRemoteCallHandler.cs index 997b1b783da..78d12b285d5 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/NoRemoteCallHandler.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/NoRemoteCallHandler.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Diagnostics.CodeAnalysis; using System.IO; using System.Net.Http; using System.Threading; @@ -19,8 +18,6 @@ private NoRemoteCallHandler(byte[] data) _data = data; } - [SuppressMessage("Performance Analysis", "CPR120:File.ReadAllXXX should be replaced by using a StreamReader to avoid adding objects to the large object heap (LOH).", - Justification = "We can live with it here")] public static NoRemoteCallHandler Create(string fileName) { var assemblyFileLocation = Path.GetDirectoryName(typeof(NoRemoteCallHandler).Assembly.Location)!; @@ -41,7 +38,7 @@ protected override Task SendAsync(HttpRequestMessage reques { StatusCode = System.Net.HttpStatusCode.OK, RequestMessage = request, - Content = new StreamContent(new MemoryStream(_data)) + Content = new StreamContent(new MemoryStream(_data, writable: false)) }; response.Content.Headers.ContentType = new("application/json"); diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/StaticOptionsMonitor.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/StaticOptionsMonitor.cs new file mode 100644 index 00000000000..615fc33cb6c --- /dev/null +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/StaticOptionsMonitor.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.Http.Diagnostics.Bench; + +internal sealed class StaticOptionsMonitor : IOptionsMonitor +{ + public StaticOptionsMonitor(T options) + { + CurrentValue = options; + } + + public T CurrentValue { get; } + + public T Get(string? name) + => CurrentValue; + + public IDisposable OnChange(Action listener) + => throw new NotSupportedException(); +} diff --git a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs index d7011726731..28278292d12 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs @@ -59,7 +59,7 @@ public void HttpHeadersReader_WhenHeadersProvided_ReadsThem() { { "Header3", FakeTaxonomy.PublicData }, { "Header4", FakeTaxonomy.PublicData }, - { "hEaDeR7", FakeTaxonomy.PrivateData } + { "hEaDeR7", FakeTaxonomy.PrivateData } // This one is to test case-insensitivity } }; @@ -92,7 +92,7 @@ public void HttpHeadersReader_WhenHeadersProvided_ReadsThem() { new KeyValuePair("Header3", "Value.3"), new KeyValuePair("Header4", "Value.4"), - new KeyValuePair("hEaDeR7", Redacted), + new KeyValuePair("Header7", Redacted), }; headersReader.ReadRequestHeaders(httpRequest, requestBuffer); @@ -130,7 +130,7 @@ public void HttpHeadersReader_WhenProvided_ReadsContentHeaders() using var requestContent = new StringContent(string.Empty); requestContent.Headers.ContentType = new(MediaTypeNames.Application.Soap); requestContent.Headers.ContentLength = 42; - requestContent.Headers.Add("Content-Header1", "Value.8"); + requestContent.Headers.Add("Content-Header1", "Content.1"); using var httpRequest = new HttpRequestMessage { Content = requestContent }; httpRequest.Headers.Add("Header1", "Value.1"); @@ -139,7 +139,7 @@ public void HttpHeadersReader_WhenProvided_ReadsContentHeaders() using var responseContent = new StringContent(string.Empty); responseContent.Headers.ContentType = new(MediaTypeNames.Text.Html); responseContent.Headers.ContentLength = 24; - responseContent.Headers.Add("Content-Header2", "Value.9"); + responseContent.Headers.Add("Content-Header2", "Content.2"); using var httpResponse = new HttpResponseMessage { Content = responseContent }; httpResponse.Headers.Add("Header3", "Value.3"); @@ -148,14 +148,14 @@ public void HttpHeadersReader_WhenProvided_ReadsContentHeaders() var expectedRequest = new[] { new KeyValuePair("Header1", "Value.1"), - new KeyValuePair("Content-Header1", "Value.8"), + new KeyValuePair("Content-Header1", "Content.1"), new KeyValuePair("Content-Type", MediaTypeNames.Application.Soap), }; var expectedResponse = new[] { new KeyValuePair("Header3", "Value.3"), - new KeyValuePair("Content-Header2", "Value.9"), + new KeyValuePair("Content-Header2", "Content.2"), new KeyValuePair("Content-Length", "24"), }; From d542bbd096dfc697c168b9007c386140ac2b16b0 Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Thu, 28 Dec 2023 17:12:01 +0100 Subject: [PATCH 03/12] add benchmark results --- .../Benchmarks/HeadersReaderBenchmark.cs | 72 +++++++++++++++++-- .../Logging/Internal/HttpHeadersReader.cs | 31 +++++++- 2 files changed, 94 insertions(+), 9 deletions(-) diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs index d2b2490aa4b..1376a7e5741 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs @@ -14,18 +14,22 @@ namespace Microsoft.Extensions.Http.Diagnostics.Bench.Benchmarks; public class HeadersReaderBenchmark { - private readonly HttpHeadersReader _headersReader; - private readonly List> _outputBuffer = new(capacity: 1024); + private List> _outputBuffer = null!; + private HttpHeadersReader _headersReader = null!; [Params(0, 5, 15)] public int HeadersCount { get; set; } - [Params(0, 3, 5, 10)] + // This one can't be 0, because in that case HttpRequestReader simply doesn't call HttpHeadersReader + [Params(1, 3, 5, 10)] public int HeadersToLogCount { get; set; } - public HttpRequestMessage Request { get; } - public HeadersReaderBenchmark() + public HttpRequestMessage? Request { get; set; } + + [GlobalSetup] + public void Setup() { + _outputBuffer = new(capacity: 10240); Request = new HttpRequestMessage(HttpMethod.Post, "https://www.microsoft.com"); for (var i = 0; i < HeadersCount; i++) { @@ -42,9 +46,63 @@ public HeadersReaderBenchmark() _headersReader = new HttpHeadersReader(new StaticOptionsMonitor(options), redactor); } + [GlobalCleanup] + public void Cleanup() + { + Request?.Dispose(); + _outputBuffer.Clear(); + _outputBuffer = null!; + } + [Benchmark] - public void HeadersReader() + public void HeadersReaderOld() { - _headersReader.ReadRequestHeaders(Request, _outputBuffer); + _headersReader.ReadRequestHeaders(Request!, _outputBuffer); + } + + [Benchmark] + public void HeadersReaderNew() + { + _headersReader.ReadRequestHeadersNew(Request!, _outputBuffer); } } + +/* + +BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22631.2861), VM=Hyper-V +Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 16 logical and 8 physical cores +.NET SDK=8.0.100 + [Host] : .NET 8.0.0 (8.0.23.53103), X64 RyuJIT AVX2 + +Job=MediumRun Jit=RyuJit Platform=X64 +Runtime=.NET 8.0 Server=True Toolchain=InProcessEmitToolchain +IterationCount=15 LaunchCount=2 WarmupCount=10 + +| Method | HeadersCount | HeadersToLogCount | Mean | Error | StdDev | Median | Gen0 | Gen1 | Allocated | +|----------------- |------------- |------------------ |------------:|----------:|-----------:|------------:|-------:|-------:|----------:| +| HeadersReaderNew | 0 | 1 | 10.40 ns | 0.013 ns | 0.018 ns | 10.40 ns | - | - | - | +| HeadersReaderNew | 0 | 5 | 10.44 ns | 0.029 ns | 0.041 ns | 10.43 ns | - | - | - | +| HeadersReaderNew | 0 | 3 | 10.69 ns | 0.216 ns | 0.295 ns | 10.69 ns | - | - | - | +| HeadersReaderNew | 0 | 10 | 10.71 ns | 0.072 ns | 0.103 ns | 10.74 ns | - | - | - | +| HeadersReaderOld | 0 | 1 | 18.90 ns | 0.071 ns | 0.100 ns | 18.89 ns | - | - | - | +| HeadersReaderOld | 0 | 3 | 41.60 ns | 0.464 ns | 0.665 ns | 41.33 ns | - | - | - | +| HeadersReaderOld | 0 | 5 | 64.16 ns | 0.655 ns | 0.919 ns | 63.49 ns | - | - | - | +| HeadersReaderOld | 0 | 10 | 120.26 ns | 0.276 ns | 0.377 ns | 120.25 ns | - | - | - | +| HeadersReaderOld | 5 | 1 | 402.43 ns | 6.976 ns | 9.549 ns | 402.17 ns | 0.0114 | 0.0057 | 296 B | +| HeadersReaderNew | 5 | 1 | 749.80 ns | 17.496 ns | 23.949 ns | 743.08 ns | 0.0191 | 0.0048 | 480 B | +| HeadersReaderOld | 5 | 3 | 1,341.08 ns | 71.636 ns | 100.423 ns | 1,337.73 ns | 0.0343 | 0.0172 | 888 B | +| HeadersReaderNew | 5 | 3 | 1,460.32 ns | 18.964 ns | 25.316 ns | 1,456.27 ns | 0.0401 | 0.0134 | 1008 B | +| HeadersReaderOld | 5 | 5 | 2,189.89 ns | 74.132 ns | 106.319 ns | 2,190.03 ns | 0.0572 | 0.0267 | 1480 B | +| HeadersReaderNew | 5 | 5 | 2,149.69 ns | 28.055 ns | 38.403 ns | 2,147.51 ns | 0.0610 | 0.0305 | 1536 B | +| HeadersReaderOld | 5 | 10 | 2,349.91 ns | 23.901 ns | 32.716 ns | 2,351.23 ns | 0.0572 | 0.0267 | 1480 B | +| HeadersReaderNew | 5 | 10 | 2,087.11 ns | 22.876 ns | 31.313 ns | 2,086.37 ns | 0.0610 | 0.0305 | 1536 B | +| HeadersReaderOld | 15 | 1 | 408.09 ns | 5.997 ns | 8.209 ns | 407.62 ns | 0.0114 | 0.0057 | 296 B | +| HeadersReaderNew | 15 | 1 | 1,404.72 ns | 7.300 ns | 9.745 ns | 1,403.54 ns | 0.0305 | 0.0076 | 800 B | +| HeadersReaderOld | 15 | 3 | 1,224.46 ns | 19.016 ns | 25.385 ns | 1,214.62 ns | 0.0343 | 0.0172 | 888 B | +| HeadersReaderNew | 15 | 3 | 2,199.79 ns | 28.790 ns | 39.407 ns | 2,191.87 ns | 0.0496 | 0.0153 | 1328 B | +| HeadersReaderOld | 15 | 5 | 2,081.42 ns | 47.198 ns | 66.165 ns | 2,065.93 ns | 0.0572 | 0.0267 | 1480 B | +| HeadersReaderNew | 15 | 5 | 2,967.58 ns | 24.105 ns | 32.995 ns | 2,962.05 ns | 0.0725 | 0.0229 | 1856 B | +| HeadersReaderOld | 15 | 10 | 4,385.35 ns | 77.832 ns | 109.109 ns | 4,362.06 ns | 0.1144 | 0.0534 | 2960 B | +| HeadersReaderNew | 15 | 10 | 4,418.11 ns | 35.622 ns | 47.555 ns | 4,408.71 ns | 0.1221 | 0.0610 | 3176 B | + + */ diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs index fb5239c8047..92bd071628e 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs @@ -43,6 +43,20 @@ public void ReadRequestHeaders(HttpRequestMessage request, List>? destination) + { + if (destination is null) + { + return; + } + + ReadHeadersNew(request.Headers, _requestHeadersToLog, destination); + if (request.Content is not null) + { + ReadHeadersNew(request.Content.Headers, _requestHeadersToLog, destination); + } + } + public void ReadResponseHeaders(HttpResponseMessage response, List>? destination) { if (destination is null) @@ -57,8 +71,7 @@ public void ReadResponseHeaders(HttpResponseMessage response, List headersToLog, List> destination) + private void ReadHeadersNew(HttpHeaders headers, FrozenDictionary headersToLog, List> destination) { foreach (var header in headers) { @@ -68,4 +81,18 @@ private void ReadHeaders(HttpHeaders headers, FrozenDictionary headersToLog, List> destination) + { + foreach (var kvp in headersToLog) + { + var classification = kvp.Value; + var header = kvp.Key; + + if (headers.TryGetValues(header, out var values)) + { + destination.Add(new(header, _redactor.Redact(values, classification))); + } + } + } } From 289899e1d80bf801efd1800ded8b947f06b53885 Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Fri, 29 Dec 2023 13:22:53 +0100 Subject: [PATCH 04/12] try another approach --- .../Benchmarks/ErasingRedactorProvider.cs | 14 ++++++++ .../Benchmarks/HeadersReaderBenchmark.cs | 2 +- ...s.Http.Diagnostics.PerformanceTests.csproj | 3 +- .../FakeRedactorProvider.cs | 2 +- .../Logging/Internal/HttpHeadersReader.cs | 34 +++++++++++++++++-- 5 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/ErasingRedactorProvider.cs diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/ErasingRedactorProvider.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/ErasingRedactorProvider.cs new file mode 100644 index 00000000000..33045d4cda6 --- /dev/null +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/ErasingRedactorProvider.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Compliance.Classification; +using Microsoft.Extensions.Compliance.Redaction; + +namespace Microsoft.Extensions.Http.Diagnostics.Bench.Benchmarks; + +internal sealed class ErasingRedactorProvider : IRedactorProvider +{ + public static ErasingRedactorProvider Instance { get; } = new(); + + public Redactor GetRedactor(DataClassificationSet classifications) => ErasingRedactor.Instance; +} diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs index 1376a7e5741..d104fbe1a84 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs @@ -42,7 +42,7 @@ public void Setup() options.RequestHeadersDataClasses.Add($"Header{i}", FakeTaxonomy.PublicData); } - var redactor = new HttpHeadersRedactor(NullRedactorProvider.Instance); + var redactor = new HttpHeadersRedactor(ErasingRedactorProvider.Instance); _headersReader = new HttpHeadersReader(new StaticOptionsMonitor(options), redactor); } diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Microsoft.Extensions.Http.Diagnostics.PerformanceTests.csproj b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Microsoft.Extensions.Http.Diagnostics.PerformanceTests.csproj index 5348f6a365a..4c5e8386e9c 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Microsoft.Extensions.Http.Diagnostics.PerformanceTests.csproj +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Microsoft.Extensions.Http.Diagnostics.PerformanceTests.csproj @@ -1,4 +1,4 @@ - + Microsoft.Extensions.Http.Diagnostics.Bench Benchmarks for Microsoft.Extensions.Http.Diagnostics. @@ -12,6 +12,7 @@ + \ No newline at end of file diff --git a/src/Libraries/Microsoft.Extensions.Compliance.Testing/FakeRedactorProvider.cs b/src/Libraries/Microsoft.Extensions.Compliance.Testing/FakeRedactorProvider.cs index 38b186688f4..c306d1d4c86 100644 --- a/src/Libraries/Microsoft.Extensions.Compliance.Testing/FakeRedactorProvider.cs +++ b/src/Libraries/Microsoft.Extensions.Compliance.Testing/FakeRedactorProvider.cs @@ -28,7 +28,7 @@ public class FakeRedactorProvider : IRedactorProvider public FakeRedactorProvider(FakeRedactorOptions? options = null, FakeRedactionCollector? eventCollector = null) { Collector = eventCollector ?? new FakeRedactionCollector(); - _redactor = new FakeRedactor(Microsoft.Extensions.Options.Options.Create(options ?? new FakeRedactorOptions()), Collector); + _redactor = new FakeRedactor(Options.Options.Create(options ?? new FakeRedactorOptions()), Collector); } /// diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs index 92bd071628e..0d432ba054b 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs @@ -17,6 +17,7 @@ internal sealed class HttpHeadersReader : IHttpHeadersReader { private readonly FrozenDictionary _requestHeadersToLog; private readonly FrozenDictionary _responseHeadersToLog; + private readonly int _headersCountThreshold; private readonly IHttpHeadersRedactor _redactor; public HttpHeadersReader(IOptionsMonitor optionsMonitor, IHttpHeadersRedactor redactor, [ServiceKey] string? serviceKey = null) @@ -27,6 +28,8 @@ public HttpHeadersReader(IOptionsMonitor optionsMonitor, IHttpHe _requestHeadersToLog = options.RequestHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); _responseHeadersToLog = options.ResponseHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); + + _headersCountThreshold = _requestHeadersToLog.Count; } public void ReadRequestHeaders(HttpRequestMessage request, List>? destination) @@ -73,11 +76,36 @@ public void ReadResponseHeaders(HttpResponseMessage response, List headersToLog, List> destination) { - foreach (var header in headers) +#if NET6_0_OR_GREATER + var nonValidated = headers.NonValidated; + if (nonValidated.Count == 0) + { + return; + } + + if (nonValidated.Count < _headersCountThreshold) + { + // We have less headers than registered for logging, iterating over the smaller collection + foreach (var header in headers) + { + if (headersToLog.TryGetValue(header.Key, out var classification)) + { + destination.Add(new(header.Key, _redactor.Redact(header.Value, classification))); + } + } + + return; + } +#endif + + foreach (var kvp in headersToLog) { - if (headersToLog.TryGetValue(header.Key, out var classification)) + var classification = kvp.Value; + var header = kvp.Key; + + if (headers.TryGetValues(header, out var values)) { - destination.Add(new(header.Key, _redactor.Redact(header.Value, classification))); + destination.Add(new(header, _redactor.Redact(values, classification))); } } } From 5eb91957ec745ec24bed50df8d20312c8eb9cefa Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Fri, 29 Dec 2023 14:02:46 +0100 Subject: [PATCH 05/12] add results --- .../Benchmarks/HeadersReaderBenchmark.cs | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs index d104fbe1a84..ccd0166487d 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Net.Http; using BenchmarkDotNet.Attributes; -using Microsoft.Extensions.Compliance.Redaction; using Microsoft.Extensions.Compliance.Testing; using Microsoft.Extensions.Http.Logging; using Microsoft.Extensions.Http.Logging.Internal; @@ -80,29 +79,45 @@ public void HeadersReaderNew() | Method | HeadersCount | HeadersToLogCount | Mean | Error | StdDev | Median | Gen0 | Gen1 | Allocated | |----------------- |------------- |------------------ |------------:|----------:|-----------:|------------:|-------:|-------:|----------:| -| HeadersReaderNew | 0 | 1 | 10.40 ns | 0.013 ns | 0.018 ns | 10.40 ns | - | - | - | -| HeadersReaderNew | 0 | 5 | 10.44 ns | 0.029 ns | 0.041 ns | 10.43 ns | - | - | - | -| HeadersReaderNew | 0 | 3 | 10.69 ns | 0.216 ns | 0.295 ns | 10.69 ns | - | - | - | -| HeadersReaderNew | 0 | 10 | 10.71 ns | 0.072 ns | 0.103 ns | 10.74 ns | - | - | - | | HeadersReaderOld | 0 | 1 | 18.90 ns | 0.071 ns | 0.100 ns | 18.89 ns | - | - | - | | HeadersReaderOld | 0 | 3 | 41.60 ns | 0.464 ns | 0.665 ns | 41.33 ns | - | - | - | | HeadersReaderOld | 0 | 5 | 64.16 ns | 0.655 ns | 0.919 ns | 63.49 ns | - | - | - | | HeadersReaderOld | 0 | 10 | 120.26 ns | 0.276 ns | 0.377 ns | 120.25 ns | - | - | - | | HeadersReaderOld | 5 | 1 | 402.43 ns | 6.976 ns | 9.549 ns | 402.17 ns | 0.0114 | 0.0057 | 296 B | -| HeadersReaderNew | 5 | 1 | 749.80 ns | 17.496 ns | 23.949 ns | 743.08 ns | 0.0191 | 0.0048 | 480 B | | HeadersReaderOld | 5 | 3 | 1,341.08 ns | 71.636 ns | 100.423 ns | 1,337.73 ns | 0.0343 | 0.0172 | 888 B | -| HeadersReaderNew | 5 | 3 | 1,460.32 ns | 18.964 ns | 25.316 ns | 1,456.27 ns | 0.0401 | 0.0134 | 1008 B | | HeadersReaderOld | 5 | 5 | 2,189.89 ns | 74.132 ns | 106.319 ns | 2,190.03 ns | 0.0572 | 0.0267 | 1480 B | -| HeadersReaderNew | 5 | 5 | 2,149.69 ns | 28.055 ns | 38.403 ns | 2,147.51 ns | 0.0610 | 0.0305 | 1536 B | | HeadersReaderOld | 5 | 10 | 2,349.91 ns | 23.901 ns | 32.716 ns | 2,351.23 ns | 0.0572 | 0.0267 | 1480 B | -| HeadersReaderNew | 5 | 10 | 2,087.11 ns | 22.876 ns | 31.313 ns | 2,086.37 ns | 0.0610 | 0.0305 | 1536 B | | HeadersReaderOld | 15 | 1 | 408.09 ns | 5.997 ns | 8.209 ns | 407.62 ns | 0.0114 | 0.0057 | 296 B | -| HeadersReaderNew | 15 | 1 | 1,404.72 ns | 7.300 ns | 9.745 ns | 1,403.54 ns | 0.0305 | 0.0076 | 800 B | | HeadersReaderOld | 15 | 3 | 1,224.46 ns | 19.016 ns | 25.385 ns | 1,214.62 ns | 0.0343 | 0.0172 | 888 B | -| HeadersReaderNew | 15 | 3 | 2,199.79 ns | 28.790 ns | 39.407 ns | 2,191.87 ns | 0.0496 | 0.0153 | 1328 B | | HeadersReaderOld | 15 | 5 | 2,081.42 ns | 47.198 ns | 66.165 ns | 2,065.93 ns | 0.0572 | 0.0267 | 1480 B | -| HeadersReaderNew | 15 | 5 | 2,967.58 ns | 24.105 ns | 32.995 ns | 2,962.05 ns | 0.0725 | 0.0229 | 1856 B | | HeadersReaderOld | 15 | 10 | 4,385.35 ns | 77.832 ns | 109.109 ns | 4,362.06 ns | 0.1144 | 0.0534 | 2960 B | + +| HeadersReaderNew | 0 | 1 | 10.40 ns | 0.013 ns | 0.018 ns | 10.40 ns | - | - | - | +| HeadersReaderNew | 0 | 5 | 10.44 ns | 0.029 ns | 0.041 ns | 10.43 ns | - | - | - | +| HeadersReaderNew | 0 | 3 | 10.69 ns | 0.216 ns | 0.295 ns | 10.69 ns | - | - | - | +| HeadersReaderNew | 0 | 10 | 10.71 ns | 0.072 ns | 0.103 ns | 10.74 ns | - | - | - | +| HeadersReaderNew | 5 | 1 | 749.80 ns | 17.496 ns | 23.949 ns | 743.08 ns | 0.0191 | 0.0048 | 480 B | +| HeadersReaderNew | 5 | 3 | 1,460.32 ns | 18.964 ns | 25.316 ns | 1,456.27 ns | 0.0401 | 0.0134 | 1008 B | +| HeadersReaderNew | 5 | 5 | 2,149.69 ns | 28.055 ns | 38.403 ns | 2,147.51 ns | 0.0610 | 0.0305 | 1536 B | +| HeadersReaderNew | 5 | 10 | 2,087.11 ns | 22.876 ns | 31.313 ns | 2,086.37 ns | 0.0610 | 0.0305 | 1536 B | +| HeadersReaderNew | 15 | 1 | 1,404.72 ns | 7.300 ns | 9.745 ns | 1,403.54 ns | 0.0305 | 0.0076 | 800 B | +| HeadersReaderNew | 15 | 3 | 2,199.79 ns | 28.790 ns | 39.407 ns | 2,191.87 ns | 0.0496 | 0.0153 | 1328 B | +| HeadersReaderNew | 15 | 5 | 2,967.58 ns | 24.105 ns | 32.995 ns | 2,962.05 ns | 0.0725 | 0.0229 | 1856 B | | HeadersReaderNew | 15 | 10 | 4,418.11 ns | 35.622 ns | 47.555 ns | 4,408.71 ns | 0.1221 | 0.0610 | 3176 B | +| Method | HeadersCount | HeadersToLogCount | Mean | Error | StdDev | Median | Gen0 | Allocated | +|----------------- |------------- |------------------ |-------------:|-----------:|-----------:|-------------:|-------:|----------:| +| HeadersReaderNew | 0 | 3 | 4.451 ns | 0.0070 ns | 0.0098 ns | 4.449 ns | - | - | +| HeadersReaderNew | 0 | 1 | 4.523 ns | 0.0114 ns | 0.0167 ns | 4.526 ns | - | - | +| HeadersReaderNew | 0 | 10 | 4.597 ns | 0.0719 ns | 0.1007 ns | 4.659 ns | - | - | +| HeadersReaderNew | 0 | 5 | 4.942 ns | 0.3811 ns | 0.5466 ns | 4.464 ns | - | - | +| HeadersReaderNew | 5 | 1 | 205.183 ns | 1.4687 ns | 1.9607 ns | 205.608 ns | 0.0095 | 256 B | +| HeadersReaderNew | 15 | 1 | 205.827 ns | 1.6307 ns | 2.1769 ns | 206.584 ns | 0.0095 | 256 B | +| HeadersReaderNew | 15 | 3 | 613.793 ns | 4.6416 ns | 6.3535 ns | 613.704 ns | 0.0305 | 768 B | +| HeadersReaderNew | 5 | 3 | 621.074 ns | 5.4308 ns | 7.4337 ns | 620.212 ns | 0.0305 | 768 B | +| HeadersReaderNew | 5 | 10 | 972.646 ns | 8.2481 ns | 11.2900 ns | 971.652 ns | 0.0515 | 1336 B | +| HeadersReaderNew | 15 | 5 | 1,086.556 ns | 9.6457 ns | 13.2032 ns | 1,085.646 ns | 0.0496 | 1280 B | +| HeadersReaderNew | 5 | 5 | 1,088.187 ns | 14.6687 ns | 19.5823 ns | 1,086.777 ns | 0.0496 | 1280 B | +| HeadersReaderNew | 15 | 10 | 2,367.874 ns | 16.3909 ns | 22.4361 ns | 2,359.508 ns | 0.0992 | 2560 B | + */ From fac54a135fc574d33dfa9a5a435ed89d5d75e83d Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Fri, 29 Dec 2023 14:07:16 +0100 Subject: [PATCH 06/12] fix warning --- .../Logging/Internal/HttpHeadersReader.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs index 0d432ba054b..19b33f92c14 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs @@ -17,7 +17,9 @@ internal sealed class HttpHeadersReader : IHttpHeadersReader { private readonly FrozenDictionary _requestHeadersToLog; private readonly FrozenDictionary _responseHeadersToLog; +#if NET6_0_OR_GREATER private readonly int _headersCountThreshold; +#endif private readonly IHttpHeadersRedactor _redactor; public HttpHeadersReader(IOptionsMonitor optionsMonitor, IHttpHeadersRedactor redactor, [ServiceKey] string? serviceKey = null) @@ -29,7 +31,9 @@ public HttpHeadersReader(IOptionsMonitor optionsMonitor, IHttpHe _requestHeadersToLog = options.RequestHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); _responseHeadersToLog = options.ResponseHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); +#if NET6_0_OR_GREATER _headersCountThreshold = _requestHeadersToLog.Count; +#endif } public void ReadRequestHeaders(HttpRequestMessage request, List>? destination) From 2f090c203cdd19913d3821eff4c5bb476597f4f8 Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Fri, 29 Dec 2023 15:41:17 +0100 Subject: [PATCH 07/12] update code & benchmark results --- .../Benchmarks/HeadersReaderBenchmark.cs | 68 +++++++------------ .../Logging/Internal/HttpHeadersReader.cs | 6 +- 2 files changed, 29 insertions(+), 45 deletions(-) diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs index ccd0166487d..34f45ca9674 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs @@ -77,47 +77,31 @@ public void HeadersReaderNew() Runtime=.NET 8.0 Server=True Toolchain=InProcessEmitToolchain IterationCount=15 LaunchCount=2 WarmupCount=10 -| Method | HeadersCount | HeadersToLogCount | Mean | Error | StdDev | Median | Gen0 | Gen1 | Allocated | -|----------------- |------------- |------------------ |------------:|----------:|-----------:|------------:|-------:|-------:|----------:| -| HeadersReaderOld | 0 | 1 | 18.90 ns | 0.071 ns | 0.100 ns | 18.89 ns | - | - | - | -| HeadersReaderOld | 0 | 3 | 41.60 ns | 0.464 ns | 0.665 ns | 41.33 ns | - | - | - | -| HeadersReaderOld | 0 | 5 | 64.16 ns | 0.655 ns | 0.919 ns | 63.49 ns | - | - | - | -| HeadersReaderOld | 0 | 10 | 120.26 ns | 0.276 ns | 0.377 ns | 120.25 ns | - | - | - | -| HeadersReaderOld | 5 | 1 | 402.43 ns | 6.976 ns | 9.549 ns | 402.17 ns | 0.0114 | 0.0057 | 296 B | -| HeadersReaderOld | 5 | 3 | 1,341.08 ns | 71.636 ns | 100.423 ns | 1,337.73 ns | 0.0343 | 0.0172 | 888 B | -| HeadersReaderOld | 5 | 5 | 2,189.89 ns | 74.132 ns | 106.319 ns | 2,190.03 ns | 0.0572 | 0.0267 | 1480 B | -| HeadersReaderOld | 5 | 10 | 2,349.91 ns | 23.901 ns | 32.716 ns | 2,351.23 ns | 0.0572 | 0.0267 | 1480 B | -| HeadersReaderOld | 15 | 1 | 408.09 ns | 5.997 ns | 8.209 ns | 407.62 ns | 0.0114 | 0.0057 | 296 B | -| HeadersReaderOld | 15 | 3 | 1,224.46 ns | 19.016 ns | 25.385 ns | 1,214.62 ns | 0.0343 | 0.0172 | 888 B | -| HeadersReaderOld | 15 | 5 | 2,081.42 ns | 47.198 ns | 66.165 ns | 2,065.93 ns | 0.0572 | 0.0267 | 1480 B | -| HeadersReaderOld | 15 | 10 | 4,385.35 ns | 77.832 ns | 109.109 ns | 4,362.06 ns | 0.1144 | 0.0534 | 2960 B | - -| HeadersReaderNew | 0 | 1 | 10.40 ns | 0.013 ns | 0.018 ns | 10.40 ns | - | - | - | -| HeadersReaderNew | 0 | 5 | 10.44 ns | 0.029 ns | 0.041 ns | 10.43 ns | - | - | - | -| HeadersReaderNew | 0 | 3 | 10.69 ns | 0.216 ns | 0.295 ns | 10.69 ns | - | - | - | -| HeadersReaderNew | 0 | 10 | 10.71 ns | 0.072 ns | 0.103 ns | 10.74 ns | - | - | - | -| HeadersReaderNew | 5 | 1 | 749.80 ns | 17.496 ns | 23.949 ns | 743.08 ns | 0.0191 | 0.0048 | 480 B | -| HeadersReaderNew | 5 | 3 | 1,460.32 ns | 18.964 ns | 25.316 ns | 1,456.27 ns | 0.0401 | 0.0134 | 1008 B | -| HeadersReaderNew | 5 | 5 | 2,149.69 ns | 28.055 ns | 38.403 ns | 2,147.51 ns | 0.0610 | 0.0305 | 1536 B | -| HeadersReaderNew | 5 | 10 | 2,087.11 ns | 22.876 ns | 31.313 ns | 2,086.37 ns | 0.0610 | 0.0305 | 1536 B | -| HeadersReaderNew | 15 | 1 | 1,404.72 ns | 7.300 ns | 9.745 ns | 1,403.54 ns | 0.0305 | 0.0076 | 800 B | -| HeadersReaderNew | 15 | 3 | 2,199.79 ns | 28.790 ns | 39.407 ns | 2,191.87 ns | 0.0496 | 0.0153 | 1328 B | -| HeadersReaderNew | 15 | 5 | 2,967.58 ns | 24.105 ns | 32.995 ns | 2,962.05 ns | 0.0725 | 0.0229 | 1856 B | -| HeadersReaderNew | 15 | 10 | 4,418.11 ns | 35.622 ns | 47.555 ns | 4,408.71 ns | 0.1221 | 0.0610 | 3176 B | - -| Method | HeadersCount | HeadersToLogCount | Mean | Error | StdDev | Median | Gen0 | Allocated | -|----------------- |------------- |------------------ |-------------:|-----------:|-----------:|-------------:|-------:|----------:| -| HeadersReaderNew | 0 | 3 | 4.451 ns | 0.0070 ns | 0.0098 ns | 4.449 ns | - | - | -| HeadersReaderNew | 0 | 1 | 4.523 ns | 0.0114 ns | 0.0167 ns | 4.526 ns | - | - | -| HeadersReaderNew | 0 | 10 | 4.597 ns | 0.0719 ns | 0.1007 ns | 4.659 ns | - | - | -| HeadersReaderNew | 0 | 5 | 4.942 ns | 0.3811 ns | 0.5466 ns | 4.464 ns | - | - | -| HeadersReaderNew | 5 | 1 | 205.183 ns | 1.4687 ns | 1.9607 ns | 205.608 ns | 0.0095 | 256 B | -| HeadersReaderNew | 15 | 1 | 205.827 ns | 1.6307 ns | 2.1769 ns | 206.584 ns | 0.0095 | 256 B | -| HeadersReaderNew | 15 | 3 | 613.793 ns | 4.6416 ns | 6.3535 ns | 613.704 ns | 0.0305 | 768 B | -| HeadersReaderNew | 5 | 3 | 621.074 ns | 5.4308 ns | 7.4337 ns | 620.212 ns | 0.0305 | 768 B | -| HeadersReaderNew | 5 | 10 | 972.646 ns | 8.2481 ns | 11.2900 ns | 971.652 ns | 0.0515 | 1336 B | -| HeadersReaderNew | 15 | 5 | 1,086.556 ns | 9.6457 ns | 13.2032 ns | 1,085.646 ns | 0.0496 | 1280 B | -| HeadersReaderNew | 5 | 5 | 1,088.187 ns | 14.6687 ns | 19.5823 ns | 1,086.777 ns | 0.0496 | 1280 B | -| HeadersReaderNew | 15 | 10 | 2,367.874 ns | 16.3909 ns | 22.4361 ns | 2,359.508 ns | 0.0992 | 2560 B | +| Method | HeadersCount | HeadersToLogCount | Mean | Error | StdDev | Gen0 | Allocated | +|----------------- |------------- |------------------ |-------------:|-----------:|-----------:|-------:|----------:| +| HeadersReaderNew | 0 | 1 | 4.372 ns | 0.0120 ns | 0.0175 ns | - | - | +| HeadersReaderNew | 0 | 5 | 4.459 ns | 0.0639 ns | 0.0916 ns | - | - | +| HeadersReaderNew | 0 | 3 | 4.648 ns | 0.2073 ns | 0.2972 ns | - | - | +| HeadersReaderNew | 0 | 10 | 4.995 ns | 0.4255 ns | 0.6369 ns | - | - | +| HeadersReaderOld | 0 | 1 | 18.607 ns | 0.0492 ns | 0.0689 ns | - | - | +| HeadersReaderOld | 0 | 3 | 41.269 ns | 0.0759 ns | 0.1088 ns | - | - | +| HeadersReaderOld | 0 | 5 | 63.525 ns | 0.1879 ns | 0.2572 ns | - | - | +| HeadersReaderOld | 0 | 10 | 119.163 ns | 0.6748 ns | 0.9678 ns | - | - | +| HeadersReaderOld | 15 | 1 | 204.099 ns | 1.3834 ns | 1.8936 ns | 0.0100 | 256 B | +| HeadersReaderOld | 5 | 1 | 205.044 ns | 1.7938 ns | 2.4553 ns | 0.0095 | 256 B | +| HeadersReaderNew | 15 | 1 | 216.454 ns | 1.1861 ns | 1.6236 ns | 0.0100 | 256 B | +| HeadersReaderNew | 5 | 1 | 217.499 ns | 1.5149 ns | 2.0736 ns | 0.0100 | 256 B | +| HeadersReaderOld | 5 | 3 | 630.982 ns | 4.4363 ns | 5.9224 ns | 0.0305 | 768 B | +| HeadersReaderOld | 15 | 3 | 641.780 ns | 4.4820 ns | 6.1351 ns | 0.0305 | 768 B | +| HeadersReaderNew | 15 | 3 | 669.542 ns | 3.6006 ns | 4.9285 ns | 0.0305 | 768 B | +| HeadersReaderNew | 5 | 3 | 677.061 ns | 3.8858 ns | 5.3190 ns | 0.0305 | 768 B | +| HeadersReaderNew | 5 | 10 | 981.974 ns | 9.8502 ns | 13.4831 ns | 0.0515 | 1336 B | +| HeadersReaderOld | 5 | 5 | 1,126.471 ns | 7.1361 ns | 9.7680 ns | 0.0496 | 1280 B | +| HeadersReaderOld | 15 | 5 | 1,134.430 ns | 6.2873 ns | 8.3934 ns | 0.0496 | 1280 B | +| HeadersReaderNew | 15 | 5 | 1,153.805 ns | 3.0554 ns | 4.1823 ns | 0.0496 | 1280 B | +| HeadersReaderNew | 5 | 5 | 1,164.717 ns | 9.8121 ns | 13.4310 ns | 0.0496 | 1280 B | +| HeadersReaderOld | 5 | 10 | 1,413.431 ns | 7.9666 ns | 10.9048 ns | 0.0496 | 1280 B | +| HeadersReaderOld | 15 | 10 | 2,489.613 ns | 19.0100 ns | 26.0211 ns | 0.0992 | 2560 B | +| HeadersReaderNew | 15 | 10 | 2,507.523 ns | 10.2106 ns | 13.9763 ns | 0.0992 | 2560 B | */ diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs index 19b33f92c14..1b5a7560a88 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs @@ -81,13 +81,13 @@ public void ReadResponseHeaders(HttpResponseMessage response, List headersToLog, List> destination) { #if NET6_0_OR_GREATER - var nonValidated = headers.NonValidated; - if (nonValidated.Count == 0) + var headersCount = headers.NonValidated.Count; + if (headersCount == 0) { return; } - if (nonValidated.Count < _headersCountThreshold) + if (headersCount < _headersCountThreshold) { // We have less headers than registered for logging, iterating over the smaller collection foreach (var header in headers) From 7cc6028e6117124367cc84de7097ab80891fae8a Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Tue, 2 Jan 2024 15:59:00 +0100 Subject: [PATCH 08/12] CR --- .../Logging/Internal/HttpHeadersReader.cs | 6 ++++-- .../Logging/LoggingOptions.cs | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs index fb5239c8047..2cfdcf5a2d5 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs @@ -17,6 +17,7 @@ internal sealed class HttpHeadersReader : IHttpHeadersReader { private readonly FrozenDictionary _requestHeadersToLog; private readonly FrozenDictionary _responseHeadersToLog; + private readonly bool _logContentHeaders; private readonly IHttpHeadersRedactor _redactor; public HttpHeadersReader(IOptionsMonitor optionsMonitor, IHttpHeadersRedactor redactor, [ServiceKey] string? serviceKey = null) @@ -27,6 +28,7 @@ public HttpHeadersReader(IOptionsMonitor optionsMonitor, IHttpHe _requestHeadersToLog = options.RequestHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); _responseHeadersToLog = options.ResponseHeadersDataClasses.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); + _logContentHeaders = options.LogContentHeaders; } public void ReadRequestHeaders(HttpRequestMessage request, List>? destination) @@ -37,7 +39,7 @@ public void ReadRequestHeaders(HttpRequestMessage request, List RouteParameterDataClasses { get; set; } = new Dictionary(); + + /// + /// Gets or sets a value indicating whether the HTTP request and response content headers are logged. + /// + /// + /// The default value is . + /// + /// + /// This property controls whether the logging of HTTP request and response representation headers (e.g. Content-Type) is enabled. + /// It doesn't affect the logging if or don't contain any representation headers. + /// + public bool LogContentHeaders { get; set; } } From a11a8c54c1070974dcf3bceddaf067a4f2e25232 Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Tue, 2 Jan 2024 16:16:37 +0100 Subject: [PATCH 09/12] fix after merge + CR --- .../Logging/Internal/HttpHeadersReader.cs | 2 +- .../Logging/LoggingOptions.cs | 2 ++ ...crosoft.Extensions.Http.Diagnostics.csproj | 5 ++-- .../Logging/HttpHeadersReaderTest.cs | 27 +++++++++---------- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs index 30fe7f8713d..8b70c11e734 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs @@ -60,7 +60,7 @@ public void ReadRequestHeadersNew(HttpRequestMessage request, ListContent-Type) is enabled. /// It doesn't affect the logging if or don't contain any representation headers. /// + [Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)] public bool LogContentHeaders { get; set; } } diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Microsoft.Extensions.Http.Diagnostics.csproj b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Microsoft.Extensions.Http.Diagnostics.csproj index b85b90b4c6f..062f2e87562 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Microsoft.Extensions.Http.Diagnostics.csproj +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Microsoft.Extensions.Http.Diagnostics.csproj @@ -1,4 +1,4 @@ - + Microsoft.Extensions.Http.Diagnostics Telemetry support for HTTP Client. @@ -13,7 +13,8 @@ true false true - false + true + true false true false diff --git a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs index 28278292d12..85c3ba8e640 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs @@ -102,8 +102,9 @@ public void HttpHeadersReader_WhenHeadersProvided_ReadsThem() responseBuffer.Should().BeEquivalentTo(expectedResponse); } - [Fact] - public void HttpHeadersReader_WhenProvided_ReadsContentHeaders() + [Theory] + [CombinatorialData] + public void HttpHeadersReader_WhenProvided_ReadsContentHeaders(bool logContentHeaders) { var mockHeadersRedactor = new Mock(); mockHeadersRedactor.Setup(r => r.Redact(It.IsAny>(), FakeTaxonomy.PublicData)) @@ -122,7 +123,8 @@ public void HttpHeadersReader_WhenProvided_ReadsContentHeaders() { "Header3", FakeTaxonomy.PublicData }, { "Content-Header2", FakeTaxonomy.PublicData }, { "Content-Length", FakeTaxonomy.PublicData } - } + }, + LogContentHeaders = logContentHeaders }; var headersReader = new HttpHeadersReader(options.ToOptionsMonitor(), mockHeadersRedactor.Object); @@ -145,19 +147,16 @@ public void HttpHeadersReader_WhenProvided_ReadsContentHeaders() httpResponse.Headers.Add("Header3", "Value.3"); httpResponse.Headers.Add("Header4", "Value.4"); - var expectedRequest = new[] + List> expectedRequest = [new KeyValuePair("Header1", "Value.1")]; + List> expectedResponse = [new KeyValuePair("Header3", "Value.3")]; + if (logContentHeaders) { - new KeyValuePair("Header1", "Value.1"), - new KeyValuePair("Content-Header1", "Content.1"), - new KeyValuePair("Content-Type", MediaTypeNames.Application.Soap), - }; + expectedRequest.Add(new KeyValuePair("Content-Header1", "Content.1")); + expectedRequest.Add(new KeyValuePair("Content-Type", MediaTypeNames.Application.Soap)); - var expectedResponse = new[] - { - new KeyValuePair("Header3", "Value.3"), - new KeyValuePair("Content-Header2", "Content.2"), - new KeyValuePair("Content-Length", "24"), - }; + expectedResponse.Add(new KeyValuePair("Content-Header2", "Content.2")); + expectedResponse.Add(new KeyValuePair("Content-Length", "24")); + } List> requestBuffer = []; headersReader.ReadRequestHeaders(httpRequest, requestBuffer); From e043d05d6ea2ede45935ab8e19ddac925ae490bc Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Wed, 3 Jan 2024 18:07:38 +0100 Subject: [PATCH 10/12] CR + fixes --- .../Benchmarks/HeadersReaderBenchmark.cs | 16 +- .../Logging/Internal/HttpHeadersReader.cs | 38 +---- .../Logging/Internal/Log.cs | 147 +++++------------- .../Logging/LoggingOptions.cs | 3 +- .../Logging/HttpHeadersReaderTest.cs | 3 +- 5 files changed, 53 insertions(+), 154 deletions(-) diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs index 34f45ca9674..f13ec8500ce 100644 --- a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs +++ b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/Benchmarks/HeadersReaderBenchmark.cs @@ -23,6 +23,9 @@ public class HeadersReaderBenchmark [Params(1, 3, 5, 10)] public int HeadersToLogCount { get; set; } + [Params(false, true)] + public bool LogContentHeaders { get; set; } + public HttpRequestMessage? Request { get; set; } [GlobalSetup] @@ -35,7 +38,7 @@ public void Setup() Request.Headers.Add($"Header{i}", $"Value{i}"); } - var options = new LoggingOptions(); + var options = new LoggingOptions { LogContentHeaders = LogContentHeaders }; for (var i = 0; i < HeadersToLogCount; i++) { options.RequestHeadersDataClasses.Add($"Header{i}", FakeTaxonomy.PublicData); @@ -54,19 +57,16 @@ public void Cleanup() } [Benchmark] - public void HeadersReaderOld() + public void HeadersReader() { _headersReader.ReadRequestHeaders(Request!, _outputBuffer); } - - [Benchmark] - public void HeadersReaderNew() - { - _headersReader.ReadRequestHeadersNew(Request!, _outputBuffer); - } } /* + + These results show comparison between a plain logic (when we enumerate over "headersToLog" dictionary), + and an updated logic when we choose the strategy based on the number of headers to read and the number of headers to log. BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22631.2861), VM=Hyper-V Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 16 logical and 8 physical cores diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs index 8b70c11e734..8e6bcedc4e7 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpHeadersReader.cs @@ -17,11 +17,11 @@ internal sealed class HttpHeadersReader : IHttpHeadersReader { private readonly FrozenDictionary _requestHeadersToLog; private readonly FrozenDictionary _responseHeadersToLog; + private readonly IHttpHeadersRedactor _redactor; private readonly bool _logContentHeaders; #if NET6_0_OR_GREATER private readonly int _headersCountThreshold; #endif - private readonly IHttpHeadersRedactor _redactor; public HttpHeadersReader(IOptionsMonitor optionsMonitor, IHttpHeadersRedactor redactor, [ServiceKey] string? serviceKey = null) { @@ -52,20 +52,6 @@ public void ReadRequestHeaders(HttpRequestMessage request, List>? destination) - { - if (destination is null) - { - return; - } - - ReadHeadersNew(request.Headers, _requestHeadersToLog, destination); - if (_logContentHeaders && request.Content is not null) - { - ReadHeadersNew(request.Content.Headers, _requestHeadersToLog, destination); - } - } - public void ReadResponseHeaders(HttpResponseMessage response, List>? destination) { if (destination is null) @@ -74,13 +60,17 @@ public void ReadResponseHeaders(HttpResponseMessage response, List headersToLog, List> destination) + private void ReadHeaders(HttpHeaders headers, FrozenDictionary headersToLog, List> destination) { #if NET6_0_OR_GREATER var headersCount = headers.NonValidated.Count; @@ -115,18 +105,4 @@ private void ReadHeadersNew(HttpHeaders headers, FrozenDictionary headersToLog, List> destination) - { - foreach (var kvp in headersToLog) - { - var classification = kvp.Value; - var header = kvp.Key; - - if (headers.TryGetValues(header, out var values)) - { - destination.Add(new(header, _redactor.Redact(values, classification))); - } - } - } } diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/Log.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/Log.cs index 2b8a627b687..ea60895faf9 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/Log.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/Log.cs @@ -12,10 +12,9 @@ namespace Microsoft.Extensions.Http.Logging.Internal; /// Logs , and the exceptions due to errors of request/response. /// [SuppressMessage("Major Code Smell", "S109:Magic numbers should not be used", Justification = "Event ID's.")] -internal static class Log +internal static partial class Log { internal const string OriginalFormat = "{OriginalFormat}"; - private const string NullString = "(null)"; private const int MinimalPropertyCount = 4; @@ -34,7 +33,7 @@ internal static class Log "An error occurred in enricher '{Enricher}' while enriching the logger context for request: " + $"{{{HttpClientLoggingTagNames.Method}}} {{{HttpClientLoggingTagNames.Host}}}/{{{HttpClientLoggingTagNames.Path}}}"; - private static readonly Func _originalFormatValueFMTFunc = OriginalFormatValueFmt; + private static readonly Func _originalFormatValueFmtFunc = OriginalFormatValueFmt; public static void OutgoingRequest(ILogger logger, LogLevel level, LogRecord record) { @@ -46,114 +45,38 @@ public static void OutgoingRequestError(ILogger logger, LogRecord record, Except OutgoingRequest(logger, LogLevel.Error, 2, nameof(OutgoingRequestError), record, exception); } - public static void RequestReadError(ILogger logger, Exception exception, HttpMethod method, string? host, string? path) - { - var state = LoggerMessageHelper.ThreadLocalState; - - _ = state.ReserveTagSpace(4); - state.TagArray[3] = new(HttpClientLoggingTagNames.Method, method); - state.TagArray[2] = new(HttpClientLoggingTagNames.Host, host); - state.TagArray[1] = new(HttpClientLoggingTagNames.Path, path); - state.TagArray[0] = new(OriginalFormat, RequestReadErrorMessage); - - logger.Log( - LogLevel.Error, - new(0, nameof(RequestReadError)), - state, - exception, - static (s, _) => - { - var method = s.TagArray[3].Value ?? NullString; - var host = s.TagArray[2].Value ?? NullString; - var path = s.TagArray[1].Value ?? NullString; - return FormattableString.Invariant( - $"An error occurred while reading the request data to fill the logger context for request: {method} {host}/{path}"); - }); - - state.Clear(); - } - - public static void ResponseReadError(ILogger logger, Exception exception, HttpMethod method, string host, string path) - { - var state = LoggerMessageHelper.ThreadLocalState; - - _ = state.ReserveTagSpace(4); - state.TagArray[3] = new(HttpClientLoggingTagNames.Method, method); - state.TagArray[2] = new(HttpClientLoggingTagNames.Host, host); - state.TagArray[1] = new(HttpClientLoggingTagNames.Path, path); - state.TagArray[0] = new(OriginalFormat, ResponseReadErrorMessage); - - logger.Log( - LogLevel.Error, - new(0, nameof(ResponseReadError)), - state, - exception, - static (s, _) => - { - var method = s.TagArray[3].Value ?? NullString; - var host = s.TagArray[2].Value ?? NullString; - var path = s.TagArray[1].Value ?? NullString; - return FormattableString.Invariant( - $"An error occurred while reading the response data to fill the logger context for request: {method} {host}/{path}"); - }); - - state.Clear(); - } - - public static void LoggerContextMissing(ILogger logger, Exception? exception, string requestState, HttpMethod method, string? host) - { - var state = LoggerMessageHelper.ThreadLocalState; - - _ = state.ReserveTagSpace(4); - state.TagArray[3] = new("RequestState", requestState); - state.TagArray[2] = new(HttpClientLoggingTagNames.Method, method); - state.TagArray[1] = new(HttpClientLoggingTagNames.Host, host); - state.TagArray[0] = new(OriginalFormat, LoggerContextMissingMessage); - - logger.Log( - LogLevel.Error, - new(0, nameof(LoggerContextMissing)), - state, - exception, - static (s, _) => - { - var requestState = s.TagArray[3].Value ?? NullString; - var method = s.TagArray[2].Value ?? NullString; - var host = s.TagArray[1].Value ?? NullString; - return FormattableString.Invariant($"The logger couldn't read its context for {requestState} request: {method} {host}"); - }); - - state.Clear(); - } - - public static void EnrichmentError(ILogger logger, Exception exception, string? enricher, HttpMethod method, string host, string path) - { - var state = LoggerMessageHelper.ThreadLocalState; - - _ = state.ReserveTagSpace(5); - state.TagArray[4] = new("Enricher", enricher); - state.TagArray[3] = new(HttpClientLoggingTagNames.Method, method); - state.TagArray[2] = new(HttpClientLoggingTagNames.Host, host); - state.TagArray[1] = new(HttpClientLoggingTagNames.Path, path); - state.TagArray[0] = new(OriginalFormat, EnrichmentErrorMessage); - - logger.Log( - LogLevel.Error, - new(0, nameof(EnrichmentError)), - state, - exception, - static (s, _) => - { - var enricher = s.TagArray[4].Value ?? NullString; - var method = s.TagArray[3].Value ?? NullString; - var host = s.TagArray[2].Value ?? NullString; - var path = s.TagArray[1].Value ?? NullString; - return FormattableString.Invariant( - $"An error occurred in enricher '{enricher}' while enriching the logger context for request: {method} {host}/{path}"); - }); - - state.Clear(); - } + [LoggerMessage(LogLevel.Error, RequestReadErrorMessage)] + public static partial void RequestReadError( + ILogger logger, + Exception exception, + [TagName(HttpClientLoggingTagNames.Method)] HttpMethod method, + [TagName(HttpClientLoggingTagNames.Host)] string? host, + [TagName(HttpClientLoggingTagNames.Path)] string? path); + + [LoggerMessage(LogLevel.Error, ResponseReadErrorMessage)] + public static partial void ResponseReadError( + ILogger logger, + Exception exception, + [TagName(HttpClientLoggingTagNames.Method)] HttpMethod method, + [TagName(HttpClientLoggingTagNames.Host)] string host, + [TagName(HttpClientLoggingTagNames.Path)] string path); + + [LoggerMessage(LogLevel.Error, LoggerContextMissingMessage)] + public static partial void LoggerContextMissing( + ILogger logger, + Exception? exception, + string requestState, + [TagName(HttpClientLoggingTagNames.Method)] HttpMethod method, + [TagName(HttpClientLoggingTagNames.Host)] string? host); + + [LoggerMessage(LogLevel.Error, EnrichmentErrorMessage)] + public static partial void EnrichmentError( + ILogger logger, + Exception exception, + string? enricher, + [TagName(HttpClientLoggingTagNames.Method)] HttpMethod method, + [TagName(HttpClientLoggingTagNames.Host)] string host, + [TagName(HttpClientLoggingTagNames.Path)] string path); // Using the code below instead of generated logging method because we have a custom formatter and custom tag keys for headers. private static void OutgoingRequest( @@ -215,7 +138,7 @@ private static void OutgoingRequest( new(eventId, eventName), loggerMessageState, exception, - _originalFormatValueFMTFunc); + _originalFormatValueFmtFunc); if (record.EnrichmentTags is null) { diff --git a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/LoggingOptions.cs b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/LoggingOptions.cs index 8a9404f85bf..fe2d096f7bc 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/LoggingOptions.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/LoggingOptions.cs @@ -154,7 +154,8 @@ public class LoggingOptions /// /// /// This property controls whether the logging of HTTP request and response representation headers (e.g. Content-Type) is enabled. - /// It doesn't affect the logging if or don't contain any representation headers. + /// Keep this option disabled if or + /// don't contain any representation headers, otherwise it will create unnecessary minor performance impact on the headers logging. /// [Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)] public bool LogContentHeaders { get; set; } diff --git a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs index 85c3ba8e640..625342d2468 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpHeadersReaderTest.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Collections.Generic; using System.Net.Http; using System.Net.Mime; @@ -92,7 +91,7 @@ public void HttpHeadersReader_WhenHeadersProvided_ReadsThem() { new KeyValuePair("Header3", "Value.3"), new KeyValuePair("Header4", "Value.4"), - new KeyValuePair("Header7", Redacted), + new KeyValuePair("hEaDeR7", Redacted) }; headersReader.ReadRequestHeaders(httpRequest, requestBuffer); From cda2aaed7a31df9763a477402bc075020d56c59e Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Thu, 4 Jan 2024 11:43:09 +0100 Subject: [PATCH 11/12] fix xUnit1030 --- .../Logging/AcceptanceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/AcceptanceTests.cs b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/AcceptanceTests.cs index 8e2df28853b..36ad783c73b 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/AcceptanceTests.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/AcceptanceTests.cs @@ -360,7 +360,7 @@ public async Task AddHttpClientLogging_StructuredPathLogging_RedactsSensitivePar httpRequestMessage.SetRequestMetadata(new RequestMetadata(httpRequestMessage.Method.ToString(), RequestRoute)); - using var _ = await httpClient.SendAsync(httpRequestMessage).ConfigureAwait(false); + using var _ = await httpClient.SendAsync(httpRequestMessage); var collector = sp.GetFakeLogCollector(); var logRecord = collector.GetSnapshot().Single(logRecord => logRecord.Category == LoggingCategory); From 75dc66cd79a6eb6a7b8af8e9b0bbed5da4e83fac Mon Sep 17 00:00:00 2001 From: Nikita Balabaev Date: Thu, 4 Jan 2024 13:27:08 +0100 Subject: [PATCH 12/12] unblock the build --- eng/pipelines/templates/VerifyCoverageReport.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/pipelines/templates/VerifyCoverageReport.yml b/eng/pipelines/templates/VerifyCoverageReport.yml index 5c55d4d42de..d956eac76d1 100644 --- a/eng/pipelines/templates/VerifyCoverageReport.yml +++ b/eng/pipelines/templates/VerifyCoverageReport.yml @@ -39,3 +39,4 @@ steps: repositoryName: '$(Build.Repository.Name)' id: $(System.PullRequest.PullRequestNumber) displayName: Report coverage to GitHub + continueOnError: 'true' # Temporary setting to avoid failing the build if the GitHub comment fails