In order to ease debugging efforts for 2.0, we should throw an informative exception when developers fail to add any (non-Orleans) assemblies to the ApplicationPartManager in ClientBuilder or SiloHostBuilder
I'm wondering, now that we sped up startup tremendously by not doing any scanning at startup, just at build time, should we go back to a similar default of loading what's in the directory if parts are not specified? or maybe follow references of the entry assembly (which can be tricky, as references can be optimized out if types are not explicitly used, as in most cases for a silo host).
Still, throwing an explicit exception is good, as it will be clear how to fix it.
If going into scanning for assemblies, maybe it'll be good to extend the scanning to also include "*.deps.json" (in whatever directory), since this file makes a hint on assemblies to include, even if optimized out. I did something similar (scanning and loading) some time ago, and "discovered" that the "deps" is useful. However, it's not a silver bullet...
I think scanning for assemblies should not be the default. We've seen enough confusion caused by silos trying to load every assembly they can find and polluting logs with scary looking but benign errors. The default behavior is clean now - only scan assemblies explicitly passed by the app.
AddApplicationPartsFromBasePath provides a backward compatible behavior already, but it has to be used explicitly, and it that generates noise in the logs, at least the connection will be clear.
Fine with me.
One thing to keep in mind when developing this fix (throwing an exception), is that soon some infrastructure-level configuration might add some parts automatically (for example when adding an EventHub stream provider, it will add that assembly as a part, as it has types that have serializers, but others could even have infrastructure grains too). We need to distinguish this scenario, and still throw the exception that the user did not add any parts explicitly.
@jdom I haven't submitted the PR yet, but I added an IsFrameworkAssembly flag to AssemblyPart, which is publicly accessible so that contrib/etc can use it. I decided against a hard coded list. Alternatively we could add an assembly-level attribute, like [assembly: OrleansFrameworkAssembly] (not sure what to call it)
BTW, we were just discussing a little bit with @ReubenBond, and it would actually be nice to have a default that doesn't require anything from the user. An OK approach would be yo try to load the dependency context of the app (basically feeding from the deps.json as @jan-johansson-mr suggested). This is the approach taken by the asp.net team, which they also support adding parts, but they have a default that just works.
One thing to note is that we came full circle. We wanted to avoid loading and scanning a ton of assemblies, mainly because the scanning used to be very expensive. Now it's just a matter of loading the assembly and searching for a specific assembly attribute that was added by the codegen (which btw, I don't believe we even require loading the assembly to check for this attribute, I think there is a simple way to read the metadata /cc @attilah who might know better)
Perhaps we should attempt DependencyContext.Load for all currently loaded assemblies and for any non-null results, find the assemblies which reference Orleans and load/add them
I'd like to use Roslyn Metadata APIs which works at the PE level to find those attributes without involving .Net assembly loading.
Resolved via #3644.
Most helpful comment
I think scanning for assemblies should not be the default. We've seen enough confusion caused by silos trying to load every assembly they can find and polluting logs with scary looking but benign errors. The default behavior is clean now - only scan assemblies explicitly passed by the app.
AddApplicationPartsFromBasePathprovides a backward compatible behavior already, but it has to be used explicitly, and it that generates noise in the logs, at least the connection will be clear.