Skip to content

Fix HTTP client logging config binding - #7691

Merged
jeffhandley merged 4 commits into
dotnet:mainfrom
Rimobul:lsokolovsky/issue-7551
Aug 11, 2026
Merged

Fix HTTP client logging config binding #7691
jeffhandley merged 4 commits into
dotnet:mainfrom
Rimobul:lsokolovsky/issue-7551

Conversation

@Rimobul

@Rimobul Rimobul commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #7551

Use a static options configurator instead of generated configuration
binding. The generated binder treats DataClassification values as complex
objects because it does not honor their type converter.

Preserve named options and configuration reload support. Add regression
coverage for both registration paths, supported classification formats,
invalid values, and reloads.

Microsoft Reviewers: Open in CodeFlow

Fixes dotnet#7551

Use a static options configurator instead of generated configuration
binding. The generated binder treats DataClassification values as complex
objects because it does not honor their type converter.

Preserve named options and configuration reload support. Add regression
coverage for both registration paths, supported classification formats,
invalid values, and reloads.
Copilot AI lite review requested due to automatic review settings August 10, 2026 12:03
@Rimobul
Rimobul requested a review from a team as a code owner August 10, 2026 12:04
@Rimobul Rimobul changed the title Lsokolovsky/issue 7551 Fix HTTP client logging config binding Aug 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses dotnet/extensions #7551 by replacing generated configuration binding for HTTP client logging options with explicit, static binding logic so that DataClassification values bind correctly from configuration (including documented "None"/"Unknown" formats). It also preserves named-options behavior and configuration reload support, and adds regression coverage for both the IHttpClientBuilder and IServiceCollection registration paths.

Changes:

  • Replaced .Bind(section) usage with a custom BindConfigurationSection implementation that correctly parses DataClassification (scalar and object forms) and supports reload via ConfigurationChangeTokenSource.
  • Added explicit options configurators for both LoggingOptions (Http Diagnostics) and LoggingRedactionOptions (ASP.NET Core middleware) to avoid the binder source generator’s DataClassification handling.
  • Added regression tests covering valid/invalid classifications, scalar parsing failures, missing sections, and reload behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/Libraries/Microsoft.Extensions.Http.Diagnostics.Tests/Logging/HttpClientLoggingExtensionsTest.cs Adds regression tests for config-based binding (including documented JSON), invalid values, and reloads for named and default options paths.
test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/HttpLoggingServiceExtensionsTests.cs Adds tests ensuring DataClassification binds correctly for HTTP logging redaction options and that invalid values throw with useful paths.
src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/LoggingOptionsConfigureOptions.cs New explicit named-options binder for LoggingOptions, including DataClassification parsing and scalar/enum parsing with wrapped exceptions.
src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/LoggingOptionsBinder.cs New internal OptionsBuilder<LoggingOptions> extension that wires the custom configurator plus reload token source.
src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/HttpClientLoggingServiceCollectionExtensions.cs Switches config binding to the new BindConfigurationSection path.
src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/HttpClientLoggingHttpClientBuilderExtensions.cs Switches config binding to the new BindConfigurationSection path for named clients (and wrapHandlersPipeline overload).
src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/Internal/LoggingRedactionOptionsConfigureOptions.cs New explicit binder for LoggingRedactionOptions to correctly parse DataClassification values from configuration.
src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/HttpLoggingServiceCollectionExtensions.cs Replaces section.Bind(o) with the new explicit configurator registration for redaction options.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@jeffhandley jeffhandley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, @Rimobul!

@jeffhandley
jeffhandley merged commit b1b8155 into dotnet:main Aug 11, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AddExtendedHttpClientLogging(configurationSection) fails with the documented appsettings.json sample for RequestHeadersDataClasses

4 participants