feat(node): Expose functional integrations to replace classes #10356
Conversation
| return new Undici(options) as unknown as IntegrationFnResult; | ||
| }) satisfies IntegrationFn; | ||
|
|
||
| export const undiciIntegration = defineIntegration(_undiciIntegration); |
There was a problem hiding this comment.
m: we should probably rename this right now tbh. Nobody recognizes Undici 😢
There was a problem hiding this comment.
ahh right! Let's go with nativeNodeFetchIntegration() as Luca proposed some time ago 👍
mydea
force-pushed
the
fn/node-integrations
branch
from
85b971c
to
a116e73
Compare
mydea
force-pushed
the
fn/node-integrations
branch
from
a116e73
to
993582b
Compare
size-limit report 📦
|
mydea
force-pushed
the
fn/node-integrations
branch
from
993582b
to
4770c56
Compare
|
Update: So I updated the node http/fetch integrations so that the functional integrations have slightly new/improved behavior: With them, span creation will be automatically enabled if tracing is enabled - so no more need to add |
There was a problem hiding this comment.
No action required/General comment: Are we actually exporting the async localvariables integration? Already stumbled upon this yesterday and wasn't sure.
There was a problem hiding this comment.
no, just the sync one right now!
| | `new Console()` | `consoleIntegration()` | `@sentry/node` | | ||
| | `new Context()` | `nodeContextIntegration()` | `@sentry/node` | | ||
| | `new Modules()` | `modulesIntegration()` | `@sentry/node` | | ||
| | `new OnUncaughtException()` | `onUncaughtExceptionIntegration()` | `@sentry/node` | | ||
| | `new OnUnhandledRejection()` | `onUnhandledRejectionIntegration()` | `@sentry/node` | | ||
| | `new LocalVariables()` | `localVariablesIntegration()` | `@sentry/node` | | ||
| | `new Spotlight()` | `spotlightIntergation()` | `@sentry/node` | | ||
| | `new Anr()` | `anrIntergation()` | `@sentry/node` | | ||
| | `new Hapi()` | `hapiIntegration()` | `@sentry/node` | | ||
| | `new Undici()` | `nativeNodeFetchIntegration()` | `@sentry/node` | | ||
| | `new Http()` | `httpIntegration()` | `@sentry/node` | |
There was a problem hiding this comment.
I think we need to do a better job to call out that these integration chanes also apply to all/most @sentry/node-based packages (serverless, fullstack framework SDKs). Doesn't strictly have to happen now but I'd say we should add them. Probably simplest to list them individually (also, they slightly differ per integration :( )
There was a problem hiding this comment.
yeah, I thought about this too but this makes the table veeery unwieldly 😓 not sure, maybe we just point it out above? E.g. node also applies to next, remix, sveltekit, ..., browser also applies to react, ...?
mydea
force-pushed
the
fn/node-integrations
branch
from
4770c56
to
86c6931
Compare
We'll need to adjust the http & undici options a bit to align with otel integrations, but we can do this in a follow up, otherwise this will become a bit unwieldly...
mydea
force-pushed
the
fn/node-integrations
branch
from
86c6931
to
6827886
Compare
This updates the general (non-tracing) node integrations to be functional.
For node & undici, the replacements are slightly different:
new Http()-->httpIntegration(): In contrast to the class integration, this will create spans by default if tracing is enabled. While at it, this also "fixes" that iftracing: falseis set, no spans will be created.new Undici()-->nativeNodeFetchIntegration(): Renamed this for consistency, and added atracingoption similar to http to allow to disable span creation.We can't really deprecate
Integrations.xxxyet until we have replacements for the tracing integrations 😬 So that would also be a todo left.