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?

[david-luna]

Hi

IMHO the fact that we have short names in OTEL_NODE_(EN|DIS)ABLED_INSTRUMENTATIONS accidentally added complexity to implement bullet 2 for EDOT but also for the SDK if it wants to have that support in the future.

export OTEL_NODE_ENABLED_INSTRUMENTATIONS=http,mysql,foo

In this line above what exactly id foo? is the full package name or a shorthand for @opentelemetry/instrumentation-foo? with a namespace there would be no doubt but not all packages have a namespace and I see no reason to force authors to do so.

@trentm for that reason I think your proposal of having a full name of the instrumentation is better than considering any other way of shortening the names. What I'm not so sure is about mixing short names with full ones in the same envvar. I think this might make the logic more complex and have to try a couple of imports to get the instrumentation registered. In the foo scenario:

• EDOT has to try to load @opentelemetry/instrumentation-foo
• if fails fallback to load foo
  (order can be inverted)

Having it separately would imply to lookup for another var but load process would be straightforward in both cases.

• load @opentelemetry/instrumentation-foo if foo is listed in OTEL_NODE_(EN|DIS)ABLED_INSTRUMENTATIONS
• load foo if defined in another envvar ELASTIC_OTEL_NODE_APPEND_INSTRUMENTATIONS (we can discuss the name)

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.

I think you're right and I picture it in a similar way. There will be a new envvar pointing to the instrumentations the SDK has to load.

[trentm]

What I'm not so sure is about mixing short names with full ones in the same envvar. I think this might make the logic more complex and have to try a couple of imports to get the instrumentation registered. In the foo scenario:

I'd like to avoid having two separate envvars (OTEL_NODE_(EN|DIS)ABLED_INSTRUMENTATIONS and ELASTIC_OTEL_NODE_APPEND_INSTRUMENTATIONS) for this if we can. However, I agree we definitely don't want to get into "try a couple of imports and fallback to something else if that doesn't work".

Basically, then foo has to be unambiguous. How about this: there is a hardcoded set of supported "shortnames". This set will be the same set supported in upstream auto-instrumentations-node (the names from InstrumentationMap in that pkg). So if a name foo is given:

• If that name is in the hardcoded set, then it refers to @opentelemetry/instrumentation-foo
• otherwise, it refers to foo.

We could limit the support foo names to just the known set of "already require(...)'d at top-level" instrumentations, and stop there. Then the implementation would be easy. This handles (1.).

---

For future work, I keep coming back to the Java mechanism mentioned above: otel.instrumentation.common.default-enabled=false|true and otel.instrumentation.[shortname].enabled=false|true. I think we should think about getting OTel JS Instrumentations to have a name property. This would be the shortname support in configuration. A policy could be defined for the possible rare name collision (e.g. first one wins). A best practice could be defined for the name to use for vendor-specific instrumentations (e.g. there are multiple instr packages out there that instrument openai).

[trentm]

David and I talked: decided for GA just do the simple (1.) case described in the last comment.

For now we will not look at the + syntax or any other syntax for (3.). We will also not consider supporting unknown instrumentations (option 2. above).