HttpDependencyMetadataResolver class for the custom downstream dependency metadata resolution #6880

rainsxng · 2025-10-02T12:27:01Z

As per following API proposal, this PR exposes the DownstreamDependencyMetadataManager class with the new name that allows you to configure the downstream dependency metadata resolution and brings more flexibility into the flow

Microsoft Reviewers: Open in CodeFlow

Copilot

Pull Request Overview

This PR refactors the downstream dependency metadata resolution system by introducing a new public API HttpDependencyMetadataResolver to replace the internal IDownstreamDependencyMetadataManager interface. The changes expose the dependency metadata resolution functionality with improved naming and flexibility.

Renames internal DownstreamDependencyMetadataManager to public HttpDependencyMetadataResolver as an abstract class
Adds DefaultHttpDependencyMetadataResolver as the default implementation
Updates registration methods to use TryAddEnumerable for proper DI behavior with multiple metadata instances

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
HttpDependencyMetadataResolver.cs	Converts internal sealed class to public abstract class with documentation and removes obsolete HttpWebRequest support
DefaultHttpDependencyMetadataResolver.cs	New sealed class providing default implementation of the resolver
HttpDiagnosticsServiceCollectionExtensions.cs	Updates registration to use TryAddEnumerable and adds new AddStandardHttpDependencyMetadataResolver method
HttpRequestReader.cs	Updates field and parameter names to use new resolver type
IDownstreamDependencyMetadataManager.cs	Marks internal interface as obsolete
DownstreamDependencyMetadataManagerTests.cs	Updates test class to use new resolver type and namespace

src/Libraries/Microsoft.Extensions.Http.Diagnostics/Http/HttpDependencyMetadataResolver.cs

Copilot · 2025-10-22T15:20:25Z

...ies/Microsoft.Extensions.Http.Diagnostics/Http/HttpDiagnosticsServiceCollectionExtensions.cs

+    /// Adds services required for HTTP dependency metadata resolution.
+    /// </summary>
+    /// <param name="services">The <see cref="IServiceCollection"/> to add the services to.</param>
+    /// <returns>The <see cref="IServiceCollection"/> so that additional calls can be chained.</returns>


The documentation should clarify the relationship between AddStandardHttpDependencyMetadataResolver and AddDownstreamDependencyMetadata. It's unclear whether calling this method requires or replaces calling AddDownstreamDependencyMetadata, or if they should be used together.

Suggested change

/// <returns>The <see cref="IServiceCollection"/> so that additional calls can be chained.</returns>

/// <returns>The <see cref="IServiceCollection"/> so that additional calls can be chained.</returns>

/// <remarks>

/// <para>

/// <b>Relationship to <see cref="AddDownstreamDependencyMetadata(IServiceCollection, IDownstreamDependencyMetadata)"/> and <see cref="AddDownstreamDependencyMetadata{T}(IServiceCollection)"/>:</b>

/// </para>

/// <para>

/// <see cref="AddStandardHttpDependencyMetadataResolver(IServiceCollection)"/> only registers the default HTTP dependency metadata resolver.

/// If you need to provide custom downstream dependency metadata, use <see cref="AddDownstreamDependencyMetadata(IServiceCollection, IDownstreamDependencyMetadata)"/>

/// or <see cref="AddDownstreamDependencyMetadata{T}(IServiceCollection)"/>. These methods also register the resolver if it is not already present.

/// </para>

/// <para>

/// You do not need to call both methods unless you want to register the resolver without any downstream dependency metadata.

/// Calling <see cref="AddDownstreamDependencyMetadata"/> is sufficient if you are registering metadata, as it also adds the resolver.

/// </para>

/// </remarks>

Copilot · 2025-10-22T15:20:26Z

...ies/Microsoft.Extensions.Http.Diagnostics/Http/HttpDiagnosticsServiceCollectionExtensions.cs

-        _ = services.AddSingleton(downstreamDependencyMetadata);
+        _ = Throw.IfNull(downstreamDependencyMetadata);

+        services.TryAddEnumerable(ServiceDescriptor.Singleton<IDownstreamDependencyMetadata>(downstreamDependencyMetadata));


The change from AddSingleton to TryAddEnumerable is correct for allowing multiple metadata registrations, but there's an inconsistency: the resolver registration on line 29 uses TryAddSingleton, which means only the first resolver wins. This could cause issues if someone registers multiple metadata instances expecting them all to be used. Consider documenting this behavior or ensuring the resolver is only registered once before any metadata registrations.

dariusclay · 2025-10-29T07:46:54Z

...ies/Microsoft.Extensions.Http.Diagnostics/Http/HttpDiagnosticsServiceCollectionExtensions.cs

    {
        _ = Throw.IfNull(services);
-        services.TryAddSingleton<IDownstreamDependencyMetadataManager, DownstreamDependencyMetadataManager>();
+        services.TryAddSingleton<HttpDependencyMetadataResolver, DefaultHttpDependencyMetadataResolver>();


Since calling AddDownstreamDependencyMetadata already includes the DefaultHttpDependencyMetadataResolver, when would a user ever need AddStandardHttpDependencyMetadataResolver? Right now, it's unclear from the documentation alone and might confuse people.

Given that we auto register these resolvers, it might be a good idea to remove this excessive AddStandardHttpDependencyMetadataResolver method. @evgenyfedorov2 wdyt?

agree, let's remove AddStandardHttpDependencyMetadataResolver. It is not part of approved API either

evgenyfedorov2 · 2025-10-29T15:51:19Z

src/Libraries/Microsoft.Extensions.Http.Diagnostics/Http/HttpDependencyMetadataResolver.cs

+    /// <summary>
+    /// Initializes a new instance of the <see cref="HttpDependencyMetadataResolver"/> class.
+    /// </summary>
+    /// <param name="dependencyMetadata">A collection of HTTP dependency metadata used for request resolution.</param>
+    /// <exception cref="ArgumentNullException"><paramref name="dependencyMetadata"/> is <see langword="null"/>.</exception>


we don't have to have XML comments for protected methods

evgenyfedorov2 · 2025-10-29T15:54:16Z

src/Libraries/Microsoft.Extensions.Http.Diagnostics/Http/HttpDependencyMetadataResolver.cs


-    public RequestMetadata? GetRequestMetadata(HttpRequestMessage requestMessage)
+    /// <summary>
+    /// Gets request metadata for the specified HTTP request message.


Suggested change

/// Gets request metadata for the specified HTTP request message.

/// Gets request metadata for the specified instance of <see cref="HttpRequestMessage">.

evgenyfedorov2 · 2025-10-29T15:55:23Z

...crosoft.Extensions.Http.Diagnostics/Logging/Internal/IDownstreamDependencyMetadataManager.cs


 /// <summary>
-/// Interface to manage dependency metadata.
+/// (Obsolete) Use <see cref="HttpDependencyMetadataResolver"/>.


why not just remove this?

evgenyfedorov2 · 2025-10-29T15:57:16Z

...ibraries/Microsoft.Extensions.Http.Diagnostics/Http/DefaultHttpDependencyMetadataResolver.cs

+/// trie-based lookup algorithm.
+/// </summary>
+[Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)]
+public sealed class DefaultHttpDependencyMetadataResolver : HttpDependencyMetadataResolver


does this need to public?

evgenyfedorov2 · 2025-10-29T15:59:01Z

...ies/Microsoft.Extensions.Http.Diagnostics/Http/HttpDiagnosticsServiceCollectionExtensions.cs

    {
        _ = Throw.IfNull(services);
-        services.TryAddSingleton<IDownstreamDependencyMetadataManager, DownstreamDependencyMetadataManager>();
+        services.TryAddSingleton<HttpDependencyMetadataResolver, DefaultHttpDependencyMetadataResolver>();


agree, let's remove AddStandardHttpDependencyMetadataResolver. It is not part of approved API either

new metadata resolver api

1c084be

github-actions bot added the area-telemetry label Oct 2, 2025

dotnet-policy-service bot assigned rainsxng Oct 2, 2025

rainsxng added 7 commits October 2, 2025 15:48

extension methods for the metadata resolvers

11ccdb3

update di resolution

798cd50

update metadata resolver usage, fix tests

040e51b

update api according to the proposal

51e6cce

remove conditional compilation

4fdeeb3

fix lint

397d5df

remove unused using

e045b3b

rainsxng changed the title ~~new metadata resolver api~~ HttpDependencyMetadataResolver class for the custom downstream dependency metadata resolution Oct 22, 2025

Merge branch 'main' into dbohdanov/metadata-resolver-api

edf2fbf

rainsxng marked this pull request as ready for review October 22, 2025 15:19

rainsxng requested a review from a team as a code owner October 22, 2025 15:19

Copilot AI review requested due to automatic review settings October 22, 2025 15:19

Copilot AI reviewed Oct 22, 2025

View reviewed changes

rainsxng added 2 commits October 22, 2025 17:35

update extension methods

fa5a9b6

fix warnings

ba2c5f3

dariusclay reviewed Oct 29, 2025

View reviewed changes

evgenyfedorov2 requested changes Oct 29, 2025

View reviewed changes

-    /// <returns>The <see cref="IServiceCollection"/> so that additional calls can be chained.</returns>
+    /// <returns>The <see cref="IServiceCollection"/> so that additional calls can be chained.</returns>
+    /// <remarks>
+    /// <para>
+    /// <b>Relationship to <see cref="AddDownstreamDependencyMetadata(IServiceCollection, IDownstreamDependencyMetadata)"/> and <see cref="AddDownstreamDependencyMetadata{T}(IServiceCollection)"/>:</b>
+    /// </para>
+    /// <para>
+    /// <see cref="AddStandardHttpDependencyMetadataResolver(IServiceCollection)"/> only registers the default HTTP dependency metadata resolver.
+    /// If you need to provide custom downstream dependency metadata, use <see cref="AddDownstreamDependencyMetadata(IServiceCollection, IDownstreamDependencyMetadata)"/>
+    /// or <see cref="AddDownstreamDependencyMetadata{T}(IServiceCollection)"/>. These methods also register the resolver if it is not already present.
+    /// </para>
+    /// <para>
+    /// You do not need to call both methods unless you want to register the resolver without any downstream dependency metadata.
+    /// Calling <see cref="AddDownstreamDependencyMetadata"/> is sufficient if you are registering metadata, as it also adds the resolver.
+    /// </para>
+    /// </remarks>

	/// Gets request metadata for the specified HTTP request message.
	/// Gets request metadata for the specified instance of <see cref="HttpRequestMessage">.

Uh oh!

HttpDependencyMetadataResolver class for the custom downstream dependency metadata resolution #6880

Are you sure you want to change the base?

HttpDependencyMetadataResolver class for the custom downstream dependency metadata resolution #6880

Uh oh!

Conversation

rainsxng commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rainsxng commented Oct 2, 2025 •

edited

Loading