Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Missing overrides in LoggingHttpMessageHandler and LoggingScopeHttpMessageHandler #85143

Merged
merged 26 commits into from
May 4, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7532cc5
add missing overrides in LoggingHttpMessageHandler and LoggingScopeHt…
mphelt Apr 21, 2023
04c3ae6
Update LoggingUriOutputTests.cs
mphelt Apr 21, 2023
0b6199c
Update LoggingHttpMessageHandler.cs
mphelt Apr 25, 2023
022e908
Update LoggingScopeHttpMessageHandler.cs
mphelt Apr 25, 2023
a69446a
Update LoggingScopeHttpMessageHandler.cs
mphelt Apr 25, 2023
44260f5
Update LoggingUriOutputTests.cs
mphelt Apr 25, 2023
854dd1e
Add 'if NET5_0_OR_GREATER', fix typo in 'Loggs', add 'ConditionalFact'
mphelt Apr 25, 2023
ccb3a05
Update LoggingScopeHttpMessageHandler.cs
mphelt Apr 25, 2023
6c14e66
Update LoggingUriOutputTests.cs
mphelt Apr 25, 2023
8813153
Update LoggingUriOutputTests.cs
mphelt Apr 25, 2023
86037ff
Update LoggingUriOutputTests.cs
mphelt Apr 26, 2023
8ca29b4
Update LoggingUriOutputTests.cs
mphelt Apr 26, 2023
838b640
Update LoggingHttpMessageHandler.cs
mphelt Apr 27, 2023
bb7daea
Update LoggingScopeHttpMessageHandler.cs
mphelt Apr 27, 2023
14aa765
Extract helper methods
mphelt Apr 27, 2023
16f55c9
Update LoggingHttpMessageHandler.cs
mphelt Apr 27, 2023
ebfe45b
Update LoggingScopeHttpMessageHandler.cs
mphelt Apr 27, 2023
06de1f5
fix helper methods
mphelt Apr 27, 2023
ba43a5d
Update LoggingHttpMessageHandler.cs
mphelt Apr 27, 2023
1d654a9
Update LoggingHttpMessageHandler.cs
mphelt Apr 27, 2023
497d6c2
Update LoggingScopeHttpMessageHandler.cs
mphelt Apr 27, 2023
080fbff
Merge pull request #4 from mphelt/mphelt-patch-1
mphelt Apr 27, 2023
91d421b
Code style update
mphelt Apr 28, 2023
67d2534
back to private methods
mphelt Apr 28, 2023
08ecd2c
merge with dotnet/runtime (#7)
mphelt May 4, 2023
caefe8b
Merge branch 'dotnet:main' into main
mphelt May 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,28 +47,44 @@ public LoggingHttpMessageHandler(ILogger logger, HttpClientFactoryOptions option
_options = options;
}

/// <inheritdoc />
/// <remarks>Loggs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
private Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage request, bool useAsync, CancellationToken cancellationToken)
{
ThrowHelper.ThrowIfNull(request);
return Core(request, cancellationToken);

async Task<HttpResponseMessage> Core(HttpRequestMessage request, CancellationToken cancellationToken)
{
Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? _shouldNotRedactHeaderValue;
var shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? _shouldNotRedactHeaderValue;
mphelt marked this conversation as resolved.
Show resolved Hide resolved

// Not using a scope here because we always expect this to be at the end of the pipeline, thus there's
// not really anything to surround.
Log.RequestStart(_logger, request, shouldRedactHeaderValue);
var stopwatch = ValueStopwatch.StartNew();
HttpResponseMessage response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
var response = useAsync
mphelt marked this conversation as resolved.
Show resolved Hide resolved
? await base.SendAsync(request, cancellationToken).ConfigureAwait(false)
#if NET5_0_OR_GREATER
: base.Send(request, cancellationToken);
#else
: throw new NotImplementedException("Unreachable code");
#endif
Log.RequestEnd(_logger, response, stopwatch.GetElapsedTime(), shouldRedactHeaderValue);

return response;
}
}

/// <inheritdoc />
/// <remarks>Logs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
=> SendCoreAsync(request, true, cancellationToken);
mphelt marked this conversation as resolved.
Show resolved Hide resolved

#if NET5_0_OR_GREATER
/// <inheritdoc />
/// <remarks>Logs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
=> SendCoreAsync(request, false, cancellationToken).GetAwaiter().GetResult();
mphelt marked this conversation as resolved.
Show resolved Hide resolved
#endif

// Used in tests.
internal static class Log
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ public LoggingScopeHttpMessageHandler(ILogger logger, HttpClientFactoryOptions o
_options = options;
}

/// <inheritdoc />
/// <remarks>Loggs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
private Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage request, bool useAsync, CancellationToken cancellationToken)
{
ThrowHelper.ThrowIfNull(request);
return Core(request, cancellationToken);
Expand All @@ -58,19 +56,37 @@ async Task<HttpResponseMessage> Core(HttpRequestMessage request, CancellationTok
{
var stopwatch = ValueStopwatch.StartNew();

Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? _shouldNotRedactHeaderValue;
var shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? _shouldNotRedactHeaderValue;
mphelt marked this conversation as resolved.
Show resolved Hide resolved

using (Log.BeginRequestPipelineScope(_logger, request))
{
Log.RequestPipelineStart(_logger, request, shouldRedactHeaderValue);
HttpResponseMessage response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
var response = useAsync
mphelt marked this conversation as resolved.
Show resolved Hide resolved
? await base.SendAsync(request, cancellationToken).ConfigureAwait(false)
#if NET5_0_OR_GREATER
: base.Send(request, cancellationToken);
#else
: throw new NotImplementedException("Unreachable code");
#endif
Log.RequestPipelineEnd(_logger, response, stopwatch.GetElapsedTime(), shouldRedactHeaderValue);

return response;
}
}
}

/// <inheritdoc />
/// <remarks>Logs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
=> SendCoreAsync(request, true, cancellationToken);
mphelt marked this conversation as resolved.
Show resolved Hide resolved

#if NET5_0_OR_GREATER
/// <inheritdoc />
/// <remarks>Logs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
=> SendCoreAsync(request, false, cancellationToken).GetAwaiter().GetResult();
mphelt marked this conversation as resolved.
Show resolved Hide resolved
#endif

// Used in tests
internal static class Log
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// 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.Linq;
using System.Net.Http;
using System.Threading;
Expand Down Expand Up @@ -90,6 +91,83 @@ public async Task LoggingScopeHttpMessageHandler_LogsAbsoluteUri()
Assert.Equal("HTTP GET http://api.example.com/search?term=Western%20Australia", message.Scope.ToString());
}

#if NET5_0_OR_GREATER
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
public void LoggingHttpMessageHandler_LogsAbsoluteUri_Sync()
{
// Arrange
var sink = new TestSink();

var serviceCollection = new ServiceCollection();
serviceCollection.AddLogging();
serviceCollection.AddSingleton<ILoggerFactory>(new TestLoggerFactory(sink, enabled: true));

serviceCollection
.AddHttpClient("test")
.ConfigurePrimaryHttpMessageHandler(() => new TestMessageHandler());

var services = serviceCollection.BuildServiceProvider();

var client = services.GetRequiredService<IHttpClientFactory>().CreateClient("test");


// Act
var request = new HttpRequestMessage(HttpMethod.Get, "http://api.example.com/search?term=Western%20Australia");

client.Send(request);

// Assert
var messages = sink.Writes.ToArray();

var message = Assert.Single(messages.Where(m =>
{
return
m.EventId == LoggingHttpMessageHandler.Log.EventIds.RequestStart &&
m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
}));

Assert.Equal("Sending HTTP request GET http://api.example.com/search?term=Western%20Australia", message.Message);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
public void LoggingScopeHttpMessageHandler_LogsAbsoluteUri_Sync()
{
// Arrange
var sink = new TestSink();

var serviceCollection = new ServiceCollection();
serviceCollection.AddLogging();
serviceCollection.AddSingleton<ILoggerFactory>(new TestLoggerFactory(sink, enabled: true));

serviceCollection
.AddHttpClient("test")
.ConfigurePrimaryHttpMessageHandler(() => new TestMessageHandler());

var services = serviceCollection.BuildServiceProvider();

var client = services.GetRequiredService<IHttpClientFactory>().CreateClient("test");


// Act
var request = new HttpRequestMessage(HttpMethod.Get, "http://api.example.com/search?term=Western%20Australia");

client.Send(request);

// Assert
var messages = sink.Writes.ToArray();

var message = Assert.Single(messages.Where(m =>
{
return
m.EventId == LoggingScopeHttpMessageHandler.Log.EventIds.PipelineStart &&
m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
}));

Assert.Equal("Start processing HTTP request GET http://api.example.com/search?term=Western%20Australia", message.Message);
Assert.Equal("HTTP GET http://api.example.com/search?term=Western%20Australia", message.Scope.ToString());
}
#endif

private class TestMessageHandler : HttpClientHandler
{
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
Expand All @@ -98,6 +176,10 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques

return Task.FromResult(response);
}

#if NET5_0_OR_GREATER
protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) => new();
#endif
}
}
}