Integrate HttpClientFactory and metrics #86888
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsPrototype for integration between HttpClientFactory and metrics
|
...xtensions.Http/tests/Microsoft.Extensions.Http.Tests/DefaultHttpMessageHandlerBuilderTest.cs
Outdated
Show resolved
Hide resolved
|
Change LGTM, I added a small question to the review. |
ghost
closed this
|
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
|
@karelz @CarnaViire @antonfirsov @noahfalk This is still to do. It needs to happen for .NET 8. I don't think this had its own issue. It might have been a checkbox or paragraph in another issue. I'll rebase this PR, react to API changes, unmark as draft, and ping for reviews. |
JamesNK
force-pushed
the
jamesnk/http-metrics
branch
from
21fb35f to
c491358
Compare
| // The MeterFactory property is available on handlers in .NET 8 or later. | ||
| if (PrimaryHandler is HttpClientHandler httpClientHandler) | ||
| { | ||
| httpClientHandler.MeterFactory = _meterFactory; |
There was a problem hiding this comment.
I'm not sure DefaultHttpMessageHandlerBuilder is a correct place to hardcode this injection...
Why it's not a ConfigurePrimaryHandler action added via ConfigureHttpClientDefaults or, at least, ConfigureAll?
There was a problem hiding this comment.
I updated it to use a filter.
Defaults or configure don't work because the meter factory should be applied last. If someone changes the handler, such as to a SocketsHttpHandler, then we want to apply the meter factory after that change.
|
@CarnaViire Please review and merge if good. |
There was a problem hiding this comment.
Test failures are related @JamesNK. From a brief look they are PNSE, meaning we need a ConditionalFact
ghost
added
the
needs-author-action
ghost
removed
the
needs-author-action
ghost
locked as resolved and limited conversation to collaborators
Prototype for integration between HttpClientFactory and metrics
Meteron handler fromIMeterFactory(waiting for API on handler)Note: HttpClient doesn't have metrics yet, so this PR is useless by itself. @antonfirsov is working on that. In this PR I want to experiment with designing everything end-to-end so we have an idea of what a user of HttpClient metrics APIs will look like.