Reconsider ResourceDetectors namespaces and usages

[julealgon]

Issue that does not fit into other categories

What are you trying to achieve?

Each resource detector package today has a unique namespace. This results in substantial noise when including multiple detectors in a project, a situation which is fairly common. 

Here is an example of a recent change on one of our projects:

This also goes against the design of other components such as instrumentations and exporters, which follow the main namespaces of their respective metric type. Eg:

opentelemetry-dotnet-contrib/src/OpenTelemetry.Instrumentation.AspNet/MeterProviderBuilderExtensions.cs

Lines 8 to 20 in 01c2257

	namespace OpenTelemetry.Metrics;
	/// <summary>
	/// Extension methods to simplify registering of ASP.NET request instrumentation.
	/// </summary>
	public static class MeterProviderBuilderExtensions
	{
	/// <summary>
	/// Enables the incoming requests automatic data collection for ASP.NET.
	/// </summary>
	/// <param name="builder"><see cref="MeterProviderBuilder"/> being configured.</param>
	/// <returns>The instance of <see cref="MeterProviderBuilder"/> to chain the calls.</returns>
	public static MeterProviderBuilder AddAspNetInstrumentation(this MeterProviderBuilder builder) =>

The main difference between these 2 is that detectors don't come with custom extension methods on ResourceBuilder, while other components are usually added via extensions on TracerProviderBuilder and MeterProviderBuilder.

What do you expect to see?

I'd like to see one of 3 approaches here:

1. Have all OpenTelemetry.ResourceDetectors.* packages have a root namespace of OpenTelemetry.ResourceDetectors
2. Have all OpenTelemetry.ResourceDetectors.* packages have a root namespace of OpenTelemetry.Resources
3. Have all OpenTelemetry.ResourceDetectors.* packages provide .Add{detector_name}Detector extension methods on top of ResourceBuilder and targeting namespace OpenTelemetry.Resources

From a consistency standpoint, I believe 3 would be the best as it stands today. After that, I like 2 as it doesn't introduce a new namespace, and then 3 as it at least unifies all of them in a single namespace.

The caveat with 2 and 1 is that they would potentially also apply to all other projects in a similar way, so it would make sense to apply them more broadly than just for Resource Detector packages.

Additionally, opting for 2 could prompt a different naming scheme altogether which I think would be more fitting, from "ResourceDetector" to just "Resources":

• OpenTelemetry.ResourceDetectors.Host -> OpenTelemetry.Resources.Host

Additional context.

I believe the current approach is extremely noisy and won't scale well as even more detectors are introduced. Additionally, even without regard to the noise issue, I dislike very granular namespaces like this and am fond of the pattern of "child" assemblies NOT including the last segment of the assembly name in the final namespace. This is a common approach in many libraries, for example
Microsoft.Extensions.DependencyInjection vs Microsoft.Extensions.DependencyInjection.Abstractions, where the "Abstractions" in the assembly name does not participate in the final namespace of the classes inside:

[matt-hensley]

+1 for extensions methods against ResourceBuilder like we see with instrumentation and other components. The style is pretty natural and helps with ergonomics IMO.

[cijothomas]

+1 for extensions methods against ResourceBuilder like we see with instrumentation and other components. The style is pretty natural and helps with ergonomics IMO.

+1
None of the ResourceDetectors are stable released, so this is still doable.

[julealgon]

None of the ResourceDetectors are stable released, so this is still doable.

@cijothomas what about the general namespaces, do you think there is still a possibility to streamline them? Even without the extension method approach, I believe having a "ResourceDetectors" namespace is a mistake, and they should all be part of the same "Resources" namespace (and package names should also change accordingly).

[cijothomas]

what about the general namespaces, do you think there is still a possibility to streamline them?

For packages already shipped as stable, we cannot make breaking changes! (without a major version bump)
We realized some mistakes like this too late : open-telemetry/opentelemetry-dotnet#2558

(and package names should also change accordingly).

I am neutral on changing package names, but fully support your proposal 3, copy-pasted below. (with the extension methods, we can make the detector class itself non-public.)

Have all OpenTelemetry.ResourceDetectors.* packages provide .Add{detector_name}Detector extension methods on top of ResourceBuilder and targeting namespace OpenTelemetry.Resources

[CodeBlanch]

2. Have all OpenTelemetry.ResourceDetectors.* packages have a root namespace of OpenTelemetry.Resources
3. Have all OpenTelemetry.ResourceDetectors.* packages provide .Add{detector_name}Detector extension methods on top of ResourceBuilder and targeting namespace OpenTelemetry.Resources

+1 for both of these. I'm kind of surprised we aren't already shipping these helper extensions 🤣

Would we also rename packages? For example OpenTelemetry.ResourceDetectors.Host -> OpenTelemetry.Resource.Host?

[julealgon]

@CodeBlanch

3. Have all OpenTelemetry.ResourceDetectors.* packages provide .Add{detector_name}Detector extension methods on top of ResourceBuilder and targeting namespace OpenTelemetry.Resources

...I'm kind of surprised we aren't already shipping these helper extensions 🤣

To be fair, I am as well considering basically everything else in the OTEL packages (and in the rest of the Microsoft.Extensions-related stuff in the .NET ecosystem) follow that approach. These stick out quite a bit today.

Would we also rename packages? For example OpenTelemetry.ResourceDetectors.Host -> OpenTelemetry.Resource.Host?

I know I'm not a decision maker for this, but if you could rename the packages now, I think it would be beneficial for discoverability and consistency in the long term. And considering how early we are in terms of OTEL support overall (especially with the detectors), this feels like a good time to make these breaking changes. The more you wait, the stronger the impact will be to the point of not being feasible anymore.

Also, keep in mind the namespace is Resources, not Resource.

[CodeBlanch]

Also, keep in mind the namespace is Resources, not Resource.

Agree the namespace should be Resources (plural). I went with singular form for the package name because that seems like what we are doing with other things (eg OpenTelemetry.Exporter.*) but TBH plural does feel more natural there too (for me).

FYI @alanwest started an effort a while back to clean up packages: open-telemetry/opentelemetry-dotnet#3469

Seems like it has stalled out though.

What we need is a maintainer who has some passion for this clean up to pick it up and run with it. Personally I don't have the bandwidth at the moment. @Kielek @vishweshbankwar?

[Kielek]

Putting on my extended TODO list. First I would like to migrate library instrumentation from main repo here.

@julealgon, I will be happy to review and merge PRs (later make a releases) if you have time to implement proposal. It is probably the fastest path to see the results.

[matt-hensley]

Going to pick up a few of these as I have time. Reasonably straightforward after applying the changes to #1691.