Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tdg5 opened this issue Nov 5, 2024 · 2 comments · May be fixed by #364
Open

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

tdg5 opened this issue Nov 5, 2024 · 2 comments · May be fixed by #364
Labels
enhancement New feature or request

Comments

@tdg5
Copy link

tdg5 commented Nov 5, 2024

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 tdg5 added the enhancement New feature or request label Nov 5, 2024
@tdg5
Copy link
Author

tdg5 commented Nov 5, 2024

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 #364 with a sketch for discussion.

@cretz
Copy link
Member

cretz commented Nov 12, 2024

👍 Discussing on PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants