getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.85k stars 1.55k forks source link

Duplication of OpenTelemetry Spans in HTTP requests with @sentry/node #12969

Closed vibaiher-qatium closed 1 month ago

vibaiher-qatium commented 1 month ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.18.0

Framework Version

Express 4.19.2

Link to Sentry event

No response

SDK Setup/Reproduction Example

Sentry.init({
      dsn: settings.server,
      environment: settings.environment,
      release: settings.release,
      enabled: settings.enabled,
      skipOpenTelemetrySetup: true,
      initialScope: (scope) => {
        scope.setTag("runtime_environment", settings.runtime);
        return scope;
      }
});

Steps to Reproduce

  1. Enable opentelemetry in your app using a file that is registered when executing node. Opentelemetry setup must use node autoinstrumentation. Example: node --require @qatium/open-telemetry-express build/api.js
  2. In you express api file, enable Sentry integration with opentelemetry setup to false and tracing disabled.
  3. Generate some requests in endpoints that have some http calls.

Expected Result

The expected behavior is that @sentry/node does not register the opentelemetry-instrumentation-node-fetch instrumentation if the tracing option is disabled in Sentry's configuration.

Actual Result

Despite having the tracing option disabled in Sentry's configuration, the opentelemetry-instrumentation-node-fetch library is still being registered, causing duplication of Span information when using OpenTelemetry instrumentation for HTTP.

image

andreiborza commented 1 month ago

Hello thanks for writing in.

Could you please provide a stackblitz or a github repo where we could try this out?

vibaiher-qatium commented 1 month ago

Thank you for your prompt response. I understand the need for a sample project to help investigate the issue. However, creating such a project would be quite time-consuming and resource-intensive for us at the moment.

While trying to understand the code, I arrived at the following points:

It appears that the code is not checking whether tracing is enabled or not. It seems likely that adding a guard clause could resolve the issue.

Is it possible to look into this issue based on the description and details provided? We have outlined the specific problem and steps to reproduce it in detail. If you need any specific logs or further information, I would be happy to provide them.

Thank you for your understanding and assistance.

andreiborza commented 1 month ago

Understood, thanks for having a closer look. We discussed this internally and we might want to guard these integrations in the future.

For now, we recommend to filter out the node fetch integration in your init call:

const integrations = Sentry.getDefaultIntegrationsWithoutPerformance().filter((integration) => integration.name !== 'NodeFetch')

Sentry.init({
   ...
   integrations,
})

Please let me know if that helps.

vibaiher-qatium commented 1 month ago

Thank you @andreiborza, we will give it a try.

vibaiher-qatium commented 1 month ago

Nope, the workaround does not work :(

image

andreiborza commented 1 month ago

@vibaiher-qatium sorry I forgot to mention you need to set defaultIntegrations: false as well. Does it work with that?

mydea commented 1 month ago

I also created an issue to improve/fix this on our end: https://github.com/getsentry/sentry-javascript/issues/12984

vibaiher-qatium commented 1 month ago

Thank you very much, in the end, the workaround of using the filtered list of integrations and disabling the defaults did work.

mydea commented 1 month ago

A note for the future, once https://github.com/getsentry/sentry-javascript/pull/13003 is released (in the next version of the SDK), this will not be necessary anymore - the node-fetch integration will not create spans anymore than in your scenario!