From edabe21a2813a1e2020160a4d3c038adea5d0141 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Wed, 7 May 2025 17:12:25 +0200 Subject: [PATCH 1/3] ref(node): Remove vercel flushing code that does nothing (#16217) We could never get this to apply properly on vercel in production, so we're removing this for now and may revisit this later. Replaces https://github.com/getsentry/sentry-javascript/pull/16178 --- .../suites/vercel/instrument.mjs | 13 -- .../suites/vercel/scenario.mjs | 13 -- .../suites/vercel/test.ts | 53 ------- .../SentryHttpInstrumentationBeforeOtel.ts | 136 ------------------ packages/node/src/integrations/http/index.ts | 18 --- 5 files changed, 233 deletions(-) delete mode 100644 dev-packages/node-integration-tests/suites/vercel/instrument.mjs delete mode 100644 dev-packages/node-integration-tests/suites/vercel/scenario.mjs delete mode 100644 dev-packages/node-integration-tests/suites/vercel/test.ts delete mode 100644 packages/node/src/integrations/http/SentryHttpInstrumentationBeforeOtel.ts 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 From 22a0e35c6a7f03ffca224544943a252bb4c1a461 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 7 May 2025 12:26:40 -0400 Subject: [PATCH 2/3] fix(core): Make sure logs get flushed in server-runtime-client (#16222) Actually fixes https://github.com/getsentry/sentry-javascript/issues/16110 --- packages/core/src/server-runtime-client.ts | 5 +++++ .../test/lib/server-runtime-client.test.ts | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index aa5228d53a14..261c5b582c23 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -52,6 +52,7 @@ export class ServerRuntimeClient< if (this._options._experiments?.enableLogs) { // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; + client.on('flushLogs', () => { client._logWeight = 0; clearTimeout(client._logFlushIdleTimeout); @@ -72,6 +73,10 @@ export class ServerRuntimeClient< }, DEFAULT_LOG_FLUSH_INTERVAL); } }); + + client.on('flush', () => { + _INTERNAL_flushLogsBuffer(client); + }); } } diff --git a/packages/core/test/lib/server-runtime-client.test.ts b/packages/core/test/lib/server-runtime-client.test.ts index 823757004814..d5b9dc33681d 100644 --- a/packages/core/test/lib/server-runtime-client.test.ts +++ b/packages/core/test/lib/server-runtime-client.test.ts @@ -278,5 +278,25 @@ describe('ServerRuntimeClient', () => { expect(sendEnvelopeSpy).not.toHaveBeenCalled(); expect(client['_logWeight']).toBe(0); }); + + it('flushes logs when flush event is triggered', () => { + const options = getDefaultClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + }); + client = new ServerRuntimeClient(options); + + const sendEnvelopeSpy = vi.spyOn(client, 'sendEnvelope'); + + // Add some logs + _INTERNAL_captureLog({ message: 'test1', level: 'info' }, client); + _INTERNAL_captureLog({ message: 'test2', level: 'info' }, client); + + // Trigger flush event + client.emit('flush'); + + expect(sendEnvelopeSpy).toHaveBeenCalledTimes(1); + expect(client['_logWeight']).toBe(0); // Weight should be reset after flush + }); }); }); From a1bdb3c0c3bc4bdb9a8093bacce8958e6143b451 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 7 May 2025 12:29:01 -0400 Subject: [PATCH 3/3] meta(changelog): Update changelog for 9.16.1 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a4ca5eec07f..eeca8aeaea1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 9.16.1 + +- fix(core): Make sure logs get flushed in server-runtime-client ([#16222](https://github.com/getsentry/sentry-javascript/pull/16222)) +- ref(node): Remove vercel flushing code that does nothing ([#16217](https://github.com/getsentry/sentry-javascript/pull/16217)) + ## 9.16.0 ### Important changes