OpenTelemetry 1.4.0-rc2 should not depend on OpenTelemetry.Extensions.DependencyInjection

[reyang]

Bug Report

https://www.nuget.org/packages/OpenTelemetry/1.4.0-rc.2#dependencies-body-tab

OpenTelemetry.Extensions.Xyz.dll can depend on OpenTelemetry.dll, the reverse must not happen as it breaks the principle how components are organized.

Consider this as 1.4.0 release blocker.

[CodeBlanch]

History on this...

There is this idea/goal that instrumentation libraries should only need a dependency on API. Most of the instrumentation in the main repo depends on SDK but some only depend on API. It generally comes down to whether or not you need to use the downstream suppression feature which is part of SDK.

If you are authoring some simple instrumentation which only needs to call AddSource or AddMeter than a dependency on API works fine. But if you are authoring some instrumentation which needs options or dependent services, things become more interesting. SDK has support for DI & Options \ IConfiguration and users will expect it to work consistently (assuming) but these libraries are not able to participate in that. Adding required dependencies directly to API didn't get off the ground so instead we added OpenTelemetry.Extensions.DependencyInjection.

What OpenTelemetry.Extensions.DependencyInjection contains is the API to extend OpenTelemetry.Api provided TracerProviderBuilder & MeterProviderBuilder contracts with the DI feature set implemented by the SDK.

Perhaps a name like OpenTelemetry.Api.Extensions would have been more palatable?

[reyang]

There is this idea/goal that instrumentation libraries should only need a dependency on API. Most of the instrumentation in the main repo depends on SDK but some only depend on API. It generally comes down to whether or not you need to use the downstream suppression feature which is part of SDK.
If you are authoring some simple instrumentation which only needs to call AddSource or AddMeter than a dependency on API works fine. But if you are authoring some instrumentation which needs options or dependent services, things become more interesting. SDK has support for DI & Options \ IConfiguration and users will expect it to work consistently (assuming) but these libraries are not able to participate in that. Adding required dependencies directly to API didn't get off the ground so instead we added OpenTelemetry.Extensions.DependencyInjection.

I want us to revisit what we envision - this is my understanding of the final shape which we'd like to see:

• Instrumentation libraries should be implemented by only taking dependencies on .NET Runtime APIs, and that's our recommendation.
• Instrumentation might need to suppress some underlying operations, which can be done by either 1) making the suppression mechanism part of the API 2) following some convention and use context slots
• Instrumentation libraries can decide to do extra things such as supporting configuration via dependency injection, which could mean that these libraries will need to take additional dependencies from Microsoft.Extension.*, or decouple the configuration part into another assembly.

What OpenTelemetry.Extensions.DependencyInjection contains is the API to extend OpenTelemetry.Api provided TracerProviderBuilder & MeterProviderBuilder contracts with the DI feature set implemented by the SDK.

Perhaps a name like OpenTelemetry.Api.Extensions would have been more palatable?

Or maybe OpenTelemetry.Api.Extensions.DependencyInjection, depending on how we think about the scope of the package (e.g. I don't expect OpenTelemetry.Api.Extension to cover both DI and EntityFramework).

Even if we rename it to something like OpenTelemetry.Api.Extensions.DependencyInjection or OpenTelemetry.Api.Extensions, it doesn't seem to solve the problem here IMHO - I feel OpenTelemetry.dll should not take dependencies on any extensions, it should be the reverse (the extensions take dependencies on the core packages).

[martinjt]

The issue we have with DI is that it's so engrained into Modern .NET that it's expected. Anything that doesn't feel out of the box to support DI feels wrong to developers.

I believe we could re-architecture on a basis like the above, but right now, I think what we have is the right balance for the 1.4 release, we've been discussing the various trade-offs for the last 3 months on the SIGs and in chats and we're happy with those trade-offs.

I'm not a maintainer, I get that, but I have been on most of the SIGs talking through these different API structures with the team. It wasn't thrown in on a whim like the Logging library was, it was considered. Specifically taking into account the user base, how they use it, the feedback through Github.

Let's start a thread for how you would architect for a 2.x release and start work doing that while we get the rest of the libraries stable and away from RCs so we can get some confidence with the .NET development community.

We need to remember that this is a community project, not a Microsoft BCL library. Developers have the option to choose vendor specific stuff, which is what's happening right now because OpenTelemetry-dotnet isn't considered stable and they're having issues with dependencies (like Diagnostic Source 7.0.0 or the rc nightmare of the instrumentation libraries we've discussed). Adoption is what we're trying to do here, to move people away from vendored libraries and we're failing at that right now. Getting this release out, as is, is going to fix a lot of those issues. Reverting functionality that has been in an RC for 2 months is not going to give anyone any confidence.

I'm at the coal face of this, working with engineers and evangelising otel wherever I can, and I constantly get "Yeah but it's in beta right, not released and stable". The worst is when I get "yeah, but it's way more complicated than using a vendor library". I'm also at the other end in my other job working with a vendor to support it, and we get the same things, asking for vendored SDKs because of the releases.

What do we need to get past this and into a 2.x cycle where we can tackle the bigger rewrite?

[martinjt]

I would suggest that we hear from the other approvers and maintainers on this issue as at the moment it feels like this is more of an opinion than something that should be blocking.

Also, given that this will delay stability of the .NET packages as a whole, I think it should be parked and fixed in the next release if we accept that it is something we should be changing.

@pellared @vishweshbankwar @alanwest

[pellared]

For my side it does not like a blocker. However, I would really welcome to have it "fixed" in future. In general, I would like us try to avoid using dependencies. If possible I would consider using "vendoring" (copying the code) or reflection if applicable 😉

[cijothomas]

We are proceeding with #4125. The new package is an extension to the API, and is meant to be consumed by Instrumentation Library authors only, who want to offer nice DI based config, but without OTel SDK dependency.