Skip to content

Commit

Permalink
Update HttpClient logging APIs per Noah's comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Martin Taillefer committed Sep 26, 2023
1 parent 57c0bd6 commit 001a5d5
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 92 deletions.
12 changes: 6 additions & 6 deletions bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogRequest(string file
.AddSingleton(_ => NoRemoteCallHandler.Create(fileName))
.AddHttpClientLogEnricher<BenchEnricher>()
.AddHttpClient(nameof(fileName))
.AddHttpClientLogging(options =>
.AddExtendedHttpClientLogging(options =>
{
options.BodySizeLimit = readLimit;
options.RequestBodyContentTypes.Add(new("application/json"));
Expand All @@ -45,7 +45,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogResponse(string fil
.AddFakeRedaction()
.AddSingleton(_ => NoRemoteCallHandler.Create(fileName))
.AddHttpClient(nameof(fileName))
.AddHttpClientLogging(options =>
.AddExtendedHttpClientLogging(options =>
{
options.BodySizeLimit = readLimit;
options.ResponseBodyContentTypes.Add(new("application/json"));
Expand All @@ -68,7 +68,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogAll(string fileName
.AddFakeRedaction()
.AddSingleton(_ => NoRemoteCallHandler.Create(fileName))
.AddHttpClient(nameof(fileName))
.AddHttpClientLogging(options =>
.AddExtendedHttpClientLogging(options =>
{
options.BodySizeLimit = readLimit;

Expand All @@ -95,7 +95,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogRequest_ChunkedEnco
.AddFakeRedaction()
.AddSingleton(_ => NoRemoteCallNotSeekableHandler.Create(fileName))
.AddHttpClient(nameof(fileName))
.AddHttpClientLogging(options =>
.AddExtendedHttpClientLogging(options =>
{
options.BodySizeLimit = readLimit;
options.RequestBodyContentTypes.Add("application/json");
Expand All @@ -118,7 +118,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogResponse_ChunkedEnc
.AddFakeRedaction()
.AddSingleton(_ => NoRemoteCallNotSeekableHandler.Create(fileName))
.AddHttpClient(nameof(fileName))
.AddHttpClientLogging(options =>
.AddExtendedHttpClientLogging(options =>
{
options.BodySizeLimit = readLimit;
options.ResponseBodyContentTypes.Add("application/json");
Expand All @@ -141,7 +141,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogAll_ChunkedEncoding
.AddFakeRedaction()
.AddSingleton(_ => NoRemoteCallNotSeekableHandler.Create(fileName))
.AddHttpClient(nameof(fileName))
.AddHttpClientLogging(options =>
.AddExtendedHttpClientLogging(options =>
{
options.BodySizeLimit = readLimit;

Expand Down
10 changes: 5 additions & 5 deletions ...ies/Microsoft.Extensions.Http.Diagnostics/Latency/HttpClientLatencyTelemetryExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public static class HttpClientLatencyTelemetryExtensions
/// </remarks>
/// <param name="services">The <see cref="IServiceCollection" />.</param>
/// <returns>The value of <paramref name="services"/>.</returns>
public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServiceCollection services)
public static IServiceCollection AddHttpClientLatencyTelemetry(this IServiceCollection services)
{
_ = Throw.IfNull(services);

Expand Down Expand Up @@ -62,14 +62,14 @@ public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServ
[UnconditionalSuppressMessage("Trimming",
"IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
Justification = "Addressed with [DynamicDependency]")]
public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServiceCollection services, IConfigurationSection section)
public static IServiceCollection AddHttpClientLatencyTelemetry(this IServiceCollection services, IConfigurationSection section)
{
_ = Throw.IfNull(section);

_ = services
.Configure<HttpClientLatencyTelemetryOptions>(section);

return services.AddDefaultHttpClientLatencyTelemetry();
return services.AddHttpClientLatencyTelemetry();
}

/// <summary>
Expand All @@ -81,13 +81,13 @@ public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServ
/// <param name="services">The <see cref="IServiceCollection" />.</param>
/// <param name="configure">The delegate to configure <see cref="HttpClientLatencyTelemetryOptions"/> with.</param>
/// <returns>The value of <paramref name="services"/>.</returns>
public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServiceCollection services, Action<HttpClientLatencyTelemetryOptions> configure)
public static IServiceCollection AddHttpClientLatencyTelemetry(this IServiceCollection services, Action<HttpClientLatencyTelemetryOptions> configure)
{
_ = Throw.IfNull(configure);

_ = services
.Configure(configure);

return services.AddDefaultHttpClientLatencyTelemetry();
return services.AddHttpClientLatencyTelemetry();
}
}
28 changes: 11 additions & 17 deletions ...osoft.Extensions.Http.Diagnostics/Logging/HttpClientLoggingHttpClientBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,36 +16,32 @@
namespace Microsoft.Extensions.DependencyInjection;

/// <summary>
/// Extension methods to register HTTP client logging feature.
/// Extensions to register extended HTTP client logging features.
/// </summary>
public static class HttpClientLoggingHttpClientBuilderExtensions
{
/// <summary>
/// Adds an <see cref="IHttpClientAsyncLogger" /> to emit logs for outgoing requests for a named <see cref="HttpClient"/>.
/// </summary>
/// <param name="builder">The <see cref="IHttpClientBuilder" />.</param>
/// <returns>
/// An <see cref="IHttpClientBuilder" /> that can be used to configure the client.
/// </returns>
/// <returns>The value of <paramref name="builder"/>.</returns>
/// <remarks>
/// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>.
/// </remarks>
/// <exception cref="ArgumentNullException">Argument <paramref name="builder"/> is <see langword="null"/>.</exception>
public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder builder)
public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder)
{
_ = Throw.IfNull(builder);

return AddNamedClientLoggingInternal(builder);
return AddExtendedHttpClientLoggingInternal(builder);
}

/// <summary>
/// Adds an <see cref="IHttpClientAsyncLogger" /> to emit logs for outgoing requests for a named <see cref="HttpClient"/>.
/// </summary>
/// <param name="builder">The <see cref="IHttpClientBuilder" />.</param>
/// <param name="section">The <see cref="IConfigurationSection"/> to use for configuring <see cref="LoggingOptions"/>.</param>
/// <returns>
/// An <see cref="IHttpClientBuilder" /> that can be used to configure the client.
/// </returns>
/// <returns>The value of <paramref name="builder"/>.</returns>
/// <remarks>
/// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>.
/// </remarks>
Expand All @@ -54,22 +50,20 @@ public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder bu
[UnconditionalSuppressMessage("Trimming",
"IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
Justification = "Addressed with [DynamicDependency]")]
public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder builder, IConfigurationSection section)
public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, IConfigurationSection section)
{
_ = Throw.IfNull(builder);
_ = Throw.IfNull(section);

return AddNamedClientLoggingInternal(builder, options => options.Bind(section));
return AddExtendedHttpClientLoggingInternal(builder, options => options.Bind(section));
}

/// <summary>
/// Adds an <see cref="IHttpClientAsyncLogger" /> to emit logs for outgoing requests for a named <see cref="HttpClient"/>.
/// </summary>
/// <param name="builder">The <see cref="IHttpClientBuilder" />.</param>
/// <param name="configure">The delegate to configure <see cref="LoggingOptions"/> with.</param>
/// <returns>
/// An <see cref="IHttpClientBuilder" /> that can be used to configure the client.
/// </returns>
/// <returns>The value of <paramref name="builder"/>.</returns>
/// <remarks>
/// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>.
/// </remarks>
Expand All @@ -78,15 +72,15 @@ public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder bu
[UnconditionalSuppressMessage("Trimming",
"IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
Justification = "Addressed with [DynamicDependency]")]
public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder builder, Action<LoggingOptions> configure)
public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, Action<LoggingOptions> configure)
{
_ = Throw.IfNull(builder);
_ = Throw.IfNull(configure);

return AddNamedClientLoggingInternal(builder, options => options.Configure(configure));
return AddExtendedHttpClientLoggingInternal(builder, options => options.Configure(configure));
}

private static IHttpClientBuilder AddNamedClientLoggingInternal(IHttpClientBuilder builder, Action<OptionsBuilder<LoggingOptions>>? configureOptionsBuilder = null)
private static IHttpClientBuilder AddExtendedHttpClientLoggingInternal(IHttpClientBuilder builder, Action<OptionsBuilder<LoggingOptions>>? configureOptionsBuilder = null)
{
var optionsBuilder = builder.Services
.AddOptionsWithValidateOnStart<LoggingOptions, LoggingOptionsValidator>(builder.Name);
Expand Down
12 changes: 6 additions & 6 deletions ...osoft.Extensions.Http.Diagnostics/Logging/HttpClientLoggingServiceCollectionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
namespace Microsoft.Extensions.DependencyInjection;

/// <summary>
/// Extension methods to register HTTP client logging features.
/// Extensions to register extended HTTP client logging features.
/// </summary>
public static class HttpClientLoggingServiceCollectionExtensions
{
Expand All @@ -28,7 +28,7 @@ public static class HttpClientLoggingServiceCollectionExtensions
/// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>.
/// </remarks>
/// <exception cref="ArgumentNullException">Argument <paramref name="services"/> is <see langword="null"/>.</exception>
public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollection services)
public static IServiceCollection AddExtendedHttpClientLogging(this IServiceCollection services)
{
_ = Throw.IfNull(services);

Expand Down Expand Up @@ -62,7 +62,7 @@ public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollec
[UnconditionalSuppressMessage("Trimming",
"IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
Justification = "Addressed with [DynamicDependency]")]
public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollection services, IConfigurationSection section)
public static IServiceCollection AddExtendedHttpClientLogging(this IServiceCollection services, IConfigurationSection section)
{
_ = Throw.IfNull(services);
_ = Throw.IfNull(section);
Expand All @@ -71,7 +71,7 @@ public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollec
.AddOptionsWithValidateOnStart<LoggingOptions, LoggingOptionsValidator>()
.Bind(section);

return services.AddDefaultHttpClientLogging();
return services.AddExtendedHttpClientLogging();
}

/// <summary>
Expand All @@ -84,7 +84,7 @@ public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollec
/// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>.
/// </remarks>
/// <exception cref="ArgumentNullException">Any of the arguments is <see langword="null"/>.</exception>
public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollection services, Action<LoggingOptions> configure)
public static IServiceCollection AddExtendedHttpClientLogging(this IServiceCollection services, Action<LoggingOptions> configure)
{
_ = Throw.IfNull(services);
_ = Throw.IfNull(configure);
Expand All @@ -93,7 +93,7 @@ public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollec
.AddOptionsWithValidateOnStart<LoggingOptions, LoggingOptionsValidator>()
.Configure(configure);

return services.AddDefaultHttpClientLogging();
return services.AddExtendedHttpClientLogging();
}

/// <summary>
Expand Down
14 changes: 7 additions & 7 deletions ...oft.Extensions.Http.Diagnositcs.Tests/Latency/HttpClientLatencyTelemetryExtensionsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ public class HttpClientLatencyTelemetryExtensionsTest
[Fact]
public void HttpClientLatencyTelemtry_NullParameter_ThrowsException()
{
var act = () => ((IServiceCollection)null!).AddDefaultHttpClientLatencyTelemetry();
var act = () => ((IServiceCollection)null!).AddHttpClientLatencyTelemetry();
act.Should().Throw<ArgumentNullException>();

act = () => Mock.Of<IServiceCollection>().AddDefaultHttpClientLatencyTelemetry((Action<HttpClientLatencyTelemetryOptions>)null!);
act = () => Mock.Of<IServiceCollection>().AddHttpClientLatencyTelemetry((Action<HttpClientLatencyTelemetryOptions>)null!);
act.Should().Throw<ArgumentNullException>();

act = () => Mock.Of<IServiceCollection>().AddDefaultHttpClientLatencyTelemetry((IConfigurationSection)null!);
act = () => Mock.Of<IServiceCollection>().AddHttpClientLatencyTelemetry((IConfigurationSection)null!);
act.Should().Throw<ArgumentNullException>();
}

Expand All @@ -36,7 +36,7 @@ public void HttpClientLatencyTelemtry_AddToServiceCollection()
using var sp = new ServiceCollection()
.AddHttpClient()
.AddNullLatencyContext()
.AddDefaultHttpClientLatencyTelemetry()
.AddHttpClientLatencyTelemetry()
.BuildServiceProvider();

var listener = sp.GetRequiredService<HttpRequestLatencyListener>();
Expand All @@ -60,7 +60,7 @@ public void HttpClientLatencyTelemtry_AddToServiceCollection_CreatesClientSucces
using var sp = new ServiceCollection()
.AddNullLatencyContext()
.AddHttpClient()
.AddDefaultHttpClientLatencyTelemetry()
.AddHttpClientLatencyTelemetry()
.BuildServiceProvider();

using var httpClient = sp.GetRequiredService<IHttpClientFactory>().CreateClient();
Expand All @@ -73,7 +73,7 @@ public void HttpClientLatencyTelemtryExtensions_Add_InvokesConfig()
bool invoked = false;
using var sp = new ServiceCollection()
.AddNullLatencyContext()
.AddDefaultHttpClientLatencyTelemetry(a =>
.AddHttpClientLatencyTelemetry(a =>
{
invoked = true;
a.EnableDetailedLatencyBreakdown = false;
Expand All @@ -97,7 +97,7 @@ public void RequestLatencyExtensions_Add_BindsToConfigSection()
var config = GetConfigSection(expectedOptions);
using var sp = new ServiceCollection()
.AddNullLatencyContext()
.AddDefaultHttpClientLatencyTelemetry(config)
.AddHttpClientLatencyTelemetry(config)
.BuildServiceProvider();

var options = sp.GetRequiredService<IOptions<HttpClientLatencyTelemetryOptions>>().Value;
Expand Down
Loading

0 comments on commit 001a5d5

Please sign in to comment.