.Net: New Feature: Improve Dependency Injection

[rosieks]

Please make Dependency Injection easier. According to documentation current way to do that is:

// Create singletons of your plugins
builder.Services.AddSingleton(() => new LightsPlugin());
builder.Services.AddSingleton(() => new SpeakerPlugin());

// Create the plugin collection (using the KernelPluginFactory to create plugins from objects)
builder.Services.AddSingleton<KernelPluginCollection>((serviceProvider) => 
    [
        KernelPluginFactory.CreateFromObject(serviceProvider.GetRequiredService<LightsPlugin>()),
        KernelPluginFactory.CreateFromObject(serviceProvider.GetRequiredService<SpeakerPlugin>())
    ]
);

// Finally, create the Kernel service with the service provider and plugin collection
builder.Services.AddTransient((serviceProvider)=> {
    KernelPluginCollection pluginCollection = serviceProvider.GetRequiredService<KernelPluginCollection>();

    return new Kernel(serviceProvider, pluginCollection);
});

This looks really more like workaround, requires me to understand Kernel internals. What I would prefer is to simplified it to something like that.

builder.Services.AddScoped<LightsPlugin>();
builder.Services.AddScoped<SpeakerPlugin>();

// Registrer Kernel
builder.Services.AddKernel(options => 
{
    options.Plugins.Add<LightsPlugin>();
    options.Plugins.Add<SpeakerPlugin>();
});

This is the same pattern that people are well familiar from ASP.NET Core for registering eg. ActionFilters.

[rosieks]

After just digging into implementation current approach might be expensive if someone switch from singleton plugins to scoped/transient. Then every time you call CreateFromObject you use reflection to enumerate all methods and check if any is KernelFunction. It would be better if you introduce some kind of metadata of plugin that is separated from exact instance that will be used.

[dmytrostruk]

@rosieks We do have simplified method for DI, which will register Plugins and Kernel as Transient:

semantic-kernel/dotnet/src/SemanticKernel.Abstractions/Services/KernelServiceCollectionExtensions.cs

Lines 18 to 34 in 4d73de4

	public static IKernelBuilder AddKernel(this IServiceCollection services)
	{
	Verify.NotNull(services);
	// Register a KernelPluginCollection to be populated with any IKernelPlugins that have been
	// directly registered in DI. It's transient because the Kernel will store the collection
	// directly, and we don't want two Kernel instances to hold on to the same mutable collection.
	services.AddTransient<KernelPluginCollection>();
	// Register the Kernel as transient. It's mutable and expected to be mutated by consumers,
	// such as via adding event handlers, adding plugins, storing state in its Data collection, etc.
	services.AddTransient<Kernel>();
	// Create and return a builder that can be used for adding services and plugins
	// to the IServiceCollection.
	return new KernelBuilder(services);
	}

I think if you want to use other lifecycles for your Plugins and Kernel, you can do that by registering your components manually, as shown in your example. Let us know if AddKernel method is something you were looking for. Thanks!

[rosieks]

No, it doesn't simplify registration of plugins with respect to lifecycle. AddKernel in current form do not address how to register scoped plugins.

[dmytrostruk]

No, it doesn't simplify registration of plugins with respect to lifecycle. AddKernel in current form do not address how to register scoped plugins.

You can register scoped and singleton plugins and use AddKernel with following syntax, where KernelPlugin is an abstract representation of Plugins in Kernel:

var serviceCollection = new ServiceCollection();

serviceCollection.AddScoped<KernelPlugin>(sp => KernelPluginFactory.CreateFromType<LightsPlugin>(serviceProvider: sp));
serviceCollection.AddSingleton<KernelPlugin>(sp => KernelPluginFactory.CreateFromType<SpeakerPlugin>(serviceProvider: sp));

serviceCollection.AddKernel();

There is also another way with adding plugins using KernelBuilder:

 var collection = new ServiceCollection();

 var kernelBuilder = collection.AddKernel();

 kernelBuilder.Plugins.AddFromType<LightsPlugin>(); // LightsPlugin will be registered as singleton

Maybe we can simplify this syntax even more. But if this syntax is not helpful and something is still missing, the title and description of this issue should be updated to emphasize the problem with registering plugins with different lifecycles etc, because currently it mentions the problem in complex syntax in general, while at the moment there are ways how to use easier syntax, which I shared above.

We will also need to make sure that simpler syntax is presented in documentation and samples.

[rosieks]

serviceCollection.AddScoped<KernelPlugin>(sp => KernelPluginFactory.CreateFromType<LightsPlugin>(serviceProvider: sp));

The option above is much better than the one presented in documentation, though it still has certain flaws:

• It's not documented
• Requires developer the familiar with KernelPlugin and KernelPluginFactory.
• Not sure how costly is CreateFromType method. It use reflection to search for functions and if someone use it in ASP.NET it might be called every request.

I think that having at least such extension method would simplify the whole registration process:

public static IKernelBuilder AddPlugin<T>(this IKernelBuilder builder)
{
    builder.Services.AddTransient(sp => KernelPluginFactory.CreateFromType<T>(serviceProvider: sp));
    return builder;
}

Then developer can just simply call:

services.AddKernel().AddPlugin<LightsPlugin>()

And doesn't care about internal implementation deatils, how Kernel handle plugins.

[markwallace-microsoft]

Thanks for creating the issue @rosieks we have discussed it and plan to proceed as follows:

1. We are going to update our documentation, we'll use this issue to track that work item
2. We cannot introduce any breaking changes to existing methods
3. We are open to a contribution in this area, so please create a PR with your proposal and the team will review