align opentelemetry instrumentation naming with specification

[toumorokoshi]

Hi!

I recently worked with opentelemetry-specification to align on a standardized naming for instrumentations across languages. Here's the final result:

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/overview.md#instrumentation-libraries

And the PR where it was discussed:

open-telemetry/opentelemetry-specification#539

I think if ruby followed the specification as written, it would have packages renamed from "adapter" to "instrumentation":

https://github.com/open-telemetry/opentelemetry-ruby/tree/master/adapters

I'm filing this issue for awareness and discussion, and happy to collect and raise questions to the spec or technical committee.

[fbogsany]

This requires a little more work than just a directory rename. We’ll need to rename all the opentelemetry-adapters-* gems, change the name spacing OpenTelemetry::Adapters::* in all gems, and rename/refactor OpenTelemetry::Instrumentation::Adapter: https://github.com/open-telemetry/opentelemetry-ruby/blob/master/api/lib/opentelemetry/instrumentation/adapter.rb.

The latter is particularly messy. The obvious rename is:

• opentelemetry-adapters-* -> opentelemetry-instrumentation-* for gems
• OpenTelemetry::Adapters::* -> OpenTelemetry::Instrumentation::* for namespacing

but we have two entities in OpenTelemetry::Instrumentation already: Adapter and Registry. I don’t like the stuttering of OpenTelemetry::Instrumentation::Instrumentation. @mwear would it be weird to rename/hoist OpenTelemetry::Instrumentation::Adapter to OpenTelemetry::Instrumentation and treat it both as a class (for inheritance by ... instrumentation) and as a module for the Registry and said instrumentation?

The spec change suggests Libraries as a term that could be used instead of Adapters, but that’s arguably more confusing in Ruby, where we talk about Gems rather than Libraries and calling the concept Gem instead is doubly confusing because an instrumentation gem could contain multiple (cough) adapters. FWIW, this distinction was behind the original term Instrumentation Adapters 😄.

In this context, it’s worth asking whether we need to elide the term Adapter or simply perform the gem and Adapters -> Instrumentation module rename, and retain OpenTelemetry::Instrumentation::Adapter and OpenTelemetry::Instrumentation::Registry?

[fbogsany]

An alternative to the name OpenTelemetry::Instrumentation::Adapter might be OpenTelemetry::Instrumentation::Installer, since that’s really the purpose of the class — to detect and install instrumentation.

[toumorokoshi]

Got it, thanks! As a my main intent is to surface the existence of this standard. At least personally I see that there's a strong tradeoff between the benefits of this change and the cost required to do so, and won't pursue evangelizing this work much more than just filing an issue.

Having an adapter (instrumentation) registry is a very cool idea. I'm not sure if other languages would benefit, but I wonder if something like that should be in the spec.

I believe something like a registry exists in other languages in the auto-instrumentation side of things. So potentially there's reconciliation there.

On the python side of things we have all instrumentations extending from a BaseInstrumentor, currently located in the auto-instrumentation module:

https://github.com/open-telemetry/opentelemetry-python/blob/master/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/instrumentor.py#L26

Not weighing in on Ruby per se, but I think 'opentelemetry.instrumentation.adapter' and 'opentelemetry.instrumentation.register' would be modules that Python would have no problem adopting. Although I wonder about the ramifications of mixing instrumentations with broader sdk objects.

The spec change suggests Libraries as a term that could be used instead of Adapters,

The culmination of the conversations I was involved in landed on "Instrumentation Libraries", although maybe that was implicit in your statement.

FWIW, this distinction was behind the original term Instrumentation Adapters

That was indeed another option, although it looks like the intention from the start was to not include "adapters" in the name, instead naming the namespace "instrumentation":

open-telemetry/opentelemetry-specification#30 (comment)

Also it seems at some point things diverged quite a bit from the original suggestion, see:

open-telemetry/opentelemetry-specification#539 (comment)

[fbogsany]

As a my main intent is to surface the existence of this standard.

Understood. Most of my remarks are “thinking out loud” about how to implement this, for the benefit of @mwear. The Registry is part of our implementation of auto-instrumentation. In this context, the rename from Adapter to Installer makes the most sense to me, though I’ll see if anyone else wants to bike shed this. I’ll start a PR for the changes described in my earlier comments.