-
Notifications
You must be signed in to change notification settings - Fork 69
Execution-scoped data in the Polly-HttpClientFactory integration: enabling rich and correct telemetry, and CachePolicy #62
Description
Polly Policy instances can be used across multiple call sites for intended functional outcomes. This is a good fit for named HttpClient configurations on HttpClientFactory with a BaseUri eg api.contoso.com indicating all calls will be to endpoints on api.contoso.com.
To complement re-use, Polly provides an execution-scoped Context which travels with every execution. This:
(a) provides a correlation id unique to each execution, allowing logging and telemetry to correlate the events particular to a single execution;
(b) distinguishes different call sites at which Policy instances may be being used, allowing logging and telemetry to correlate statistics particular to a single usage site/call path;
- For instance, it is typical to use the same circuit-breaker instance for all endpoints on
api.contoso.com, but when analysing failures, it can be very useful to extract that call path A tended to be healthy but call path B tended to require retries, break circuits, etc.
(c) allows the functioning of CachePolicy. With CachePolicy, one typically configures a single CachePolicy instance, then Context supplies the key that the policy should use for caching items on that particular call path.
The execution-scoped Polly.Context is already made available to policy delegate hooks (eg onRetry, onBreak etc) and will also be available to forthcoming raw telemetry events.
Without HttpClientFactory doing anything around this, correlation ids (a) would be inconsistent in cases where multiple Polly policies are applied by layering DelegatingHandlers (each PolicyHttpMessageHandler layer would generate a fresh guid). And features (b) and (c) would not be available.
Options for supporting Polly.Context in PolicyHttpMessageHandler
Here within PolicyHttpMessageHandler we could:
Polly.Context pollyExecutionContext = request.Properties[PollyContext] ?? new Polly.Context(); // [1]
request.Properties[PollyContext] = pollyExecutionContext; // [2]
return _policy.ExecuteAsync((context, token) => base.SendAsync(request, token), pollyExecutionContext, cancellationToken); // [3]where:
private static readonly string PollyContext = "PollyExecutionContext"; // [4][1] and [2] ensure when there is a chain of delegating handlers, they all use the same context, fixing (a) above. [3] just despatches so that all Polly logging and telemetry has access to the Context.
If we changed [4] to:
public static readonly string PollyContext = "PollyExecutionContext"; // [4b]then users who desired, would have this usage:
request.Properties[PolicyHttpMessageHandler.PollyContext] = new Polly.Context("MyUsageKey"); // [5]
var response = client.DoWhateverAsync(request...); // (as usual)This enables and fixes both use cases (b) and (c) above.
Stepping back: the issue here is that Polly users normally code the fooPolicy.ExecuteAsync(...) call directly, so can pass in a Context. In the HttpClientFactory integration, that call is (necessarily) within PolicyHttpMessageHandler.cs, so the user cannot directly pass in Context.
HttpClient being (presumably) not for modification, HttpRequestMessage.Properties seems like the intended(?)/available place for attaching execution-scoped data.
The extra line, [5], would not be necessary for ordinary usage. It would be an available option, for those who wanted the richer telemetry or to use CachePolicy.
Thoughts? Should this be part of the Polly-HttpClientFactory story? Are there any good alternatives?