Particular / NServiceBus.Persistence.Sql

Native SQL Persistence for NServiceBus
https://docs.particular.net/persistence/sql/
Other
36 stars 27 forks source link

Referenced NuGet packages with saga implementations do not generate SQL scripts in host project #761

Closed markphillips100 closed 3 years ago

markphillips100 commented 3 years ago

I've create a sample to demonstrate an issue I'm seeing, or at least an observation that doesn't meet my expectation. The sample is on github here. Just following the repository's readme.

The sample demonstrates that saga scripts are not generated for NuGet referenced packages containing saga code in a consuming project's bin directory. They are included in the app's bin directory for project references.

As the sample shows, this occurs even though directly referencing NServiceBus.Persistence.Sql in the projects containing the saga and also in the app referencing those projects.

Should it be possible to have the saga SQL scripts generated when referencing packges with saga code in them? If so any ideas what I'm doing wrong?

bording commented 3 years ago

Hi @markphillips100,

The script generation runs as part of the compilation for a project, and it only inspects the assembly being compiled in that project. That means that the scripts for samplelib2 are created when the samplelib2 project itself is compiled, which you should be able to verify by looking at the bin folder of that project.

When compiling your sampleapp project, script generation runs and generates scripts for the sampleapp assembly. The project reference to samplelib means that samplelib is also compiled, and script generation runs for it as well. Because sampleapp has a project reference to samplelib, the scripts generated in the samplelib bin folder are copied to sampleapp's bin folder as well.

However, since samplelib2 is not being compiled as part of compiling sampleapp, no script generation is being triggered at that time.

If you want to distribute saga code in a NuGet package, when the project that creates the package builds, you need to bundle the generated scripts as content in the NuGet package itself. They can be included in the package in a way that indicates that they should be copied to the referencing project's bin folder.

If you do that, you should get the desired behavior.

markphillips100 commented 3 years ago

Perhaps that information might be really useful in documentation? I would have thought it quite a common use case to package these projects as the only artifacts of a single business service repository is all.

markphillips100 commented 3 years ago

It's also a little disappointing that the package needs to do this at all considering it should be published agnostic of any persistence dependency.

Is there no other way of achieving the goal of generating scripts?

markphillips100 commented 3 years ago

If I run the NServiceBus.Persistence.Sql.CommandLine global tool on the samplelib2 dll it does generate saga scripts. I assume this tool uses reflection rather than MSBuild to do this?

markphillips100 commented 3 years ago

Okay so the global tool uses Mono.Cecil to inspect dlls. This requires running the tool against each dll that (I know) has a saga within it. I can't run it against the host (endpoint) dll as I don't believe the tool walks a dependency hierarchy (or even could if it wanted to).

I'm wondering if a post-build msbuild exec task could be used to run the global tool over all package references from the host project (using a glob pattern to exclude obvious non-business packages)?

DavidBoike commented 3 years ago

Perhaps that information might be really useful in documentation?

The script creation is already documented here. As the scripts are part of the compiled output, if you choose to package a saga's compiled output into a NuGet package, then that package should contain all of the outputs, not just the binaries.

But honestly packaging sagas into a NuGet package for reuse elsewhere isn't something I've really heard of happening with any sort of frequency. Could you explain more about your specific use case there?

To answer your last question, removing the MSBuild dependency and generating the scripts in a different way is something that is on our radar, but if we did that we would need to be careful to not break workflows for existing users. So for now, they're generated as part of the build process.

markphillips100 commented 3 years ago

Hi David.

From a use case perspective I am merely following Udi's teachings. He states in his video series that the only artifacts of a business service repository are NuGet packages (ACs). Notably these do contain sagas, as well as other handlers etc. The intention being to deploy these packages into physical endpoints as required from whatever scale requirements are necessary.

In short, my physical endpoint code and my services code are not in the same repository. Is this not the guidance Udi expressed?

DavidBoike commented 3 years ago

In that case, you're just using a NuGet package as a vehicle to distribute build outputs. You could use a zip file too, the packaging is just an implementation detail. That means that the SQL scripts (which are also build outputs) also need to be in the package as well. It's not the responsibility of the hosting endpoint to create additional build outputs from your build outputs - that's precisely the type of coupling you're trying to avoid in the first place.

markphillips100 commented 3 years ago

For now I guess I'll see about packaging the outputs with the nuget containing the saga. But I would much prefer to just run a tool over a convention glob pattern of dlls to generate scripts.

Your last sentence re coupling has me worried. I thought the only coupling we were trying to avoid was between business services, not business and ITOps?

Just to clarify my meaning, I see ITOps as responsible for deployment of hosts and whatever endpoints are required, what messages route to which endpoints, etc. Likewise it's responsible for any infrastructure dependencies, including generating migration scripts for the target database type. I also generate migration scripts in the hosts repository for any DBContext that is used by ACs. I don't generate their scripts as part of a build in the business service.

My system architecture could easily be wrong (too coupled) but would like to clarify over in the discussion forum. Please feel free to chime in as it's important to me I understand if I'm going down a wrong path.

DavidBoike commented 3 years ago

I'm really not sure. I really think an autonomous service should really host its own endpoints. Sagas are business logic that are internal to the service and shouldn't really be composed with other things. It would be especially worrisome if 2 sagas that handled the same event would get composed into the same host, because sagas and message handlers don't subscribe to events…the endpoint does. So in that case, one endpoint would get one copy of an event, and would have to invoke both saga handlers in a single transactional boundary, which can build contention. IT/Ops would host specific integration endpoints - websites or things that generate emails where you have to do some ViewModel composition and then code from different services would packaged in NuGet packages and deployed to those IT/Ops endpoints. But not sagas.

I don't think there's any clear answer.

But if you want to go that way, you could run the command-line tool on every .NET DLL in a location. If it doesn't contain sagas it just won't do anything. I'm not sure you have to have knowledge beforehand that a DLL can or should contain a saga.

markphillips100 commented 3 years ago

That insight on contention is very useful to know and not something I had considered. I do think though that it is a concern of ITOps rather than the business service to care about the physical deployment and separation concerns. At least, this is my understanding from Udi's teachings.

What I think I understand from what you are saying is that business service saga code should never be hosted alongside any other handlers/sagas, whether from the same business service or not. And in addition should be isolated to their own endpoint. I've not seen or heard that as guidance but would be grateful to be pointed towards any.

Is it possible @udidahan could chime in on this topic perhaps?

DavidBoike commented 3 years ago

I wouldn't say a saga must be completely isolated in its own endpoint, but it shouldn't be co-hosted with any other handlers (or sagas) processing the same message type.

markphillips100 commented 3 years ago

Thanks for the advice @DavidBoike. I understand now the contention is due to the processing of handlers/sagas for the same message. I shall keep that in mind from a deployment perspective.

By the way, I don't suppose there's a feature we can enable to prevent an endpoint ever accidentally registering more than one handler/saga for any individual event type?

DavidBoike commented 3 years ago

Not built in, some systems rely on that for advanced scenarios. But you could implement a check with a feature like this:

using NServiceBus;
using NServiceBus.Configuration.AdvancedExtensibility;
using NServiceBus.Features;

public class CheckForMultipleHandlersPerMessageTypeFeature : Feature
{
    public CheckForMultipleHandlersPerMessageTypeFeature()
    {
        EnableByDefault();
    }

    protected override void Setup(FeatureConfigurationContext context)
    {
        var allHandlerTypes = context.Settings.GetAvailableTypes()
            .Where(type => typeof(IHandleMessages<>).IsAssignableFrom(type));

        // Do some reflection and throw if you don't like what you find
    }
}
markphillips100 commented 3 years ago

That's awesome @DavidBoike . I shall definitely make use of that. Thanks once again.