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

Allow disabling nodejs auto-instrumentations via environment variables #2622

Closed
mderazon opened this issue Feb 13, 2024 · 3 comments · Fixed by #2745 · May be fixed by open-telemetry/opentelemetry-js-contrib#2174
Closed
Labels

Comments

@mderazon
Copy link

mderazon commented Feb 13, 2024

Hi. I would like to disable an instrumentation. Seems like @opentelemetry/instrumentation-fs is causing a lot of unnecessary traces and is recommended to be disabled:
open-telemetry/opentelemetry-js-contrib#1344 (comment)

It's possible to disable it via node auto instrumentation:
open-telemetry/opentelemetry.io#2988 (review)

https://github.com/open-telemetry/opentelemetry.io/blob/16328c6c00bb5c163f200b22ef90b881a80a86d1/content/en/docs/languages/js/libraries.md?plain=1#L145-L154

But I am not sure how to do it through otel operator ?

Originally posted by @mderazon in #1283 (comment)

@mderazon mderazon changed the title Hi. I also would like to disable an instrumentation. Seems like @opentelemetry/instrumentation-fs is causing a lot of unnecessary traces and is recommended to be disabled: Disable @opentelemetry/instrumentation-fs instrumentation Feb 13, 2024
@mderazon mderazon changed the title Disable @opentelemetry/instrumentation-fs instrumentation Disable @opentelemetry/instrumentation-fs instrumentation Feb 13, 2024
@pavolloffay
Copy link
Member

Is there a way to disable it with env var?

Can it be disabled here https://github.com/open-telemetry/opentelemetry-operator/blob/main/autoinstrumentation/nodejs/src/autoinstrumentation.ts#L35 ? We could define an env var (specific for the operator) to enable it conditionally.

@pavolloffay pavolloffay added the help wanted Extra attention is needed label Feb 14, 2024
@atsu85
Copy link
Contributor

atsu85 commented Feb 21, 2024

Hi @mderazon and @pavolloffay!

I think we'd need a generic solution to disable any instrumentations, not just specific environment variable for disabling @opentelemetry/instrumentation-fs instrumentation.

There are different approaches we could take:
a) disabling instrumentations in opentelemetry-operator code based on opentelemetry-operator specific environment variables, like @pavolloffay mentioned
b) disabling instrumentations in @opentelemetry/auto-instrumentations-node package (that this operator is using) based on environment variables. Note that there are at least two issues created there related to it:

While both approaches would work without much effort, I'd prefer the the @opentelemetry/auto-instrumentations-node package specific solution, that would make life easier also for developers that don't use it with opentelemetry-operator.

I hope that people will agree to environment variable naming conventions for disabling nodejs auto-instrumentations. For now Java, Python and .Net have different naming conventions (as mentioned in this comment) and it would be bad if this library implemented its own naming convention that @opentelemetry/auto-instrumentations-node might not start using

@atsu85
Copy link
Contributor

atsu85 commented Mar 11, 2024

FYI: i created a PR #2745 to address this issue.

@mderazon, could you rename this issue from

Disable @opentelemetry/instrumentation-fs instrumentation

to

Allow disabling @opentelemetry/instrumentation-fs instrumentation

or even better:

Allow disabling nodejs auto-instrumentations via environment variables

so it could be closed after merging my PR without causing confusion if it is now disabled by default or can be disabled

@mderazon mderazon changed the title Disable @opentelemetry/instrumentation-fs instrumentation Allow disabling nodejs auto-instrumentations via environment variables Mar 11, 2024
atsu85 added a commit to atsu85/opentelemetry-operator that referenced this issue Mar 11, 2024
to allow enabling only selected instrumentations specified via environment variable `OTEL_NODE_ENABLED_INSTRUMENTATIONS`

open-telemetry#2622
atsu85 added a commit to atsu85/opentelemetry-operator that referenced this issue Mar 11, 2024
to allow enabling only selected instrumentations specified via environment variable `OTEL_NODE_ENABLED_INSTRUMENTATIONS`

open-telemetry#2622
pavolloffay pushed a commit that referenced this issue Mar 11, 2024
to allow enabling only selected instrumentations specified via environment variable `OTEL_NODE_ENABLED_INSTRUMENTATIONS`

#2622
atsu85 added a commit to atsu85/opentelemetry-operator that referenced this issue Mar 12, 2024
To trigger release for open-telemetry#2622 that already bumped auto-instrumentations-node via open-telemetry#2745
atsu85 added a commit to atsu85/opentelemetry-operator that referenced this issue Mar 12, 2024
To trigger release for open-telemetry#2622 that already bumped auto-instrumentations-node via open-telemetry#2745
atsu85 added a commit to atsu85/opentelemetry-operator that referenced this issue Mar 12, 2024
To trigger release for open-telemetry#2622 that already bumped auto-instrumentations-node via open-telemetry#2745
pavolloffay pushed a commit that referenced this issue Mar 13, 2024
…toinstrumentation/nodejs (#2754)

* Bump all NodeJS devDependences to latest versions

* Bump all NodeJS dependences to latest versions

To trigger release for #2622 that already bumped auto-instrumentations-node via #2745
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this issue May 1, 2024
to allow enabling only selected instrumentations specified via environment variable `OTEL_NODE_ENABLED_INSTRUMENTATIONS`

open-telemetry#2622
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this issue May 1, 2024
…toinstrumentation/nodejs (open-telemetry#2754)

* Bump all NodeJS devDependences to latest versions

* Bump all NodeJS dependences to latest versions

To trigger release for open-telemetry#2622 that already bumped auto-instrumentations-node via open-telemetry#2745
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment