-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(node): Always set up express, fastify, koa, hapi integrations #23473
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
Changes from all commits
8328ed4
cff54b9
3137104
54b9801
55dbca3
b733457
20ff2d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // <reference lib="deno.ns" /> | ||
|
|
||
| import { channel } from 'node:diagnostics_channel'; | ||
| import type { DenoClient } from '@sentry/deno'; | ||
| import { init } from '@sentry/deno'; | ||
| import { assert } from 'https://deno.land/std@0.212.0/assert/assert.ts'; | ||
| import { assertEquals } from 'https://deno.land/std@0.212.0/assert/assert_equals.ts'; | ||
| import { assertExists } from 'https://deno.land/std@0.212.0/assert/assert_exists.ts'; | ||
| import { errorSink, resetGlobals, withTimeout } from '../../src/index.ts'; | ||
|
|
||
| Deno.test('fastify instrumentation: included in default integrations (Deno 2.8.0+)', () => { | ||
| resetGlobals(); | ||
| const client = init({ traceLifecycle: 'static', dsn: 'https://username@domain/123' }) as DenoClient; | ||
| const names = client.getOptions().integrations.map(i => i.name); | ||
| assert(names.includes('Fastify'), `Fastify should be in defaults, got ${names.join(', ')}`); | ||
| }); | ||
|
|
||
| Deno.test('fastify instrumentation: tracing:fastify.request.handler:error channel captures the error', async () => { | ||
| resetGlobals(); | ||
| const sink = errorSink(); | ||
| init({ | ||
| traceLifecycle: 'static', | ||
| dsn: 'https://username@domain/123', | ||
| beforeSend: sink.beforeSend, | ||
| }); | ||
|
|
||
| const error = new Error('fastify boom'); | ||
|
|
||
| // Fastify v5 publishes this native diagnostics channel when a request handler errors; the | ||
| // integration subscribes to it directly (no orchestrion injection needed). A 5xx reply passes the | ||
| // default `shouldHandleError`, so the error is captured. | ||
| channel('tracing:fastify.request.handler:error').publish({ | ||
| error, | ||
| request: { method: 'GET', routeOptions: { url: '/boom' } }, | ||
| reply: { statusCode: 500 }, | ||
| }); | ||
|
|
||
| const event = await withTimeout( | ||
| sink.waitFor(e => e.exception?.values?.[0]?.value === 'fastify boom'), | ||
| 5000, | ||
| "the captured 'fastify boom' error", | ||
| ); | ||
|
|
||
| assertExists(event.exception?.values?.[0]); | ||
| assertEquals(event.exception?.values?.[0]?.mechanism?.type, 'auto.function.fastify'); | ||
| assertEquals(event.exception?.values?.[0]?.mechanism?.handled, false); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,9 +12,13 @@ import type { NodeClient } from '@sentry/node'; | |
| import { | ||
| consoleIntegration, | ||
| contextLinesIntegration, | ||
| expressIntegration, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, interesting consequence of this: Bun adds these integrations, but they can't actually fire unless the plugin passed is to The exception is Fastify, of course, since it ships its own channels, so that'll work for |
||
| fastifyIntegration, | ||
| getAutoPerformanceIntegrations, | ||
| hapiIntegration, | ||
| httpIntegration, | ||
| init as initNode, | ||
| koaIntegration, | ||
| modulesIntegration, | ||
| nodeContextIntegration, | ||
| onUncaughtExceptionIntegration, | ||
|
|
@@ -64,6 +68,12 @@ export function getDefaultIntegrationsWithoutPerformance(): Integration[] { | |
| nodeContextIntegration(), | ||
| modulesIntegration(), | ||
| processSessionIntegration(), | ||
| // Framework-level integrations. These are not performance-only: they also handle error capture, so | ||
| // they are added by default rather than gated behind tracing (matching the Node SDK). | ||
| expressIntegration(), | ||
| fastifyIntegration(), | ||
| hapiIntegration(), | ||
| koaIntegration(), | ||
| // Bun Specific | ||
| bunServerIntegration(), | ||
| bunHttpServerIntegration(), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,14 +3,11 @@ import { | |
| prismaIntegration, | ||
| amqplibIntegration, | ||
| anthropicAIIntegration, | ||
| expressIntegration, | ||
| firebaseIntegration, | ||
| genericPoolIntegration, | ||
| googleGenAIIntegration, | ||
| graphqlIntegration, | ||
| hapiIntegration, | ||
| kafkaIntegration, | ||
| koaIntegration, | ||
| langChainIntegration, | ||
| langGraphIntegration, | ||
| lruMemoizerIntegration, | ||
|
|
@@ -25,12 +22,13 @@ import { | |
| tediousIntegration, | ||
| vercelAIIntegration, | ||
| } from '@sentry/server-utils'; | ||
| import { fastifyIntegration } from './fastify'; | ||
|
|
||
| export function getAutoPerformanceIntegrations(): Integration[] { | ||
| // The following integrations are not considered performance integrations because they are "framework"-level | ||
| // meaning they may also handle error capture and similar things. | ||
| // Thus, we add them by default: | ||
| // express, fastify, hapi, koa | ||
|
Comment on lines
+27
to
+30
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this comment is weirdly placed. Shouldn't it be at packages/node/src/sdk/index.ts line 73, or wherever the integrations ultimately get moved to? |
||
| return [ | ||
| expressIntegration(), | ||
| fastifyIntegration(), | ||
| graphqlIntegration(), | ||
| mongoIntegration(), | ||
| mongooseIntegration(), | ||
|
|
@@ -39,8 +37,6 @@ export function getAutoPerformanceIntegrations(): Integration[] { | |
| redisIntegration(), | ||
| postgresIntegration(), | ||
| prismaIntegration(), | ||
| hapiIntegration(), | ||
| koaIntegration(), | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| tediousIntegration(), | ||
| genericPoolIntegration(), | ||
| kafkaIntegration(), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ import { | |
| stackParserFromStackParserOptions, | ||
| } from '@sentry/core'; | ||
| import { isMainThread, parentPort } from 'node:worker_threads'; | ||
| import { detectOrchestrionSetup } from '@sentry/server-utils'; | ||
| import { detectOrchestrionSetup, expressIntegration, hapiIntegration, koaIntegration } from '@sentry/server-utils'; | ||
| import { registerDiagnosticsChannelInjection } from '@sentry/server-utils/orchestrion/register'; | ||
| import { DEBUG_BUILD } from '../debug-build'; | ||
| import { childProcessIntegration } from '../integrations/childProcess'; | ||
|
|
@@ -41,6 +41,7 @@ import { getSpotlightConfig } from '../utils/spotlight'; | |
| import { defaultStackParser, getSentryRelease } from './api'; | ||
| import { NodeClient } from './client'; | ||
| import { initOpenTelemetry } from './initOtel'; | ||
| import { fastifyIntegration } from '../integrations/tracing/fastify'; | ||
|
|
||
| /** | ||
| * Get the base default integrations shared by all Node SDK default-integration sets. | ||
|
|
@@ -69,6 +70,11 @@ function getBaseDefaultIntegrations(): Integration[] { | |
| workerThreadsIntegration(), | ||
| processSessionIntegration(), | ||
| modulesIntegration(), | ||
| // Framework-level integrations | ||
| expressIntegration(), | ||
| fastifyIntegration(), | ||
| hapiIntegration(), | ||
| koaIntegration(), | ||
|
Comment on lines
+73
to
+77
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These four show up in a few places now:
I fee like this is prone to overlooking, if we add another framework that belongs to this set. Could it live in a list somewhere centrally reusable? |
||
| ]; | ||
| } | ||
|
|
||
|
|
@@ -84,9 +90,8 @@ export function getDefaultIntegrations(options: Options): Integration[] { | |
| return [ | ||
| ...getDefaultIntegrationsWithoutPerformance(), | ||
| // We only add performance integrations if tracing is enabled | ||
| // Note that this means that without tracing enabled, e.g. `expressIntegration()` will not be added | ||
| // This means that generally request isolation will work (because that is done by httpIntegration) | ||
| // But `transactionName` will not be set automatically | ||
| // Note that integrations like `httpIntegration` or `expressIntegration` are always added, | ||
| // because they also handle non-tracing related functionality. | ||
| ...(hasSpansEnabled(options) ? getAutoPerformanceIntegrations() : []), | ||
| ]; | ||
| } | ||
|
|
@@ -147,20 +152,20 @@ function _init( | |
| } | ||
| } | ||
|
|
||
| // Resolve the tracing-affecting options (e.g. `SENTRY_TRACES_SAMPLE_RATE`) up front so that both | ||
| // the span-enablement gate below and default-integration selection see the final values. Without | ||
| // this, enabling tracing purely via env would leave `hasSpansEnabled` false at this point and skip | ||
| // the performance integrations. `getClientOptions` resolves the remaining options later. | ||
| // Resolve the tracing-affecting options (e.g. `SENTRY_TRACES_SAMPLE_RATE`) up front so that | ||
| // default-integration selection sees the final values. Without this, enabling tracing purely via | ||
| // env would leave `hasSpansEnabled` false at this point and skip the performance integrations. | ||
| // `getClientOptions` resolves the remaining options later. | ||
| const optionsWithResolvedTracing = { | ||
| ...options, | ||
| tracesSampleRate: getTracesSampleRate(options.tracesSampleRate), | ||
| }; | ||
|
|
||
| // Gate channel-based (orchestrion diagnostics-channel) instrumentation on span recording: the | ||
| // channel integrations only produce spans, so with tracing off there are no subscribers and | ||
| // injecting the module hooks would be pointless work. Install the hooks as early as possible, | ||
| // before the app imports its instrumented modules. | ||
| const useChannelInjection = hasSpansEnabled(optionsWithResolvedTracing); | ||
| // Install the channel-based (orchestrion diagnostics-channel) instrumentation hooks by default, | ||
| // independent of tracing — the channel integrations also capture errors, not just spans. Opt out | ||
| // with `enableRuntimeChannelInjection: false`. Install as early as possible, before the app imports | ||
| // its instrumented modules. | ||
|
sentry[bot] marked this conversation as resolved.
|
||
| const useChannelInjection = options.enableRuntimeChannelInjection !== false; | ||
| if (useChannelInjection) { | ||
| registerDiagnosticsChannelInjection(); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a copy-paste of TransactionSink above it. Could we abstract them both in to a
function makeSink<T>(): { record(e: T): null; waitFor(p): Promise<T> }reusable?It's a test, so not super high priority, but could be a bit less to look at.