AzureAD / microsoft-identity-web

Helps creating protected web apps and web APIs with Microsoft identity platform and Azure AD B2C
MIT License
663 stars 204 forks source link

Usage of the IDownstreamApi service in a worker service #2515

Open EnricoMassone opened 9 months ago

EnricoMassone commented 9 months ago

Hello, this is a question about the usage of the IDownstreamApiservice inside a .NET core generic host worker service.

The suggested approach for daemon applications is documented here. The main issue that I see is that, in order to get an instance of the IDownstreamApiservice, there is the need to create the token acquirer factory and to manually create a service provider from the token acquirer factory.

This is fine and well if you are writing a plain console application, with no built-in support for dependency injection, but can be problematic in the case of a worker service implemented as a .NET core generic host (I mean this kind of services). In the case of a .NET core generic host it would be nice being able to register the IDownstreamApiservice and all of its dependencies inside the host service collection, with no need to have a separated service provider used only to build the IDownstreamApiservice.

My first question is if there is any plan to provide this kind of functionality in the future.

A possible workaround for this limitation is creating the IDownstreamApiservice manually in the application entry point (the Mainmethod of the Programclass basically) and to register it as a singleton inside the generic host service collection. Doing so requires some manual wiring, but it is not a big deal.

This leads to my second question: is it safe to create a singleton instance of the IDownstreamApiservice and reuse it for the entire application lifetime ?

This is an example of what I mean:

public static void Main(string[] args)
{
  var tokenAcquirerFactory = TokenAcquirerFactory.GetDefaultInstance();

  tokenAcquirerFactory.Services.AddDownstreamApi("MyApi",
      tokenAcquirerFactory.Configuration.GetSection("MyWebApi"));

  var sp = tokenAcquirerFactory.Build();

  var api = sp.GetRequiredService<IDownstreamApi>();

  IHost host = Host.CreateDefaultBuilder(args)
      .ConfigureServices(services =>
      {
        services.AddSingleton<IDownstreamApi>(api);
        services.AddHostedService<Worker>();
      })
      .Build();

  host.Run();
}

I'm asking because I noticed that in the provided extension methods the IDownStreamApi service is currently registered as a scoped lifetime service.

I think that the topic of this discussion is somewhat related with issue #1755

Thanks for helping.

EnricoMassone commented 9 months ago

I did some investigation on this topic.

My conclusion is that using a singleton instance of IDownstreamApi for the entire application lifetime can be done and should not lead to any problem from the program correctness standpoint, but it is probably not the best possible choice in terms of performance.

If you take a look at the constructor of the DownstreamApiclass, it depends on the following services:

Notice that ILogger<T>, IOptionsMonitor<T> and IHttpClientFactory are all designed to be used as singleton services (that's the standard way to use them), so the only possible concern is about IAuthorizationHeaderProvider.

The sdk offers the possibility of registering IAuthorizationHeaderProvider either as a singleton or as a scoped service. By default it is registered as a scoped service, but it is possible to register it as a singleton service too. This is done inside the AddTokenAcquisitionextension method, based on the value of the isTokenAcquisitionSingletonparameter.

The possibility of registering the token acquisition services with either scoped or singleton lifestyle was discussed in #1. The final decision was to actually implement this feature.

Based on the analysis done by @bgavrilMS, when using singleton token acquisition services there may be less than optimal performances, because of lock contention. Whether or not this is an actual problem depends on the load on the involved worker service application.

So, to cut a long story short, my understanding is that using a singleton instance of IDownstreamApi does not pose any risk (at least from the program correctness standpoint).

If performance is an issue an alternative is using the service provider obtained by calling tokenAcquirerFactory.Build() as a service locator to instantiate a new instance of IDownstreamApi each time it is needed. Doing so is probably the safest approach, even though it is verbose and a bit convoluted.

Here is an example of this approach:

using Microsoft.Identity.Web;

namespace WorkerService1
{
  public class Program
  {
    public static void Main(string[] args)
    {
      var tokenAcquirerFactory = TokenAcquirerFactory.GetDefaultInstance();

      tokenAcquirerFactory.Services.AddDownstreamApi("MyApi",
          tokenAcquirerFactory.Configuration.GetSection("MyWebApi"));

      var downstreamApiServiceLocator = tokenAcquirerFactory.Build();

      IHost host = Host.CreateDefaultBuilder(args)
          .ConfigureServices(services =>
          {
            services.AddHostedService<Worker>(serviceProvider =>
              ActivatorUtilities.CreateInstance<Worker>(serviceProvider, downstreamApiServiceLocator)
            );
          })
          .Build();

      host.Run();
    }
  }
}

using Microsoft.Identity.Abstractions;

namespace WorkerService1
{
  public class Worker : BackgroundService
  {
    private readonly ILogger<Worker> _logger;
    private readonly IServiceProvider _downstreamApiServiceLocator;

    public Worker(IServiceProvider downstreamApiServiceLocator, ILogger<Worker> logger)
    {
      _logger = logger;
      _downstreamApiServiceLocator = downstreamApiServiceLocator;
    }

    protected override async Task ExecuteAsync(CancellationToken stoppingToken)
    {
      while (!stoppingToken.IsCancellationRequested)
      {
        using (var scope = _downstreamApiServiceLocator.CreateScope())
        {
          var downstreamApi = scope.ServiceProvider.GetRequiredService<IDownstreamApi>();

          // use the downstreamApi here to do whatever you need
        }

        _logger.LogInformation("Worker running at: {time}", DateTimeOffset.Now);

        await Task.Delay(1000, stoppingToken);
      }
    }
  }
}

In any case all of this seems a workaround to me. From my point of view, the correct approach should be adding the possibility to directly register the IDownstreamApi service at the generic host service collection level, without piggybacking on TokenAcquirerFactory.