[API Proposal]: HttpClient telemetry extensibility

antonfirsov commented 3 weeks ago

Background and motivation

There are 2 feature requests for .NET 9 which require configuring HttpClient telemetry and logging behavior on per-request basis:

To address concerns described in #93221, we need to introduce a Uri redaction mechanism that allows redacting PII from the url-s emitted by Distributed Tracing, EventSource events and HttpClientFactory loggers and potentially in Metrics.
93019 requires us to introduce enrichment and filtering capabilities for HttpClient's Distributed Tracing. It seems reasonable to match MetricsEnrichmentContext functionality enabling the registration of enricher and filtering callbacks per-request, enabling various extension points (eg. handlers) to add their own callbacks independently.

Besides redaction, in the future we may want to implement a url templatization mechanism so we can emit url.template for metrics and activities where the template has to be set on a per-request basis.

We could introduce distinct utility methods for each feature like we did when we added MetricsEnrichmentContext.AddCallback. However, this would lead to API friction and poor discoverability.

Instead, I'm proposing a new type, HttpRequestDiagnosticOptions to encapsulate all settings that allow customizing telemetry behavior on a per-request basis. I'm also proposing to move the existing metrics enrichment callback registration functionality to the new type.

API Proposal


namespace System.Net.Http;

// The new type to encapsulate telemetry customization options.
public sealed class HttpRequestDiagnosticOptions
{
    // Enable handlers to append new callbacks for enrichment.
    public void AddActivityEnrichmentCallback(Action<HttpActivityEnrichmentContext> callback);

    // Follow the same pattern for filters, however we may consider to get a way with a single filter.
    public void AddActivityFilter(Predicate<HttpRequestMessage> filter);
    // public Predicate<HttpRequestMessage>? ActivityFilter { get; set; } // ???

    // Fully customizable Uri redaction mechanism that might be used HttpClientFactory and 3rd party libs.
    // Shall we pass HttpRequestMessage instead of Uri?
    public Func<Uri, string?>? UriRedactorCallback { get; set; }

    // Move MetricsEnrichmentContext.AddCallback() functionality into the new type.
    public void AddMetricsEnrichmentCallback(Action<Metrics.HttpMetricsEnrichmentContext> callback);
}

// Enrichment happens when a response is received or an exception is thrown.
public sealed class HttpActivityEnrichmentContext
{
    private HttpActivityEnrichmentContext() { }

    // Expose the activity so users can add or potentially modify tags.
    public Activity Activity { get; } }
    public HttpRequestMessage Request { get; }
    public HttpResponseMessage? Response { get; }
    public Exception? Exception { get; }
}

// Expose the diagnostic options through a new property on the request object
public class HttpRequestMessage
{
    public HttpRequestDiagnosticOptions DiagnosticOptions { get; }
}

namespace System.Net.Http.Metrics;

// To harmonize the API, move the metrics enrichment callback registration functionality from HttpMetricsEnrichmentContext to HttpRequestDiagnosticOptions.
public sealed class HttpMetricsEnrichmentContext
{
    // We may even consider obsoletion.
    // [Obsolete("Use HttpRequestDiagnosticOptions.MetricsEnrichmentCallbacks.")]
    [EditorBrowsable(EditorBrowsableState.Never)]
    public static void AddCallback(HttpRequestMessage request, Action<HttpMetricsEnrichmentContext> callback);
}

API Usage

1. Redacting `Uri`-s

If redirects are not expected, the application can configure a constant placeholder string to log instead of the actual Uri:

using HttpClient client = new();
using HttpRequestMessage request = new(HttpMethod.Get, "https://aa.org/u/231");
request.DiagnosticOptions.UriRedactorCallback = _ => "https://aa.org/u/REDACTED";
await client.SendAsync(request);

One possible way to handle redirects:

using HttpClient client = new();
using HttpRequestMessage request = new(HttpMethod.Get, "https://aa.org/u/321");
request.DiagnosticOptions.UriRedactorCallback = uri =>
{
    if (uri.Host == "aa.org" && uri.LocalPath.StartsWith("/u/"))
    {
        return "https://aa.org/u/REDACTED";
    }

    return "REDIRECTED";
};

2. Activity enrichment

using HttpClient client = new();
using HttpRequestMessage request = new("https://aa.org/u/321");
request.DiagnosticOptions.AddActivityEnrichmentCallback(ctx =>
{
    ctx.Activity.AddTag("url.template", "https://aa.org/u/{userid}");
});
using HttpResponseMessage response = await client.SendAsync(request);

3. Activity filtering

[TODO]

4. Metrics enrichment

[TODO]

Alternative Designs

There are several aspects where we could consider alternative apporoaches.

1. Overall API shape

Instead of introducing HttpRequestDiagnosticOptions expose utility methods.
Do not introduce strongly-typed API-s for anything but activity enrichment. Use well-known option names for the rest.

2. Expose callback collections via `IList<T>` properties instead of `Add*Callback()` methods:

public IList<Action<HttpActivityEnrichmentContext>> ActivityEnrichmentCallbacks { get; }

This would make it more difficult to expose additional enrichment callbacks registration when the request is being sent. With the current method approach we can do the following:

public enum EnrichmentTiming
{
    RequestStart,
    RequestEnd
}

public sealed class HttpRequestDiagnosticOptions
{
    // New overload to control the enrichment timing.
    public void AddActivityEnrichmentCallback(Action<HttpActivityEnrichmentContext> callback, EnrichmentTiming timing);
}

3. Instead of `UriRedactorCallback`, expose pre-defined redaction modes.

This might be easier to use for the most common use-cases, but less flexible. Eg. it wouldn't allow users to handle redirects.

public enum UriRedactionMode
{
    EmitWithoutQueryString,
    EmitFull,
    DoNotEmit,
    EmitPlaceholderString
}

public sealed class HttpRequestDiagnosticOptions
{
    public UriRedactionMode UriRedactionMode { get; set; }
    public string? PlaceholderString { get; set; }
}

Risks

UriRedactionCallback turns out to be too difficult to use
We would need to implement Activity enrichment also for request start
Users would start to use activity enrichment for general Activity manipulation tasks, so it wouldn't be "enrichment" any longer.

dotnet-policy-service[bot] commented 3 weeks ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

julealgon commented 3 weeks ago

@antonfirsov instead of a "list of callbacks" or AddCallback methods, why not leverage IObservable instead?

martincostello commented 3 weeks ago

I know the examples are just that, but they made me consider something, so I'm going to mention it here for something to consider towards the design.

The examples use hard-coded URLs, so the conditional filters/templates etc are also specified inline to do the redaction using the same hard-coded values. For applications where the host is configured through IConfiguration, it should be relatively simple to match a request URI against resolved configuration/options. Sometimes callback-based APIs cause friction here due to lack of access to the service provider or needing to capture state into it because of that.

antonfirsov commented 3 weeks ago

For applications where the host is configured through IConfiguration, it should be relatively simple to match a request URI against resolved configuration/options. Sometimes callback-based APIs cause friction

Wouldn't that matching be also done by a callback? Or would you expect the HttpClientFactory middleware to implement it? Can you elaborate on your idea with some examples?

martincostello commented 3 weeks ago

So with OpenTelemetry as an example, the enrichment is typically configured as part of the IoC registrations, and the IServiceProvider is available to be able to reach into the configuration to do matching, and knowledge of OpenTelemetry is kept out of the knowledge of the callsite of the HttpClient itself.

Essentially, anything regarding DiagnosticOptions would be configured via HttpClientFactory and transparent to the consuming code. Something similar to the below is what I'm thinking of:

Configuration:

services.AddHttpClient("MyClient", (client, serviceProvider) =>
{
    var configuration = serviceProvider.GetRequiredService<IConfiguration>();
    client.BaseAddress = new(configuration["MyClientBaseAddress"]);
}).ConfigureDiagnosticOptions((options, serviceProvider) =>
{
    // This example is just illustrative - as written it's not very efficient or well-named
    options.UriRedactorCallback = uri =>
    {
        // If IConfiguration supports being reloaded remotely, it needs to be retrieved per-request.
        // A better approach would be via IOptionsMonitor<T> with bound configuration, for example.
        var configuration = serviceProvider.GetRequiredService<IConfiguration>();
        var baseAddress = new(configuration["MyClientBaseAddress"])
        if (uri.Host == baseAddress.Host && uri.LocalPath.StartsWith("/u/"))
        {
            return $"{baseAddress}/u/REDACTED";
        }

        return "REDIRECTED";
    };
});

Usage:

using HttpClient client = httpClientFactory.CreateClient("MyClient");
using HttpRequestMessage request = new(HttpMethod.Get, "/u/321");
using HttpResponseMessage response = await client.SendAsync(request);

antonfirsov commented 6 days ago

There was a strong pushback against the proposed API shape because the way it's tied to a request. We were not able to gather enough information on time to create a better design everyone will be happy about. Pushing this to .NET 10.

dotnet / runtime