prometheus-net-contrib icon indicating copy to clipboard operation
prometheus-net-contrib copied to clipboard

HttpClientListenerHandler handle TaskCanceledException

Open faustodavid opened this issue 4 years ago • 2 comments

When using the HttpClientListenerHandler and an http request task is canceled either by the user or by a timeout, the call is not registered in the metric.

faustodavid avatar Apr 13 '21 16:04 faustodavid

Each DiagnosticListenerHandler has a static class PrometheusCounters, I would like to propose to move the creation of the counter a level up without making them static, this can help us to test the DiagnosticListenerHandler.

Something like:

 public class SomeListenerHandler : DiagnosticListenerHandler
{
    private readonly ISomeListenerHandlerCounters _counters;

    public HttpClientListenerHandler(string sourceName, ISomeListenerHandlerCounters counters) : base(sourceName)
    {
       _counters = counters;
    }

    public override void OnStopActivity(Activity activity, object payload)
    {
      ...
    }
}

 public static class DiagnosticServiceCollectionExtensions
 {
    public static void AddPrometheusSomeMetrics(this IServiceCollection services, ISomeListenerHandlerCounters counters = null)
        {
           counters ??= new SomeListenerHandlerCounters();
            var someListenerHandler = new DiagnosticSourceSubscriber(
                name => new SomeListenerHandler(name, counters),
                listener => listener.Name.Equals("SomeHandlerDiagnosticListener"));
            someListenerHandler.Subscribe();

            services.AddSingleton(someListenerHandler);
        }
 }

faustodavid avatar Apr 13 '21 18:04 faustodavid

Do you have a preference on how to test the code?

Let me know if you prefer to don't introduce the new interfaces because of the extra complexity/perf-impact, I can rollback these changes and just add the condition for the TaskCanceledExceptions .

faustodavid avatar Apr 13 '21 20:04 faustodavid