feat(auto-instrumentations-node)!: disable @opentelemetry/instrumentation-fs by default

[pichlermarc]

Which problem is this PR solving?

see #2453 - many users seek to disable @opentelemetry/instrumentation-fs due to it's verbosity. This is so common that we have added instructions on how to disable it everywhere.

This PR disables the instrumentation by default. To get back to the previous behavior, end-users can add 'fs' to a comma seperated list in OTEL_NODE_ENABLED_INSTRUMENTATIONS when using enviornment variable configuration, or they can explicitly enable the instrumentation like so when using getNodeAutoInstrumentations():

    getNodeAutoInstrumentations({
      '@opentelemetry/instrumentation-fs': {
        enabled: true,
      },
    }),

FAQ

I need @opentelemetry/instrumentation-fs back, how do I get back to the previous state?

If you're using @opentelemetry/auto-instrumentations-node/register without OTEL_NODE_ENABLED_INSTRUMENTATIONS set

• set OTEL_NODE_ENABLED_INSTRUMENTATIONS=amqplib,aws-lambda,aws-sdk,bunyan,cassandra-driver,connect,cucumber,dataloader,dns,express,fastify,fs,pool,graphql,grpc,hapi,http,ioredis,kafkajs,knex,koa,memoizer,memcached,instrumentation-mongodb,instrumentation-mongoose,instrumentation-mysql2,instrumentation-mysql,instrumentation-nestjs-core,instrumentation-net,instrumentation-pg,instrumentation-pino,instrumentation-redis,instrumentation-redis-4,instrumentation-restify,instrumentation-router

If you're using getAutoInstrumentationsNode() without an options object that contains @opentelemetry/fs and without OTEL_NODE_ENABLED_INSTRUMENTATIONS set

• set OTEL_NODE_ENABLED_INSTRUMENTATIONS=amqplib,aws-lambda,aws-sdk,bunyan,cassandra-driver,connect,cucumber,dataloader,dns,express,fastify,fs,pool,graphql,grpc,hapi,http,ioredis,kafkajs,knex,koa,memoizer,memcached,instrumentation-mongodb,instrumentation-mongoose,instrumentation-mysql2,instrumentation-mysql,instrumentation-nestjs-core,instrumentation-net,instrumentation-pg,instrumentation-pino,instrumentation-redis,instrumentation-redis-4,instrumentation-restify,instrumentation-router

If you're using getAutoInstrumentationsNode() but you don't want to use enviornment variables

• change the code to

    getNodeAutoInstrumentations({
      '@opentelemetry/instrumentation-fs': {
        enabled: true,
      },
    }),

Fixes #2453

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.74%. Comparing base (0341e89) to head (776a937).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2467   +/-   ##
=======================================
  Coverage   90.74%   90.74%           
=======================================
  Files         156      156           
  Lines        7723     7725    +2     
  Branches     1588     1588           
=======================================
+ Hits         7008     7010    +2     
  Misses        715      715           

Files with missing lines	Coverage Δ
...tapackages/auto-instrumentations-node/src/utils.ts	98.23% <100.00%> (+0.03%)	⬆️

[trentm]

To get back to the previous behavior, end-users can add 'fs' to a comma seperated list in OTEL_NODE_ENABLED_INSTRUMENTATIONS

nit: The linked issue points out this change for the (expected to be very rare) user that relies on the fs instrumentation (emphasis mine):

This has the drawback of breaking users that rely on the telemetry generated by this instrumentation. If they want to get it back, they have to explicitly enable it via one of the above options. OTEL_NODE_ENABLED_INSTRUMENTATIONS is an allow-list so they will have to also explicitly list every other instrumentation they want to have enabled, which may be tedious.

Ideally it would be nice to point this out in the changelog. However, I don't recall if we have a way for a PR to add some explicit prose to the changelog. A fallback is to perhaps clearly point out what the user needs to do in the PR description?

[pichlermarc]

To get back to the previous behavior, end-users can add 'fs' to a comma seperated list in OTEL_NODE_ENABLED_INSTRUMENTATIONS

nit: The linked issue points out this change for the (expected to be very rare) user that relies on the fs instrumentation (emphasis mine):

This has the drawback of breaking users that rely on the telemetry generated by this instrumentation. If they want to get it back, they have to explicitly enable it via one of the above options. OTEL_NODE_ENABLED_INSTRUMENTATIONS is an allow-list so they will have to also explicitly list every other instrumentation they want to have enabled, which may be tedious.

Ideally it would be nice to point this out in the changelog. However, I don't recall if we have a way for a PR to add some explicit prose to the changelog. A fallback is to perhaps clearly point out what the user needs to do in the PR description?

Good point. I've added some info to the PR description. I have not found a simple way to have the auto-generated changelog include details, but I'll modify the Github Release to include that info as well 🙂

[ilushaR]

@pichlermarc the fix isn’t working as expected.

Even when I use '@opentelemetry/instrumentation-fs': { enabled: true }, the instrumentation still doesn’t enable because it isn’t included in the enabledInstrumentationsFromEnv list.

This check seems incorrect.

[pichlermarc]

@pichlermarc the fix isn’t working as expected.

Even when I use '@opentelemetry/instrumentation-fs': { enabled: true }, the instrumentation still doesn’t enable because it isn’t included in the enabledInstrumentationsFromEnv list.

This check seems incorrect.

Hi @ilushaR - thanks for letting me know. 🙂 I'll have a look. I opened #2515 to follow up.

(FYI: in the future please always open a new issue rather then commenting on a closed PR/issue, opening a new one will ensure that we see a bug report - I usually filter out any activity on closed PRs/issues. 🙂 )