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

ServiceProvider is not scoped in outgoing behaviors hosted in the Generic Host #6944

Closed
sylvaingirardbe opened this issue Jan 17, 2024 · 10 comments

Comments

@sylvaingirardbe
Copy link

sylvaingirardbe commented Jan 17, 2024

Describe the bug

Description

The ServiceProvider in behaviors seems not to be scoped (throwing Cannot resolve scoped service ‘ITerminalContext’ from root provider) when you host your endpoint in the Generic Host. I used the sample regarding DI found here: https://docs.particular.net/samples/dependency-injection/aspnetcore/ and modified it a little bit to reproduce the issue. This repro can be found here: https://github.com/sylvaingirardbe/nsb-scoped-di-repro. Dennis van der Stelt provided a link to the multi-tenant sample here: https://discuss.particular.net/t/use-data-from-message-handler-in-a-behavior/3755 and this basically is the use case I had in mind, except for a few terminological changes and running it in a Generic Host.

Note: that this only seems to be the case for outgoing contexts.

Expected behavior

The service should be resolved for the correct scope.

Actual behavior

Throws an exception

Versions

image

Steps to reproduce

https://github.com/sylvaingirardbe/nsb-scoped-di-repro

Relevant log output

No response

Additional Information

Workarounds

A whole lot of intrusive code to manage UoW data.

@sylvaingirardbe sylvaingirardbe changed the title ServiceProvider is not scoped in behaviors hosted in the Generic Host ServiceProvider is not scoped in outgoing behaviors hosted in the Generic Host Jan 17, 2024
@danielmarbach danielmarbach removed the Bug label Jan 17, 2024
@danielmarbach
Copy link
Contributor

danielmarbach commented Jan 17, 2024

@sylvaingirardbe

NServiceBus creates a child provider for everything that is executed while handling an incoming message. The child provider is exposed on the context like your sample already demonstrates in

var terminalContext = context.Builder.GetRequiredService<ITerminalContext>();

For behaviors in the outgoing stages the builder behavior switches between the two following scenarios:

  • Sending a message outside a message handler session.Send/Publish
  • Sending a message inside a message handler context.Send/Publish/Reply

When sending a message outside a message handler the builder is the root container. Many any scoped dependencies would effectively become singletons.

When sending inside a message handler the builder gets "adjusted" to be a child builder.

In your case because the OutgoingTerminalBehavior does context.Builder.GetRequiredService<ITerminalContext>() and your startup class endpointInstance.SendLocal(myMessage) the builder there is the root since it falls into the "sending outside a message handler scenario"

This behavior is by design because the message session is a singleton and has no access to for example child providers that might be spawned by something like ASP.NET Core. Even if we would create a child builder as part of a message session operation this would potentially not solve your problem since I understand you are trying to set data in the terminal context probably somewhere within ASP.NET Core that should then be available inside the outgoing behavior, is that correct?

In cases when you want to "flow information" from the outside a message session send you can do the following:

  • For primitive values you can use the options like SendOptions to set "headers" that is then transport into the outgoing parts of the pipeline as well as the outgoing message
  • You can use the options like SendOptions Extensionbag to add contextual information that is than available inside the outgoing pipeline sendOptions.GetExtensions().Set
  • You can use a singleton that provides an ambient context state property that uses an AsyncLocal that you are setting that is then available inside the outgoing pipeline

In case you only ever want to propagate a header from the incoming message to the outgoing messages you can guard against that case in the outgoing pipeline by doing something like

public class OutgoingTerminalBehavior : 
    Behavior<IOutgoingLogicalMessageContext>
{
    public override Task Invoke(IOutgoingLogicalMessageContext context, Func<Task> next)
    {
       if(context.TryGetIncomingPhysicalMessage(out _)) {
           var terminalContext = context.Builder.GetRequiredService<ITerminalContext>();
           context.Headers["x-terminal"] = terminalContext.Terminal.ToString();
       }
        return next();
    }
}

which then makes sure Builder is only ever the child builder since you are in the cases of inside the message handler.

@sylvaingirardbe
Copy link
Author

Thanks for the detailed explanation. Seeing it like this makes it obvious that it's the root container at that moment.

What I was trying to solve is that incoming http requests contain a terminal code in the headers that needs to be propagated to any outgoing message as a result of that http request. The terminal code is set on the ITerminalContext in a piece of middleware. The idea was to use this context to add the terminal code automatically to outgoing messages.

Thanks for your suggestions.

@danielmarbach
Copy link
Contributor

danielmarbach commented Jan 18, 2024

@sylvaingirardbe If it is about correlation according to the terminal value, you could leverage NServiceBus OpenTelemetry support. You have multiple options available that fall into the category of "ambient state" mentioned above.

As an example, you could have your own ActivitySource that starts an activity and set the terminal code on the activity source based on the HTTP header as a tag. You can then use that tag to then set the outgoing message header inside a behavior, since that activity should be visible inside the behavior. This sample gives you a hint how to do it

https://docs.particular.net/samples/open-telemetry/customizing/

You would need a combination of custom activity source, Activity.Current.SetTag and Activity.Current.GetTagItem. This also has the benefit of the terminal code being visible in your monitoring solution on the trace for the current process.

https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry.Api/README.md

Depending on how far you want the terminal code to propagate, you can also use the baggage API. Things added to the baggage are transferred across process boundaries. NServiceBus also adds information on the bag automatically to a specific header that will contain a concatenated list of the bag items and extracts them on the receiving end back from that header into the baggage. The ASP.NET Core integration for OpenTelementry has an enrichment capability that allows you to automatically enrich activities based on the httpRequest. You can use that to set the baggage, which will then automatically propagate out over NServiceBus as mentioned above.

Be aware that Baggage has two occurrences. Once as part of System.Diagnostics and one as part of OpenTelemetry and they are not the same. See this discussion

RehanSaeed/Serilog.Enrichers.Span#164

NServiceBus currently propagates Activity.Current.Baggage keys and values to the header called baggage.

(*) Of course you can also use the same enrichment capability to set a tag (which would only apply for the current processes) but then you would still need to propagate that to the outgoing message headers manually as mentioned above.

@danielmarbach
Copy link
Contributor

Another option that falls into ambient state category would be to use the HttpContextAccessor directly in the outgoing behavior and then map the terminal code HTTP header there directly to the message header when the HttpContext is not null and the header is there.

@sylvaingirardbe
Copy link
Author

@sylvaingirardbe If it is about correlation according to the terminal value, you could leverage NServiceBus OpenTelemetry support. You have multiple options available that fall into the category of "ambient state" mentioned above.

As an example, you could have your own ActivitySource that starts an activity and set the terminal code on the activity source based on the HTTP header as a tag. You can then use that tag to then set the outgoing message header inside a behavior, since that activity should be visible inside the behavior. This sample gives you a hint how to do it

https://docs.particular.net/samples/open-telemetry/customizing/

You would need a combination of custom activity source, Activity.Current.SetTag and Activity.Current.GetTagItem. This also has the benefit of the terminal code being visible in your monitoring solution on the trace for the current process.

https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry.Api/README.md

Depending on how far you want the terminal code to propagate, you can also use the baggage API. Things added to the baggage are transferred across process boundaries. NServiceBus also adds information on the bag automatically to a specific header that will contain a concatenated list of the bag items and extracts them on the receiving end back from that header into the baggage. The ASP.NET Core integration for OpenTelementry has an enrichment capability that allows you to automatically enrich activities based on the httpRequest. You can use that to set the baggage, which will then automatically propagate out over NServiceBus as mentioned above.

Be aware that Baggage has two occurrences. Once as part of System.Diagnostics and one as part of OpenTelemetry and they are not the same. See this discussion

RehanSaeed/Serilog.Enrichers.Span#164

NServiceBus currently propagates Activity.Current.Baggage keys and values to the header called baggage.

(*) Of course you can also use the same enrichment capability to set a tag (which would only apply for the current processes) but then you would still need to propagate that to the outgoing message headers manually as mentioned above.

That's quite ingenious but it's not merely about correlation. We're using the terminal code to filter data and check against the user's claims so I'm not feeling too comfortable to use opentelemetry for this, for some reason.

@sylvaingirardbe
Copy link
Author

Another option that falls into ambient state category would be to use the HttpContextAccessor directly in the outgoing behavior and then map the terminal code HTTP header there directly to the message header when the HttpContext is not null and the header is there.

Yep, thought about that and that might actually be the cleanest atm.

@danielmarbach
Copy link
Contributor

That's quite ingenious but it's not merely about correlation. We're using the terminal code to filter data and check against the user's claims so I'm not feeling too comfortable to use opentelemetry for this, for some reason.

I can understand that. I was more coming from the angle of having the terminal code within the traces might be super helpful anyway and then it would automatically float across boundaries too if you want that to happen.

@danielmarbach
Copy link
Contributor

@sylvaingirardbe based on your investigations do you have any suggestions on how we could make the documentation a bit clearer?

@sylvaingirardbe
Copy link
Author

I see that it is implicitly documented here. I'd explicitly state the fact that this is not the case when you send a message with an IMessageSession instance. And maybe add such a case in the UoW example.

@danielmarbach
Copy link
Contributor

Raised Particular/docs.particular.net#6486. Closing

@danielmarbach danielmarbach closed this as not planned Won't fix, can't repro, duplicate, stale Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants