feat(otel): Disable node tracing auto instrumentation when using otel

[mydea]

With this PR, when instrumenter: 'otel' is configured in the Node SDK, any auto-instrumentation that generates transactions is skipped.

This is a bit tricky to test, so I created a separate PR for the tests, if we don't like the approach: #6143

ref #6127

[Lms24]

Overall this LGTM and I think we can merge this.

Just out of curiousity: The way I understand this, with this change we'd call setupOnce but early return if we see that the instrumenter is something else than 'sentry'. I'm wondering if we could achieve the same thing by simply removing the integrations from the list in the Node SDK's init function? You probably already thought this through so take this as a question rather than a request to change anything

[mydea]

Overall this LGTM and I think we can merge this.

Just out of curiousity: The way I understand this, with this change we'd call setupOnce but early return if we see that the instrumenter is something else than 'sentry'. I'm wondering if we could achieve the same thing by simply removing the integrations from the list in the Node SDK's init function? You probably already thought this through so take this as a question rather than a request to change anything

Honestly, there wasn't a great reason for this. There has already been some logic for "skipping" integrations in there, which is why I added onto there. But IMHO the other approach could also work well 🤔 I guess, if we remove entries after getIntegrationsToSetup in the node SDK, we can be sure that they can never be added?

So, I'm fine with either approach, really. I guess the key question is what place should "own" the decision to skip, the integration itself or a more central/global place. Let me know if you prefer the other solution, then I can also change it.

[Lms24]

As I said, I'm fine with both options as well and if there is already logic to skip in the integrations then that makes an argument to add it there as well.

The only reason I'd see to maybe do it in the init call is that custom integrations that don't check for the instrumenter could be skipped as well. Although I don't know how we want to handle that in general (maybe something we should consider for all SDKs?)

[mydea]

yeah, the tricky part is that we don't want to skip all integrations, only ones that create transactions 😅 So I don't think we can realistically filter out integrations generally (Sidenote: Kind of related to #4790, if we could split up integrations into single responsibilities and maybe even tag them somehow e.g. with a type or so, then we could actually dynamically filter them out, possibly. That would be nice!)

So, then I'd say I will go with the current approach (simply because it is done), but we maybe can/should revisit this when we (hopefully) at some point revisit integrations a bit.

[Lms24]

So, then I'd say I will go with the current approach

Sounds good to me 👍

[AbhiPrasad]

One day we will revamp the integrations API.

and when it happens it will be a glorious

until then we do what it takes.

[github-actions]

size-limit report 📦

Path	Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified)	19.5 KB (0%)
@sentry/browser - ES5 CDN Bundle (minified)	60.37 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)	18.15 KB (+0.02% 🔺)
@sentry/browser - ES6 CDN Bundle (minified)	53.71 KB (0%)
@sentry/browser - Webpack (gzipped + minified)	19.89 KB (0%)
@sentry/browser - Webpack (minified)	65.12 KB (0%)
@sentry/react - Webpack (gzipped + minified)	19.92 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified)	45.88 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)	26.3 KB (+0.01% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)	24.7 KB (+0.01% 🔺)