supporting non-otel instrs in OTEL_NODE_{DIS,EN}ABLED_INSTRUMENTATIONS

[trentm]

intro

OTEL_NODE_DISABLED_INSTRUMENTATIONS and OTEL_NODE_ENABLED_INSTRUMENTATIONS are envvars supported by @opentelemetry/auto-instrumentations-node. They are also supported by EDOT Node.js. E.g.:

export OTEL_NODE_DISABLED_INSTRUMENTATIONS=http,dns,net
node my-instrumented-app.js

Currently the strings are mapped to @opentelemetry/instrumentation-SHORTNAME and checked against a hardcode list of known instrumentations. That list provides the mapping from an instr package (import path) to the export name of the Instrumentation class to use. E.g.:

const InstrumentationMap = {
  '@opentelemetry/instrumentation-amqplib': AmqplibInstrumentation,
  '@opentelemetry/instrumentation-aws-lambda': AwsLambdaInstrumentation,
  '@opentelemetry/instrumentation-aws-sdk': AwsInstrumentation,
  '@opentelemetry/instrumentation-bunyan': BunyanInstrumentation,
...

Some things that might be nice to add to this:

1. Support for an instrumentation that is known, but does not match @opentelemetry/instrumentation-SHORTNAME, e.g. @elastic/opentelemetry-instrumentation-openai.
2. Support for an unknown instrumentations, i.e. they aren't in the mapping of instrs that the SDK knows about, but are installed and importable.
3. Support for saying "I want to enable the defaults plus these few instrumentations." This last one is recently an issue with auto-instrumentations-node because the fs instrumentation was dropped from the default set. So if a user wants to get it back, they need to list 'fs' and every other instrumentation.

This issue proposes to handle just (1.) for now. However, the discussion will explore ideas for all three, because I don't want to propose a design that limits doing (2.) and (3.) later.

proposal

Also allow the full entry-point in the envvar, e.g.:

OTEL_NODE_ENABLED_INSTRUMENTATIONS=http,express,@opentelemetry/instrumentation-fastify,@elastic/opentelemetry-instumentation-openai

This is long-winded, but unambiguous.

I didn't want to use openai as the shortname used by Elastic's distro, because if upstream OTel JS contrib gets an instrumentation-openai, then the openai shortname would be ambiguous.

also considered: @NAMESPACE/SHORTNAME

I also considered support a shortform for non-@opentelemetry instrumentations: map @NAMESPACE/SHORTNAME to @NAMESPACE/opentelemetry-instrumentation-SHORTNAME. This would allow:

OTEL_NODE_ENABLED_INSTRUMENTATIONS=http,express,@opentelemetry/fastify,@elastic/openai

An assumed-common naming pattern for non-OTel-provided instrumentation packages is: @NAMESPACE/opentelemetry-instrumentation-SHORTNAME, e.g.: (npmjs query: https://www.npmjs.com/search?q=opentelemetry-instrumentation&page=2&perPage=20)

      @azure/opentelemetry-instrumentation-azure-sdk
    @elastic/opentelemetry-instumentation-openai
@heliosphere/opentelemetry-instrumentation-jest
    @imqueue/opentelemetry-instrumentation-imqueue
  @totalsoft/opentelemetry-instrumentation-ws
  @appsignal/opentelemetry-instrumentation-bullmq

Some counter examples:

opentelemetry-instrumentation-mongodb2

opentelemetry-instrumentation-kafkajs
   This and other aspect-io ones are deprecated.

@odigos/instrumentation-kafkajs
   A newer one from Amir that doesn't match the pattern.

I think the naming pattern is fairly common, but I don't feel the gain of the slightly shorter name (@NAMESPACE/SHORTNAME) is worth having two ways to identify a package, and a way that isn't completely self-explanatory.

digression on supporting unknown instrumentations

This is bullet (2.) above. The idea here is that I can say:

OTEL_NODE_ENABLED_INSTRUMENTATIONS=@somens/opentelemetry-instrumentation-foobar

and the SDK will attempt to import @somens/opentelemetry-instrumentation-foobar at runtime; and further try to determine the exported name that is the appropriate Instrumentation class to use.

Problems here:

• What export name to use? This would be a heuristic or could be part of the string, e.g.: @somens/opentelemetry-instrumentation-foobar.FooBarInstrumentation. This could be workable, but is starting to get overloaded.
• Dynamic loading of packages. This means it is async, in general (for ESM), which might be a design concern. It is also a subtle addition to the existing config envvar: saying OTEL_NODE_ENABLED_INSTRUMENTATIONS=rimraf will dynamically load the rimraf package? That could be surprising.

Ultimately I think this feature of "adding some external-to-the-SDK instrumentations" to the OTel setup is the job of a new OTel JS feature something like OTel Java's extensions: https://opentelemetry.io/docs/zero-code/java/agent/extensions/ That's a separate discussion.

OTel Java digression

Here are OTel Java's config options for selecting instrumentations: https://opentelemetry.io/docs/zero-code/java/agent/disable/#enable-only-specific-instrumentation

• otel.instrumentation.common.default-enabled
• otel.instrumentation.[name].enabled

That works for them because each built-in instrumentation has a [name] that works as a system property or envvar name. OTel JS doesn't. Setting OTEL_INSTRUMENTATION_@ELASTIC_OPENTELEMETRY_INSTRUMENTATION_OPENAI_ENABLED=true doesn't really work.

Digression on (3.)

OTEL_NODE_ENABLED_INSTRUMENTATIONS=+fs

I wonder about using a + prefix to allow saying "keep the default set, just add this one". I think that could work, but I'm not exploring that further for this issue.

[trentm]

@david-luna Thoughts?