Rename Ruby Modules

[arielvalentin]

TL;DR

Rename existing OpenTelemetry libraries to a shorter namespace OTel and remove redundant submodules in instrumentation gems. This will reduce overhead for the application being instrumented as well as reduce friction for users and maintainers who use the gems.

Background

Our end users at GitHub find it difficult to leverage Instrumentation Scope Names in our backends, navigate the source code, and review backtraces due to the verbose nature of our module namespaces.

I have found that this has resulted in our users being less likely to contribute fixes upstream and requires our o11y team to add transformations to rename verbose instrumentation scopes in order to make it simpler for our users to query for attributes in our backend.

Proposal

The existing OpenTelemetry module names are quite verbose and often are difficult to navigate when attempting to reference in code or configurations.

DSL Changes

The example below demonstrates taken from our documentation¹ demonstrates the minimal example of how to configure the SDK with selective instrumentations:

# install all compatible instrumentation with default configuration
OpenTelemetry::SDK.configure do |c|
  c.use 'OpenTelemetry::Instrumentation::Sinatra'
  c.use 'OpenTelemetry::Instrumentation::Faraday', { opt: 'value' }
end

Using terse namespaces will make the code succinct without losing any features in domain specific language API:
 

# install all compatible instrumentation with default configuration
OTel::SDK.configure do |c|
  c.instrument 'sinatra'
  c.instrument 'faraday', { opt: 'value' }
end

Something else to consider is that the DSL is not accounting for is the configurations for metrics and logs. Proposing this change may also mean making the DSL more verbose to account for what we are configuring in the SDK

Taking into account that trace configurations it may resemble the DD Trace DSL a bit more closely:

# install all compatible instrumentation with default configuration
OTel::SDK.configure do |c|
  c.traces.instrument 'sinatra'
  c.traces.instrument 'faraday', { opt: 'value' }
end

 

Instrumentation Namespaces

The current namespaces for instrumentations have an additional submodule without providing any additional benefit. In cases where one has to refer to an instance of an instrumentation the code may resemble the following:

 OpenTelemetry::Instrumentation::Faraday::Instrumentation.instance.installed?

Compared to when using more succinct namespaces, one could then interrogate the instrumentation singleton like this:

# Option 1 keeping Instrumentation namespace
 OTel::Intrumentation::Faraday.instance.installed?

# Option 2 create a traces specific namespace
OTel::Traces::Faraday.instance.installed?

# Option 3 creates a contrib specific namespace
OTel::Contrib::Faraday.instance.installed?

# Option 4 Omits any intermediate namespaces
OTel::Faraday.instance.installed?

Instrumentation Scope

Choosing a succinct namespace is related to a conversation we had during the SIG to renaming Instrumentation Scope names, which default to the module name.

During that conversation I proposed reducing the Instrumentation Scope name to a more minimal name otel.<instrumented library name>, e.g. instead of having to search for or use verbose names when searching for traces like this instrumentation.scope: OpenTelemetry::Instrumentation::Faraday one could use instrumentation.scope: otel.faraday instead.

Thought it will have a minimal impact on performance, this change would reduce the size of an exporter batch by a minimum of 27 bytes per instrumentation (assuming rate/second it would be around ~2.33 MB per day per instrumentation).

Migration plan

End users will no doubt experience disruption when working with new namespaces.

We may be able to mitigate this by creating existing constants that reference the renamed less verbose namespaces:

module OTel
  # new code is relocated here
end

OpenTelemetry = OTel

This breaks down in cases where we remove redundant namespaces in instrumentations:

module OTel
  module Contrib
    class ActiveRecord < Base
      # code is relocated here
    end
  end
end

OpenTelemetry::Instrumentation = OTel::Contrib

OpenTelemetry::Instrumentation::ActiveRecord::Instrumentation.instance.installed? # results in NameError

For this reason I would like to consider this a breaking change that result in breaking changes and require our end users to adopt new namespaces and gems that are not guaranteed to be interoperable with each other.
 

Footnotes

1. https://opentelemetry.io/docs/instrumentation/ruby/automatic/ ↩

[stevenharman]

As an end user and contributor, I often get frustrated and/or lost while navigating the source while trying to track down unexpected behavior, improve/update tracing semantics, etc… I also feel the occasional friction of the verbose instrumentation.scope naming.

As an end user, I also foresee the pain in migrating our extensive usage of these libs.

I understand the problems and the tradeoffs we'd be making in removing the extra, unnecessary module. And in changing the organizational structure to make room for the coming OTel Metrics and Logging.

I've not formed an opinion on the "best" way to organize (though I think either OTel::Traces::Faraday or OTel::Contrib::Faraday make most sense), but I do think it's worth pursing this change.

[fbogsany]

I can honestly say that as the company with by far the greatest experience using and contributing to OpenTelemetry Ruby, we have never found the length of the OpenTelemetry module name to be a pain point. If it is painful for you, I suggest using an alias.

I don't believe the module name is a pain point for instrumentation either, however I can see that the instrumentation scope name could reasonably be shortened. Arguably, that change might be more difficult to navigate for some, since it affects backends (e.g. saved searches, dashboards, etc.), but I don't think it'd be a problem for our users. The instrumentation_name can be explicitly set by instrumentation or infer_name can be changed if we decide to rename them all. Those alternatives are both instrumentation concerns, so probably best discussed in https://github.com/open-telemetry/opentelemetry-ruby-contrib.

Instrumentation module nesting can also be discussed in https://github.com/open-telemetry/opentelemetry-ruby-contrib. I can see the value in removing the last level of nesting (::Instrumentation). Shortening the other ::Instrumentation:: to ::Contrib:: is less clear, since the -contrib repo contains more than just instrumentation and it is useful to distinguish exporters (none yet), resource detectors, propagators and instrumentation.

[arielvalentin]

No problem. I can split this RFC into separate discussions and have a focused discussion on Instrumentation Scope naming in the contrib repo.

Would you prefer I close this topic and open a separate discussions? I.e.

1. Renaming OpenTelemetry => OTel
2. DSL changes

[fbogsany]

Sorry, I skimmed the DSL changes on first read. Those are worth discussing here, since they relate to the SDK configuration.

My considered opinion is that renaming OpenTelemetry => OTel is not going to happen for the API and SDK. For auto-instrumentation in -contrib, I would argue pretty strongly that the community would be better served by working with maintainers of auto-instrumented gems to natively instrument their code with OpenTelemetry. That would have a much larger positive impact on maintainability of -contrib than renaming the top-level module.

[fbogsany]

As a point of comparison, here's an instrumentation scope name from OpenTelemetry Go:

const instrumentationName = "go.opentelemetry.io/contrib/instrumentation/gopkg.in/macaron.v1/otelmacaron"

[arielvalentin]

Yes those are quite unpleasant as well.