From 7b4a79bde7c76e3099be95ac1163527ced6ca532 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Tue, 21 Nov 2023 22:46:06 +0100 Subject: [PATCH 1/7] feat: Add spotlight option to Node SDK --- packages/node/src/sdk.ts | 29 +++++++++++++++++++++++++++++ packages/node/src/types.ts | 6 ++++++ 2 files changed, 35 insertions(+) diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index a01cdfbcae23..ca74b133a630 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -10,7 +10,9 @@ import type { SessionStatus, StackParser } from '@sentry/types'; import { createStackParser, GLOBAL_OBJ, + logger, nodeStackLineParser, + serializeEnvelope, stackParserFromStackParserOptions, tracingContextFromHeaders, } from '@sentry/utils'; @@ -179,6 +181,33 @@ export function init(options: NodeOptions = {}): void { } updateScopeFromEnvVariables(); + + connectToSpotlight(options); +} + +function connectToSpotlight(options: NodeOptions): void { + if (!options.spotlight) { + return; + } + + const spotlightUrl = typeof options.spotlight === 'string' ? options.spotlight : 'http://localhost:8969/stream'; + + const client = getCurrentHub().getClient(); + if (client) { + client.setupIntegrations(true); + client.on('beforeEnvelope', envelope => { + fetch(spotlightUrl, { + method: 'POST', + body: serializeEnvelope(envelope), + headers: { + 'Content-Type': 'application/x-sentry-envelope', + }, + mode: 'cors', + }).catch(() => { + logger.warn('[Spotlight] Failed to send envelope to Spotlight Sidecar'); + }); + }); + } } /** diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index b63c49e440e7..d73c60324668 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -79,6 +79,12 @@ export interface BaseNodeOptions { /** Callback that is executed when a fatal global error occurs. */ onFatalError?(this: void, error: Error): void; + + /** + * The SDK tries to relay all events to Spotlight in development. + * Either set it to true, or provide a specific Spotlight URL. + */ + spotlight?: boolean | string; } /** From 7ec688f7060143719d98436ec4b81f7094c6ee3b Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Tue, 21 Nov 2023 23:05:02 +0100 Subject: [PATCH 2/7] ref: Add safeguard too many tries --- packages/node/src/sdk.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index ca74b133a630..760daeca9b6d 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -195,7 +195,12 @@ function connectToSpotlight(options: NodeOptions): void { const client = getCurrentHub().getClient(); if (client) { client.setupIntegrations(true); + let tries = 0; client.on('beforeEnvelope', envelope => { + if (tries > 3) { + logger.warn('[Spotlight] Disabled Sentry -> Spotlight integration due to too many failed requests'); + return; + } fetch(spotlightUrl, { method: 'POST', body: serializeEnvelope(envelope), @@ -204,6 +209,7 @@ function connectToSpotlight(options: NodeOptions): void { }, mode: 'cors', }).catch(() => { + tries++; logger.warn('[Spotlight] Failed to send envelope to Spotlight Sidecar'); }); }); From 06e427afa5918c60593e698510e76809cbd9ad53 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Nov 2023 11:10:51 +0100 Subject: [PATCH 3/7] use integration for logic and http for requests --- packages/node/src/integrations/index.ts | 1 + packages/node/src/integrations/spotlight.ts | 109 ++++++++++++++++++++ packages/node/src/sdk.ts | 44 ++------ 3 files changed, 121 insertions(+), 33 deletions(-) create mode 100644 packages/node/src/integrations/spotlight.ts diff --git a/packages/node/src/integrations/index.ts b/packages/node/src/integrations/index.ts index 62e7b58e85b2..33cb6a381281 100644 --- a/packages/node/src/integrations/index.ts +++ b/packages/node/src/integrations/index.ts @@ -8,3 +8,4 @@ export { Context } from './context'; export { RequestData } from './requestdata'; export { LocalVariables } from './localvariables'; export { Undici } from './undici'; +export { Spotlight } from './spotlight'; diff --git a/packages/node/src/integrations/spotlight.ts b/packages/node/src/integrations/spotlight.ts new file mode 100644 index 000000000000..9c77620bccaa --- /dev/null +++ b/packages/node/src/integrations/spotlight.ts @@ -0,0 +1,109 @@ +import type { Client, Integration } from '@sentry/types'; +import { logger, serializeEnvelope } from '@sentry/utils'; +import * as http from 'http'; +import { URL } from 'url'; + +type SpotlightConnectionOptions = { + /** + * Set this if the Spotlight Sidecar is not running on localhost:8969 + * By default, the Url is set to http://localhost:8969 + */ + sidecarUrl?: string; +}; + +/** + * Use this integration to send errors and transactions to Spotlight. + * + * Learn more about spotlight at https://spotlightjs.com + * + * Important: This integration only works with Node 18 or newer + * + * @param options + * @returns + */ +export class Spotlight implements Integration { + public name = 'Spotlight'; + + private readonly _options: Required; + + public constructor(options: SpotlightConnectionOptions) { + this._options = { + sidecarUrl: options.sidecarUrl || 'http://localhost:8969', + }; + } + + /** + * JSDoc + */ + public setupOnce(): void { + // empty but otherwise TS complains + } + + /** + * Sets up forwarding envelopes to the Spotlight Sidecar + */ + public setup(client: Client): void { + connectToSpotlight(client, this._options); + } +} + +function connectToSpotlight(client: Client, options: Required): void { + const spotlightUrl = parseSidecarUrl(options.sidecarUrl); + if (!spotlightUrl) { + return; + } + + let failedRequests = 0; + + if (typeof client.on !== 'function') { + logger.warn('[Spotlight] Cannot connect to spotlight due to missing method on SDK client (`client.on`)'); + return; + } + + client.on('beforeEnvelope', envelope => { + if (failedRequests > 3) { + logger.warn('[Spotlight] Disabled Sentry -> Spotlight integration due to too many failed requests'); + return; + } + + const serializedEnvelope = serializeEnvelope(envelope); + + const req = http.request( + { + method: 'POST', + path: '/stream', + hostname: spotlightUrl.hostname, + port: spotlightUrl.port, + headers: { + 'Content-Type': 'application/x-sentry-envelope', + }, + }, + res => { + res.on('data', () => { + // Drain socket + }); + + res.on('end', () => { + // Drain socket + }); + res.setEncoding('utf8'); + }, + ); + + req.on('error', () => { + failedRequests++; + logger.warn('[Spotlight] Failed to send envelope to Spotlight Sidecar'); + }); + req.write(serializedEnvelope); + req.end(); + }); +} + +function parseSidecarUrl(url: string): URL | undefined { + try { + return new URL(`${url}/stream`); + } catch { + logger.warn(`[Spotlight] Invalid sidecar URL: ${url}`); + return undefined; + } +} diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 760daeca9b6d..41b70e43a6dd 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -1,5 +1,6 @@ /* eslint-disable max-lines */ import { + addIntegration, getCurrentHub, getIntegrationsToSetup, getMainCarrier, @@ -10,9 +11,7 @@ import type { SessionStatus, StackParser } from '@sentry/types'; import { createStackParser, GLOBAL_OBJ, - logger, nodeStackLineParser, - serializeEnvelope, stackParserFromStackParserOptions, tracingContextFromHeaders, } from '@sentry/utils'; @@ -30,6 +29,7 @@ import { OnUncaughtException, OnUnhandledRejection, RequestData, + Spotlight, Undici, } from './integrations'; import { getModuleFromFilename } from './module'; @@ -182,37 +182,15 @@ export function init(options: NodeOptions = {}): void { updateScopeFromEnvVariables(); - connectToSpotlight(options); -} - -function connectToSpotlight(options: NodeOptions): void { - if (!options.spotlight) { - return; - } - - const spotlightUrl = typeof options.spotlight === 'string' ? options.spotlight : 'http://localhost:8969/stream'; - - const client = getCurrentHub().getClient(); - if (client) { - client.setupIntegrations(true); - let tries = 0; - client.on('beforeEnvelope', envelope => { - if (tries > 3) { - logger.warn('[Spotlight] Disabled Sentry -> Spotlight integration due to too many failed requests'); - return; - } - fetch(spotlightUrl, { - method: 'POST', - body: serializeEnvelope(envelope), - headers: { - 'Content-Type': 'application/x-sentry-envelope', - }, - mode: 'cors', - }).catch(() => { - tries++; - logger.warn('[Spotlight] Failed to send envelope to Spotlight Sidecar'); - }); - }); + if (options.spotlight) { + const client = getCurrentHub().getClient(); + if (client && client.addIntegration) { + // force integrations to be setup even if no DSN was set + client.setupIntegrations(true); + client.addIntegration( + new Spotlight({ sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined }), + ); + } } } From 760bf80cbb11e504cd1b26cf919d0a115e11969b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Nov 2023 13:05:46 +0100 Subject: [PATCH 4/7] add basic tests --- packages/node/src/integrations/spotlight.ts | 7 +- packages/node/src/sdk.ts | 1 - packages/node/src/types.ts | 18 ++- .../node/test/integrations/spotlight.test.ts | 120 ++++++++++++++++++ 4 files changed, 137 insertions(+), 9 deletions(-) create mode 100644 packages/node/test/integrations/spotlight.test.ts diff --git a/packages/node/src/integrations/spotlight.ts b/packages/node/src/integrations/spotlight.ts index 9c77620bccaa..561234f2bcee 100644 --- a/packages/node/src/integrations/spotlight.ts +++ b/packages/node/src/integrations/spotlight.ts @@ -26,9 +26,9 @@ export class Spotlight implements Integration { private readonly _options: Required; - public constructor(options: SpotlightConnectionOptions) { + public constructor(options?: SpotlightConnectionOptions) { this._options = { - sidecarUrl: options.sidecarUrl || 'http://localhost:8969', + sidecarUrl: options?.sidecarUrl || 'http://localhost:8969', }; } @@ -43,6 +43,9 @@ export class Spotlight implements Integration { * Sets up forwarding envelopes to the Spotlight Sidecar */ public setup(client: Client): void { + if (process.env.NODE_ENV !== 'development') { + logger.warn("[Spotlight] It seems you're not in dev mode. Do you really want to have Spoltight enabled?"); + } connectToSpotlight(client, this._options); } } diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 41b70e43a6dd..af4ce7905fda 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -1,6 +1,5 @@ /* eslint-disable max-lines */ import { - addIntegration, getCurrentHub, getIntegrationsToSetup, getMainCarrier, diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index d73c60324668..39d7eff27278 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -59,6 +59,18 @@ export interface BaseNodeOptions { * */ clientClass?: typeof NodeClient; + /** + * If you use Spotlight by Sentry during development, use + * this option to forward captured Sentry events to Spotlight. + * + * Either set it to true, or provide a specific Spotlight Sidecar URL. + * + * More details: https://spotlightjs.com/ + * + * IMPORTANT: Only set this optoin to `true` while developing, not in production! + */ + spotlight?: boolean | string; + // TODO (v8): Remove this in v8 /** * @deprecated Moved to constructor options of the `Http` and `Undici` integration. @@ -79,12 +91,6 @@ export interface BaseNodeOptions { /** Callback that is executed when a fatal global error occurs. */ onFatalError?(this: void, error: Error): void; - - /** - * The SDK tries to relay all events to Spotlight in development. - * Either set it to true, or provide a specific Spotlight URL. - */ - spotlight?: boolean | string; } /** diff --git a/packages/node/test/integrations/spotlight.test.ts b/packages/node/test/integrations/spotlight.test.ts new file mode 100644 index 000000000000..228f4151dbfe --- /dev/null +++ b/packages/node/test/integrations/spotlight.test.ts @@ -0,0 +1,120 @@ +import type { Envelope, EventEnvelope } from '@sentry/types'; +import { createEnvelope, logger } from '@sentry/utils'; +import * as http from 'http'; + +import { NodeClient } from '../../src'; +import { Spotlight } from '../../src/integrations'; +import { getDefaultNodeClientOptions } from '../helper/node-client-options'; + +describe('Spotlight', () => { + const loggerSpy = jest.spyOn(logger, 'warn'); + + afterEach(() => { + loggerSpy.mockClear(); + jest.clearAllMocks(); + }); + + const options = getDefaultNodeClientOptions(); + const client = new NodeClient(options); + + it('has a name', () => { + const integration = new Spotlight(); + expect(integration.name).toEqual('Spotlight'); + }); + + it('registers a callback on the `beforeEnvelope` hook', () => { + const clientWithSpy = { + ...client, + on: jest.fn(), + }; + const integration = new Spotlight(); + // @ts-expect-error - this is fine in tests + integration.setup(clientWithSpy); + expect(clientWithSpy.on).toHaveBeenCalledWith('beforeEnvelope', expect.any(Function)); + }); + + it('sends an envelope POST request to the sidecar url', () => { + const httpSpy = jest.spyOn(http, 'request').mockImplementationOnce(() => { + return { + on: jest.fn(), + write: jest.fn(), + end: jest.fn(), + } as any; + }); + + let callback: (envelope: Envelope) => void = () => {}; + const clientWithSpy = { + ...client, + on: jest.fn().mockImplementationOnce((_, cb) => (callback = cb)), + }; + + const integration = new Spotlight(); + // @ts-expect-error - this is fine in tests + integration.setup(clientWithSpy); + + const envelope = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ + [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }], + ]); + + callback(envelope); + + expect(httpSpy).toHaveBeenCalledWith( + { + headers: { + 'Content-Type': 'application/x-sentry-envelope', + }, + hostname: 'localhost', + method: 'POST', + path: '/stream', + port: '8969', + }, + expect.any(Function), + ); + }); + + describe('no-ops if', () => { + it('an invalid URL is passed', () => { + const integration = new Spotlight({ sidecarUrl: 'invalid-url' }); + integration.setup(client); + expect(loggerSpy).toHaveBeenCalledWith(expect.stringContaining('Invalid sidecar URL: invalid-url')); + }); + + it("the client doesn't support life cycle hooks", () => { + const integration = new Spotlight({ sidecarUrl: 'http://mylocalhost:8969' }); + const clientWithoutHooks = { ...client }; + // @ts-expect-error - this is fine in tests + delete client.on; + // @ts-expect-error - this is fine in tests + integration.setup(clientWithoutHooks); + expect(loggerSpy).toHaveBeenCalledWith(expect.stringContaining(' missing method on SDK client (`client.on`)')); + }); + }); + + it('warns if the NODE_ENV variable doesn\'t equal "development"', () => { + const oldEnvValue = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; + + const integration = new Spotlight({ sidecarUrl: 'http://localhost:8969' }); + integration.setup(client); + + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining("It seems you're not in dev mode. Do you really want to have Spoltight enabled?"), + ); + + process.env.NODE_ENV = oldEnvValue; + }); + + it('doesn\'t warn if the NODE_ENV variable equals "development"', () => { + const oldEnvValue = process.env.NODE_ENV; + process.env.NODE_ENV = 'development'; + + const integration = new Spotlight({ sidecarUrl: 'http://localhost:8969' }); + integration.setup(client); + + expect(loggerSpy).not.toHaveBeenCalledWith( + expect.stringContaining("It seems you're not in dev mode. Do you really want to have Spoltight enabled?"), + ); + + process.env.NODE_ENV = oldEnvValue; + }); +}); From ea7c8ed3019725411bbe6fa8c4531b4bb08f97b3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Nov 2023 13:14:42 +0100 Subject: [PATCH 5/7] id --- packages/node/src/integrations/spotlight.ts | 3 ++- packages/node/test/integrations/spotlight.test.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/node/src/integrations/spotlight.ts b/packages/node/src/integrations/spotlight.ts index 561234f2bcee..bcece0055145 100644 --- a/packages/node/src/integrations/spotlight.ts +++ b/packages/node/src/integrations/spotlight.ts @@ -22,7 +22,8 @@ type SpotlightConnectionOptions = { * @returns */ export class Spotlight implements Integration { - public name = 'Spotlight'; + public static id = 'Spotlight'; + public name = Spotlight.id; private readonly _options: Required; diff --git a/packages/node/test/integrations/spotlight.test.ts b/packages/node/test/integrations/spotlight.test.ts index 228f4151dbfe..f03b3fb31713 100644 --- a/packages/node/test/integrations/spotlight.test.ts +++ b/packages/node/test/integrations/spotlight.test.ts @@ -17,9 +17,10 @@ describe('Spotlight', () => { const options = getDefaultNodeClientOptions(); const client = new NodeClient(options); - it('has a name', () => { + it('has a name and id', () => { const integration = new Spotlight(); expect(integration.name).toEqual('Spotlight'); + expect(Spotlight.id).toEqual('Spotlight'); }); it('registers a callback on the `beforeEnvelope` hook', () => { From ad44031bad9852563c16950be2c574fcf3e3d06b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Nov 2023 14:02:37 +0100 Subject: [PATCH 6/7] Update packages/node/src/types.ts --- packages/node/src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index 39d7eff27278..01f91fb46cbe 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -67,7 +67,7 @@ export interface BaseNodeOptions { * * More details: https://spotlightjs.com/ * - * IMPORTANT: Only set this optoin to `true` while developing, not in production! + * IMPORTANT: Only set this option to `true` while developing, not in production! */ spotlight?: boolean | string; From 9cf1ea8892762a7766ad26d2ea6b858822039b24 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 22 Nov 2023 14:03:04 +0100 Subject: [PATCH 7/7] Update packages/node/src/integrations/spotlight.ts --- packages/node/src/integrations/spotlight.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/node/src/integrations/spotlight.ts b/packages/node/src/integrations/spotlight.ts index bcece0055145..4b4b9d907721 100644 --- a/packages/node/src/integrations/spotlight.ts +++ b/packages/node/src/integrations/spotlight.ts @@ -17,9 +17,6 @@ type SpotlightConnectionOptions = { * Learn more about spotlight at https://spotlightjs.com * * Important: This integration only works with Node 18 or newer - * - * @param options - * @returns */ export class Spotlight implements Integration { public static id = 'Spotlight';