search

temporalio / sdk-dotnet

Temporal .NET SDK
MIT License
394 stars 33 forks source link

[Feature Request] Make scoped IServiceProvider available to ActivityInboundInterceptor #363

Open tdg5 opened 3 weeks ago

tdg5 commented 3 weeks ago

Is your feature request related to a problem? Please describe.

Interceptors offer a means of handling cross-cutting concerns in a way that insulates any given Activity instance from those cross-cutting concerns. However, if a given cross-cutting concern depends on a service provided by the scoped IServiceProvider instance, this insolation can't be maintained because only Activities can have dependencies injected via constructor injection.

As far as I understand things, there's no philosophical reason an ActivityInboundInterceptor couldn't have the scoped IServiceProvider instance made available when the ExecuteActivityAsync method is invoked.

Describe the solution you'd like

Constructor injection wouldn't work for ActivityInboundInterceptor because the interceptors don't share the same lifetime as an Activity. However, I believe that when ExecuteActivityAsync, it should be in the context of an Activity. I'm not looking for method signature injection, I'd be satisfied with just getting access to the IServiceProvider instance and having to pull out dependencies by other means.

That said, this is all complicated by the fact that Temporalio.Worker.Interceptors is agnostic of IServiceProvider and or Microsoft.Extensions.DependencyInjection and the creation of the scoped IServiceProvider is locked up in Temporalio.Extensions.Hosting.ServiceProviderExtensions.

Something like ASP.NET's HttpContextAccessor could be repurposed as a ServiceLocator for the scoped IServiceProvider instance, but it's not so elegant.

Additional context

I'm happy to help make this happen if there's a path forward, but I don't see an implementation I can obviously make a pull request from.

tdg5 commented 3 weeks ago

Thinking about this some more, I think I'd vote against changing the method signature of ExecuteActivityAsync. I think some mechanism completely separate from the ActivityInboundInterceptor is the cleanest (er, decoupled?) option that allows for keeping the knowledge about IServiceProvider hidden from the more agnostic portions of Temporalio.

I opened https://github.com/temporalio/sdk-dotnet/pull/364 with a sketch for discussion.

cretz commented 2 weeks ago

:+1: Discussing on PR.