Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(otel-node): restrict instrumentations to a list of known ones #625

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

david-luna
Copy link
Member

As discussed in a meeting EDOT should support only a list of known instrumentations. The decision was documented in #481 (comment)

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.).

That's exactly what this implementation does therefore the envvars OTEL_NODE_{DIS,EN}ABLE_INSTRUMENTATIONS refers only to the instrumentations included in EDOT. In sumary:

  • The envvars refer to otel instrumentations via its short name
  • The envars refer to the rest of included instrumentations (openai for now) via its full name

Closes #481

@david-luna david-luna changed the title feat(otel-node): restrict instrumentations to a list of know ones feat(otel-node): restrict instrumentations to a list of known ones Feb 24, 2025
@david-luna
Copy link
Member Author

mongoose tests failed twice: https://github.com/elastic/elastic-otel-node/actions/runs/13501299951/job/37727580220?pr=625

need to check if something works different in that specific node version (20.6.0)

@david-luna david-luna requested a review from trentm February 24, 2025 18:05
Comment on lines +172 to +174
const OTEL_INSTRUMENTATION_PREFIX = '@opentelemetry/instrumentation-';
const OTEL_INSTR_SHORT_NAMES = new Set();
const NON_OTEL_INSTR_NAMES = new Set();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Thoughts on having these var names be otelInstrPrefix, otelInstrShortNames, etc. I.e. not the "SHOUTY_NAMES"? OTEL_INSTRUMENTATION_PREFIX is a constant, so I can see wanting to USE_THIS_STYLE, hence this is just a nit.

A small nice thing with not using OTEL_... is that we can (try to) limit any grep for OTEL_ to be about environment variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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