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

NodeJS auto-instrumentation: Allow disabling specific instrumentations with environment variables #1672

Closed
FredrikAugust opened this issue Sep 7, 2023 · 10 comments

Comments

@FredrikAugust
Copy link

FredrikAugust commented Sep 7, 2023

Describe the solution you'd like to see

We would like to keep all instrumentations active except for redis-4. This is something that is possible with the Java SDK, but currently not with the JavaScript SDK, using the OTEL_INSTRUMENTATION_[NAME]_ENABLED environment variables.

Something like this is possible for the resource detectors so a solution akin to that would be amazing.

Describe alternatives you've considered

As we're using the Kubernetes OTEL Agent Operator we are AFAIK limited to environment variables in terms of configuration of the autoinstrumentation so the only other solution I've thought of is disabling the instrumentation all-together.

Additional context

I'll happily contribute to this if you'd like:)

@FredrikAugust
Copy link
Author

Alright, we just confirmed that disabling the OTEL instrumentation no longer produces the error described above. Should I create a separate issue for that as it's likely a redis issue?

@rpesk
Copy link

rpesk commented Oct 6, 2023

I second this. It's really annoying having to implement manual configuration file for tracing just to disable/enable single instrumentation. Implementation for this should not be complex at all, as @FredrikAugust mentioned, it's already done for resource detectors.

Copy link
Contributor

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Dec 11, 2023
@FredrikAugust
Copy link
Author

I don't think this would be a lot of work, and would allow people to control what instrumentations are being used so I would love to see this implemented!

@github-actions github-actions bot removed the stale label Dec 18, 2023
@hamin
Copy link

hamin commented Dec 21, 2023

Can we please get this in?

@atsu85
Copy link
Contributor

atsu85 commented Feb 21, 2024

My use-case

I need this feature, because I can't disable the integrations from the code and application startup is a lot slower with all integrations enabled (specially if CPU is limited).

I can't disable from code, because I'm using k8s opentelemetry operator, that loads all instrumentations, so applications that use it can't customize it.

Alternative solutions

One option for me would be to solve the issue in opentelemetry-operator, but i think it would be bad idea:

  • it wouldn't help solving this issue for developers that use @opentelemetry/auto-instrumentations-node directly instead of k8s opentelemetry operator.
  • the env-variable naming convention that might be adopted by opentelemetry-operator might be adopted in this project, and it might cause confusion later

My preferred solution

I'd prefer it if this library also supported disabling instrumentations via environment variables (currently it only supports disabling via inputConfigs object passed to getNodeAutoInstrumentations function).

Let's get ready for the PR - environment variable naming conventions

If we could agree upon a naming convention, i could send a PR to implement it. For example @TylerHelmuth mentioned that Java, Python and .Net all use environment variables for that purpose, but their naming conventions are different.
For this library I'm considering one of the following approaches

Single environment variable like used for configuring node resource detectors

One option would be to have single variable that lists enabled instrumentations (like configuring resource detectors via OTEL_NODE_RESOURCE_DETECTORS):

  • OTEL_NODE_INSTRUMENTATIONS=http,aws-sdk,nestjs-core

Separate environment variables, like used in Java, Python and .Net SDKs

  • format: OTEL_NODE_INSTRUMENTATION_[NAME]_ENABLED=false
  • exampleOTEL_NODE_INSTRUMENTATION_AWS_SDK_ENABLED=false to disable @opentelemetry/instrumentation-aws-sdk

where NAME is a mapping from npm package name

like @opentelemetry/instrumentation-aws-sdk -> AWS_SDK, because @ isn't valid character in environment variable, and all default instrumentations have @opentelemetry/instrumentation- prefix

Then it would be possible to derive the name of the environment variable to check based on name of the package to enable/disable.

Related issues:

@f3l1x98
Copy link

f3l1x98 commented Feb 22, 2024

@atsu85 That is quite a coincidence because I have started looking into adding this feature a few days ago and have started with the implementation today.
This is my current implementation and I would have created a PR tomorrow.

@hamin
Copy link

hamin commented Feb 22, 2024

@atsu85 @f3l1x98 yes please! Let's please get this in!

@atsu85
Copy link
Contributor

atsu85 commented Feb 23, 2024

@f3l1x98,

and have started with the implementation today.

sorry, I didn't notice your comment before i already created the PR (includes test and documentation).

@hamin and @f3l1x98, please have a look!

@atsu85
Copy link
Contributor

atsu85 commented Feb 23, 2024

FYI: i added auto-instrumentation slowness investigation results summary to my PR - perhaps someone stumbling upon this issue might find it useful as well to understand the startup slowness better

atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Feb 26, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Feb 26, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Feb 27, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Feb 27, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Feb 28, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Feb 28, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Feb 28, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Feb 28, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Mar 1, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Mar 1, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Mar 5, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Mar 5, 2024
atsu85 added a commit to atsu85/opentelemetry-js-contrib that referenced this issue Mar 6, 2024
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

No branches or pull requests

5 participants