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

[DependencyInjection] IServiceProvider GetService(s) behavior differences #107153

Closed
CodeBlanch opened this issue Aug 29, 2024 · 5 comments
Closed

Comments

@CodeBlanch
Copy link
Contributor

Ran into a couple of issues in OpenTelemetry .NET: open-telemetry/opentelemetry-dotnet#5537 & open-telemetry/opentelemetry-dotnet#5803

It boils down to behavior differences between M.E.DI and other container implementations.

Let's say I have some service:

internal sealed class SomeService {}

When using Microsoft.Extensions.DependencyInjection if you ask the IServiceProvider for MyService it won't return anything if MyService wasn't explicitly added to the IServiceCollection:

var mseServices = new ServiceCollection();

using var mseServiceProvider = ServiceCollectionContainerBuilderExtensions.BuildServiceProvider(mseServices);
var mseFoundServices = mseServiceProvider.GetServices<SomeService>();
if (mseFoundServices.Any())
{
    throw new NotSupportedException("Found SomeService registrations in MSE"); // Not thrown when executed
}

This is the behavior we rely on in OpenTelemetry .NET.

But it turns out other containers have different behavior.

This code using Unity.Microsoft.DependencyInjection does throw:

var mseServices = new ServiceCollection();

var unityServiceProvider = ServiceProviderExtensions.BuildServiceProvider(mseServices);
var unityFoundServices = unityServiceProvider.GetServices<SomeService>();
if (unityFoundServices.Any())
{
    throw new NotSupportedException("Found SomeService registrations in Unity"); // Thrown with executed
}

The reason for that is some containers (reported for Unity & Grace so far) will automatically create services even if they weren't registered into the IServiceCollection.

Questions...

  • Is there a better API to use to ask an IServiceProvider if some service was registered?

  • Could we add such an API if there is no way to do it?

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

@eerhardt
Copy link
Member

Check out #54047.

/// <summary>
/// Optional service used to determine if the specified type is available from the <see cref="IServiceProvider"/>.
/// </summary>
public interface IServiceProviderIsService
{
/// <summary>
/// Determines if the specified service type is available from the <see cref="IServiceProvider"/>.
/// </summary>
/// <param name="serviceType">An object that specifies the type of service object to test.</param>
/// <returns>true if the specified service is a available, false if it is not.</returns>
bool IsService(Type serviceType);

@julealgon
Copy link

@CodeBlanch have you guys considered a different approach to the problem (of detecting local exporter vs global one) that doesn't rely on knowing what has been registered in the container to work?

I feel like, in general, explicitly trying to check what is registered and what is not in the container is a code smell.

(and yes, @eerhardt suggestion above should work, but I figured I'd drop this here anyways...).

@steveharter
Copy link
Member

Is there a better API to use to ask an IServiceProvider if some service was registered?
Could we add such an API if there is no way to do it?

Closing per workaround of using IServiceProviderIsService.

Also, using the singular GetRequiredService() will throw if the service is not found - would adding a plural version GetRequiredServices() help with your scenario?

@steveharter steveharter closed this as not planned Won't fix, can't repro, duplicate, stale Sep 3, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Sep 3, 2024
@CodeBlanch
Copy link
Contributor Author

Doesn't seem like IServiceProviderIsService is implemented in Unity so it isn't going to be much use. May be there in Grace. Seems to be in the source searching GitHub but didn't verify in the NuGet bits. Didn't check other containers. Didn't really bother because it would need to be ubiquitous to be useful. Let's say it gets added to Unity. Doubtful, but if it did, what do we do with older versions where it isn't there? The linked PR says that the behavior when IServiceProviderIsService is not implemented is undefined 🤣

My workaround was to not allow my service to be instantiated by giving it a private ctor. Works fine (tested with Unity & Grace) but the problem is it won't catch potential future mistakes with other services which may be introduced.

We'll just have to live with it I guess.

Also, using the singular GetRequiredService() will throw if the service is not found - would adding a plural version GetRequiredServices() help with your scenario?

Not for our scenario. I'm calling GetServices<MyService>() to retrieve what was registered. Empty/nothing registered is perfectly valid result which is why I'm not calling the "required" API.

More details. We have two APIs. One is called "UseOtlpExporter" and the other is called "AddOtlpExpoter". We have rules that say for a given IServiceCollection you may only call "UseOtlpExporter" once and you can't call "AddOtlpExpoter" if you call "UseOtlpExporter". How we implement that (currently) is by dropping a service into IServiceCollection anytime "UseOtlpExporter" is called so we can detect what was performed when the IServiceProvider is available and throw if needed.

@julealgon

have you guys considered a different approach to the problem (of detecting local exporter vs global one) that doesn't rely on knowing what has been registered in the container to work?

You have an idea how to do it? Happy to explore alternatives. Don't say statics though 🤣 Because it is perfectly valid to have multiple pipelines in the same process. The IServiceCollection \ IServiceProvider mechanism matches the model pretty well IMO if only there was a ubiquitous way to check if something was registered or not 😄

@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants