diff --git a/dev-packages/node-integration-tests/suites/vercel/instrument.mjs b/dev-packages/node-integration-tests/suites/vercel/instrument.mjs deleted file mode 100644 index e3a1dbab7ba5..000000000000 --- a/dev-packages/node-integration-tests/suites/vercel/instrument.mjs +++ /dev/null @@ -1,13 +0,0 @@ -import * as Sentry from '@sentry/node'; -import { loggingTransport } from '@sentry-internal/node-integration-tests'; - -process.env.VERCEL = 'true'; - -Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - release: '1.0', - tracesSampleRate: 1.0, - transport: loggingTransport, - // We look at debug logs in this test - debug: true, -}); diff --git a/dev-packages/node-integration-tests/suites/vercel/scenario.mjs b/dev-packages/node-integration-tests/suites/vercel/scenario.mjs deleted file mode 100644 index b2ed413175df..000000000000 --- a/dev-packages/node-integration-tests/suites/vercel/scenario.mjs +++ /dev/null @@ -1,13 +0,0 @@ -import * as Sentry from '@sentry/node'; -import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; -import express from 'express'; - -const app = express(); - -app.get('/test/express', (_req, res) => { - res.send({ response: 'response 1' }); -}); - -Sentry.setupExpressErrorHandler(app); - -startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/vercel/test.ts b/dev-packages/node-integration-tests/suites/vercel/test.ts deleted file mode 100644 index 4517d0eaf115..000000000000 --- a/dev-packages/node-integration-tests/suites/vercel/test.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { afterAll, describe, expect } from 'vitest'; -import { cleanupChildProcesses, createEsmAndCjsTests } from '../../utils/runner'; - -describe('vercel xxx', () => { - afterAll(() => { - cleanupChildProcesses(); - }); - - createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { - test('should flush events correctly on Vercel', async () => { - const runner = createRunner() - .expect({ - transaction: { - transaction: 'GET /test/express', - }, - }) - .start(); - runner.makeRequest('get', '/test/express'); - await runner.completed(); - - const actualLogs = runner.getLogs(); - - // We want to test that the following logs are present in this order - // other logs may be in between - const expectedLogs = [ - 'Sentry Logger [log]: @sentry/instrumentation-http Patching server.emit', - 'Sentry Logger [log]: @sentry/instrumentation-http Handling incoming request', - 'Sentry Logger [log]: @sentry/instrumentation-http Patching request.on', - 'Sentry Logger [debug]: @opentelemetry_sentry-patched/instrumentation-http http instrumentation incomingRequest', - 'Sentry Logger [log]: [Tracing] Starting sampled root span', - // later... - 'Sentry Logger [log]: Patching response to flush on Vercel', - 'Sentry Logger [log]: Patching res.end()', - // later... - 'Sentry Logger [log]: Flushing events before Vercel Lambda freeze', - 'Sentry Logger [log]: SpanExporter exported 4 spans, 0 spans are waiting for their parent spans to finish', - ]; - - // Test that the order of logs is correct - for (const log of actualLogs) { - if (expectedLogs.length === 0) { - break; - } - - if (log === expectedLogs[0]) { - expectedLogs.shift(); - } - } - - expect(expectedLogs).toEqual([]); - }); - }); -}); diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentationBeforeOtel.ts b/packages/node/src/integrations/http/SentryHttpInstrumentationBeforeOtel.ts deleted file mode 100644 index ace8cbfb4399..000000000000 --- a/packages/node/src/integrations/http/SentryHttpInstrumentationBeforeOtel.ts +++ /dev/null @@ -1,136 +0,0 @@ -import type * as http from 'node:http'; -import type * as https from 'node:https'; -import { VERSION } from '@opentelemetry/core'; -import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; -import { flush, logger, vercelWaitUntil } from '@sentry/core'; -import { DEBUG_BUILD } from '../../debug-build'; -import { stealthWrap } from './utils'; - -type Http = typeof http; -type Https = typeof https; - -// The reason this "before OTEL" integration even exists is due to timing reasons. We need to be able to register a -// `res.on('close')` handler **after** OTEL registers its own handler (which it uses to end spans), so that we can do -// something (ie. flush) after OTEL has ended a span for a request. If you think about it like an onion: -// -// (Sentry after OTEL instrumentation -// (OTEL instrumentation -// (Sentry before OTEL instrumentation -// (orig HTTP request handler)))) -// -// registering an instrumentation before OTEL allows us to do this for incoming requests. - -/** - * A Sentry specific http instrumentation that is applied before the otel instrumentation. - */ -export class SentryHttpInstrumentationBeforeOtel extends InstrumentationBase { - public constructor() { - super('@sentry/instrumentation-http-before-otel', VERSION, {}); - } - - // eslint-disable-next-line jsdoc/require-jsdoc - public init(): [InstrumentationNodeModuleDefinition, InstrumentationNodeModuleDefinition] { - return [this._getHttpsInstrumentation(), this._getHttpInstrumentation()]; - } - - /** Get the instrumentation for the http module. */ - private _getHttpInstrumentation(): InstrumentationNodeModuleDefinition { - return new InstrumentationNodeModuleDefinition('http', ['*'], (moduleExports: Http): Http => { - // Patch incoming requests - stealthWrap(moduleExports.Server.prototype, 'emit', this._getPatchIncomingRequestFunction()); - - return moduleExports; - }); - } - - /** Get the instrumentation for the https module. */ - private _getHttpsInstrumentation(): InstrumentationNodeModuleDefinition { - return new InstrumentationNodeModuleDefinition('https', ['*'], (moduleExports: Https): Https => { - // Patch incoming requests - stealthWrap(moduleExports.Server.prototype, 'emit', this._getPatchIncomingRequestFunction()); - - return moduleExports; - }); - } - - /** - * Patch the incoming request function for request isolation. - */ - private _getPatchIncomingRequestFunction(): ( - original: (event: string, ...args: unknown[]) => boolean, - ) => (this: unknown, event: string, ...args: unknown[]) => boolean { - return ( - original: (event: string, ...args: unknown[]) => boolean, - ): ((this: unknown, event: string, ...args: unknown[]) => boolean) => { - return function incomingRequest(this: unknown, ...args: [event: string, ...args: unknown[]]): boolean { - // Only traces request events - if (args[0] !== 'request') { - return original.apply(this, args); - } - - const response = args[2] as http.OutgoingMessage; - patchResponseToFlushOnServerlessPlatforms(response); - - return original.apply(this, args); - }; - }; - } -} - -function patchResponseToFlushOnServerlessPlatforms(res: http.OutgoingMessage): void { - // Freely extend this function with other platforms if necessary - if (process.env.VERCEL) { - DEBUG_BUILD && logger.log('Patching response to flush on Vercel'); - - // In some cases res.end does not seem to be defined leading to errors if passed to Proxy - // https://github.com/getsentry/sentry-javascript/issues/15759 - if (typeof res.end !== 'function') { - DEBUG_BUILD && logger.warn('res.end is not a function, skipping patch...'); - return; - } - - let markOnEndDone = (): void => undefined; - const onEndDonePromise = new Promise(res => { - markOnEndDone = res; - }); - - res.on('close', () => { - markOnEndDone(); - }); - - logger.log('Patching res.end()'); - - // eslint-disable-next-line @typescript-eslint/unbound-method - res.end = new Proxy(res.end, { - apply(target, thisArg, argArray) { - vercelWaitUntil( - new Promise(finishWaitUntil => { - // Define a timeout that unblocks the lambda just to be safe so we're not indefinitely keeping it alive, exploding server bills - const timeout = setTimeout(() => { - finishWaitUntil(); - }, 2000); - - onEndDonePromise - .then(() => { - DEBUG_BUILD && logger.log('Flushing events before Vercel Lambda freeze'); - return flush(2000); - }) - .then( - () => { - clearTimeout(timeout); - finishWaitUntil(); - }, - e => { - clearTimeout(timeout); - DEBUG_BUILD && logger.log('Error while flushing events for Vercel:\n', e); - finishWaitUntil(); - }, - ); - }), - ); - - return target.apply(thisArg, argArray); - }, - }); - } -} diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index d25d19a86c8c..9d1113702410 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -12,7 +12,6 @@ import { addOriginToSpan } from '../../utils/addOriginToSpan'; import { getRequestUrl } from '../../utils/getRequestUrl'; import type { SentryHttpInstrumentationOptions } from './SentryHttpInstrumentation'; import { SentryHttpInstrumentation } from './SentryHttpInstrumentation'; -import { SentryHttpInstrumentationBeforeOtel } from './SentryHttpInstrumentationBeforeOtel'; const INTEGRATION_NAME = 'Http'; @@ -117,10 +116,6 @@ interface HttpOptions { }; } -const instrumentSentryHttpBeforeOtel = generateInstrumentOnce(`${INTEGRATION_NAME}.sentry-before-otel`, () => { - return new SentryHttpInstrumentationBeforeOtel(); -}); - const instrumentSentryHttp = generateInstrumentOnce( `${INTEGRATION_NAME}.sentry`, options => { @@ -162,19 +157,6 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) => return { name: INTEGRATION_NAME, setupOnce() { - // TODO: get rid of this too - // Below, we instrument the Node.js HTTP API three times. 2 times Sentry-specific, 1 time OTEL specific. - // Due to timing reasons, we sometimes need to apply Sentry instrumentation _before_ we apply the OTEL - // instrumentation (e.g. to flush on serverless platforms), and sometimes we need to apply Sentry instrumentation - // _after_ we apply OTEL instrumentation (e.g. for isolation scope handling and breadcrumbs). - - // This is Sentry-specific instrumentation that is applied _before_ any OTEL instrumentation. - if (process.env.VERCEL) { - // Currently this instrumentation only does something when deployed on Vercel, so to save some overhead, we short circuit adding it here only for Vercel. - // If it's functionality is extended in the future, feel free to remove the if statement and this comment. - instrumentSentryHttpBeforeOtel(); - } - const instrumentSpans = _shouldInstrumentSpans(options, getClient()?.getOptions()); // This is Sentry-specific instrumentation for request isolation and breadcrumbs