From 1b83a368d50b4b2aceb4c0f010e89c91cb79ac10 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 7 Aug 2024 13:10:38 +0100 Subject: [PATCH 01/10] feat: Add spotlightBrowser integration --- .vscode/settings.json | 5 +- .../src/integrations-bundle/index.debug.ts | 1 + .../browser/src/integrations/spotlight.ts | 100 ++++++++++++++++++ packages/browser/src/sdk.ts | 3 + packages/core/src/integration.ts | 2 +- packages/node/src/integrations/spotlight.ts | 2 + packages/types/src/options.ts | 7 ++ 7 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 packages/browser/src/integrations/spotlight.ts diff --git a/.vscode/settings.json b/.vscode/settings.json index 1a8f9ce92cfc..615ca5b24472 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -36,10 +36,11 @@ ], "deno.enablePaths": ["packages/deno/test"], "editor.codeActionsOnSave": { - "source.organizeImports.biome": "explicit", + "source.organizeImports.biome": "explicit" }, "editor.defaultFormatter": "biomejs.biome", "[typescript]": { "editor.defaultFormatter": "biomejs.biome" - } + }, + "cSpell.words": ["arrayify"] } diff --git a/packages/browser/src/integrations-bundle/index.debug.ts b/packages/browser/src/integrations-bundle/index.debug.ts index 39e8920e381f..2aaa53377c88 100644 --- a/packages/browser/src/integrations-bundle/index.debug.ts +++ b/packages/browser/src/integrations-bundle/index.debug.ts @@ -1 +1,2 @@ export { debugIntegration } from '@sentry/core'; +export { spotlightBrowser } from '../integrations/spotlight'; diff --git a/packages/browser/src/integrations/spotlight.ts b/packages/browser/src/integrations/spotlight.ts new file mode 100644 index 000000000000..42be00041704 --- /dev/null +++ b/packages/browser/src/integrations/spotlight.ts @@ -0,0 +1,100 @@ +import { getNativeImplementation } from '@sentry-internal/browser-utils'; +import { defineIntegration } from '@sentry/core'; +import type { Client, Envelope, Event, IntegrationFn } from '@sentry/types'; +import { logger, serializeEnvelope } from '@sentry/utils'; +import type { WINDOW } from '../helpers'; + +import { DEBUG_BUILD } from '../debug-build'; + +export type SpotlightConnectionOptions = { + /** + * Set this if the Spotlight Sidecar is not running on localhost:8969 + * By default, the Url is set to http://localhost:8969/stream + */ + sidecarUrl?: string; +}; + +export const INTEGRATION_NAME = 'SpotlightBrowser'; + +const _spotlightIntegration = ((options: Partial = {}) => { + const sidecarUrl = options.sidecarUrl || 'http://localhost:8969/stream'; + + return { + name: INTEGRATION_NAME, + setupOnce: () => { + /* Empty function to ensure compatibility w/ JS SDK v7 >= 7.99.0 */ + }, + setup: () => { + DEBUG_BUILD && logger.log('Using Sidecar URL', sidecarUrl); + }, + processEvent: event => { + // We don't want to send interaction transactions/root spans created from + // clicks within Spotlight to Sentry. Neither do we want them to be sent to + // spotlight. + if (isSpotlightInteraction(event)) { + return null; + } + + if (event.type || !event.exception || !event.exception.values) { + return event; + } + + return event; + }, + afterAllSetup: (client: Client) => { + setupSidecarForwarding(client, sidecarUrl); + }, + }; +}) satisfies IntegrationFn; + +function setupSidecarForwarding(client: Client, sidecarUrl: string): void { + const makeFetch: typeof WINDOW.fetch | undefined = getNativeImplementation('fetch'); + let failCount = 0; + + client.on('beforeEnvelope', (envelope: Envelope) => { + if (failCount > 3) { + logger.warn('[Spotlight] Disabled Sentry -> Spotlight integration due to too many failed requests:', failCount); + return; + } + + makeFetch(sidecarUrl, { + method: 'POST', + body: serializeEnvelope(envelope), + headers: { + 'Content-Type': 'application/x-sentry-envelope', + }, + mode: 'cors', + }).then( + // Reset fail count on success + () => (failCount = 0), + err => { + failCount++; + logger.error( + "Sentry SDK can't connect to Sidecar is it running? See: https://spotlightjs.com/sidecar/npx/", + err, + ); + }, + ); + }); +} + +/** + * Use this integration to send errors and transactions to Spotlight. + * + * Learn more about spotlight at https://spotlightjs.com + */ +export const spotlightBrowser = defineIntegration(_spotlightIntegration); + +/** + * Flags if the event is a transaction created from an interaction with the spotlight UI. + */ +export function isSpotlightInteraction(event: Event): boolean { + return Boolean( + event.type === 'transaction' && + event.spans && + event.contexts && + event.contexts.trace && + event.contexts.trace.op === 'ui.action.click' && + event.spans.some(({ description }) => description && description.includes('#sentry-spotlight')), + ); +} diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 1421814ae9e5..e8ec2968e968 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -162,6 +162,9 @@ export function init(browserOptions: BrowserOptions = {}): Client | undefined { return; } + if (options.spotlight) { + logger.error('You cannot use `spotlight: true` in the browser SDKs. Please use SpotlightBrowser() integration.'); + } if (DEBUG_BUILD) { if (!supportsFetch()) { logger.warn( diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 80a539bbe3d7..500b717c3487 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -19,7 +19,7 @@ export type IntegrationIndex = { /** * Remove duplicates from the given array, preferring the last instance of any duplicate. Not guaranteed to - * preseve the order of integrations in the array. + * preserve the order of integrations in the array. * * @private */ diff --git a/packages/node/src/integrations/spotlight.ts b/packages/node/src/integrations/spotlight.ts index bfb9559958f9..019b1408bff7 100644 --- a/packages/node/src/integrations/spotlight.ts +++ b/packages/node/src/integrations/spotlight.ts @@ -72,6 +72,8 @@ function connectToSpotlight(client: Client, options: Required { // Drain socket + // Reset failed requests counter on success + failedRequests = 0; }); res.setEncoding('utf8'); }, diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 5179c1fdb70e..87d927127722 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -211,6 +211,13 @@ export interface ClientOptions Date: Wed, 7 Aug 2024 15:27:28 +0100 Subject: [PATCH 02/10] rename to sentryBrowserIntegration --- packages/browser/src/integrations-bundle/index.debug.ts | 2 +- packages/browser/src/integrations/spotlight.ts | 2 +- packages/browser/src/sdk.ts | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/browser/src/integrations-bundle/index.debug.ts b/packages/browser/src/integrations-bundle/index.debug.ts index 2aaa53377c88..c6da394f3a13 100644 --- a/packages/browser/src/integrations-bundle/index.debug.ts +++ b/packages/browser/src/integrations-bundle/index.debug.ts @@ -1,2 +1,2 @@ export { debugIntegration } from '@sentry/core'; -export { spotlightBrowser } from '../integrations/spotlight'; +export { spotlightBrowserIntegration } from '../integrations/spotlight'; diff --git a/packages/browser/src/integrations/spotlight.ts b/packages/browser/src/integrations/spotlight.ts index 42be00041704..6a8d98d1d9e0 100644 --- a/packages/browser/src/integrations/spotlight.ts +++ b/packages/browser/src/integrations/spotlight.ts @@ -83,7 +83,7 @@ function setupSidecarForwarding(client: Client, sidecarUrl: string): void { * * Learn more about spotlight at https://spotlightjs.com */ -export const spotlightBrowser = defineIntegration(_spotlightIntegration); +export const spotlightBrowserIntegration = defineIntegration(_spotlightIntegration); /** * Flags if the event is a transaction created from an interaction with the spotlight UI. diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index e8ec2968e968..a6078141f4c6 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -163,7 +163,9 @@ export function init(browserOptions: BrowserOptions = {}): Client | undefined { } if (options.spotlight) { - logger.error('You cannot use `spotlight: true` in the browser SDKs. Please use SpotlightBrowser() integration.'); + logger.error( + 'You cannot use `spotlight: true` in the browser SDKs. Please use spotlightBrowserIntegration() in your Sentry.init() call.', + ); } if (DEBUG_BUILD) { if (!supportsFetch()) { From 0deb983e1c1820fc65cfc93bd3063b5fe3d415d4 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 7 Aug 2024 15:27:48 +0100 Subject: [PATCH 03/10] remove obsolete setupOnce() method --- packages/browser/src/integrations/spotlight.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/browser/src/integrations/spotlight.ts b/packages/browser/src/integrations/spotlight.ts index 6a8d98d1d9e0..83bebebd67dd 100644 --- a/packages/browser/src/integrations/spotlight.ts +++ b/packages/browser/src/integrations/spotlight.ts @@ -21,9 +21,6 @@ const _spotlightIntegration = ((options: Partial = { return { name: INTEGRATION_NAME, - setupOnce: () => { - /* Empty function to ensure compatibility w/ JS SDK v7 >= 7.99.0 */ - }, setup: () => { DEBUG_BUILD && logger.log('Using Sidecar URL', sidecarUrl); }, From 2954e41c8f429fdf0a6e98cb35908622612fbaa8 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 7 Aug 2024 15:31:20 +0100 Subject: [PATCH 04/10] remove obsolete lines from processEvent --- packages/browser/src/integrations/spotlight.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/packages/browser/src/integrations/spotlight.ts b/packages/browser/src/integrations/spotlight.ts index 83bebebd67dd..1fa106bdcdb8 100644 --- a/packages/browser/src/integrations/spotlight.ts +++ b/packages/browser/src/integrations/spotlight.ts @@ -24,20 +24,10 @@ const _spotlightIntegration = ((options: Partial = { setup: () => { DEBUG_BUILD && logger.log('Using Sidecar URL', sidecarUrl); }, - processEvent: event => { - // We don't want to send interaction transactions/root spans created from - // clicks within Spotlight to Sentry. Neither do we want them to be sent to - // spotlight. - if (isSpotlightInteraction(event)) { - return null; - } - - if (event.type || !event.exception || !event.exception.values) { - return event; - } - - return event; - }, + // We don't want to send interaction transactions/root spans created from + // clicks within Spotlight to Sentry. Neither do we want them to be sent to + // spotlight. + processEvent: event => (isSpotlightInteraction(event) ? null : event), afterAllSetup: (client: Client) => { setupSidecarForwarding(client, sidecarUrl); }, From f88efc58aebaea6a1dbb3b0f148b827d3449aac1 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 8 Aug 2024 11:08:41 +0100 Subject: [PATCH 05/10] revert spotlight:true related changes --- packages/browser/src/sdk.ts | 5 ----- packages/types/src/options.ts | 7 ------- 2 files changed, 12 deletions(-) diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 20ae304fb696..04aa82b5f0e6 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -170,11 +170,6 @@ export function init(browserOptions: BrowserOptions = {}): Client | undefined { return; } - if (options.spotlight) { - logger.error( - 'You cannot use `spotlight: true` in the browser SDKs. Please use spotlightBrowserIntegration() in your Sentry.init() call.', - ); - } if (DEBUG_BUILD) { if (!supportsFetch()) { logger.warn( diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 87d927127722..5179c1fdb70e 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -211,13 +211,6 @@ export interface ClientOptions Date: Thu, 8 Aug 2024 13:37:11 +0100 Subject: [PATCH 06/10] fix spotlight browser init when no DSN was set --- packages/core/src/baseclient.ts | 10 +++++- packages/node/src/integrations/spotlight.ts | 2 +- packages/node/src/sdk/index.ts | 34 +++++++-------------- packages/types/src/options.ts | 12 ++++++++ 4 files changed, 33 insertions(+), 25 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 64410360e51d..c7a26f45ab70 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -311,7 +311,15 @@ export abstract class BaseClient implements Client { /** @inheritdoc */ public init(): void { - if (this._isEnabled()) { + if ( + this._isEnabled() || + // Force integrations to be setup even if no DSN was set when we have + // Spotlight enabled. This is particularly important for browser as we + // don't support the `spotlight` option there and rely on the users + // adding the `spotlightBrowserIntegration()` to their integrations which + // wouldn't get initialized with the check below when there's no DSN set. + this._options.integrations.some(({ name }) => name.startsWith('Spotlight')) + ) { this._setupIntegrations(); } } diff --git a/packages/node/src/integrations/spotlight.ts b/packages/node/src/integrations/spotlight.ts index 019b1408bff7..aa2b16b0d9d1 100644 --- a/packages/node/src/integrations/spotlight.ts +++ b/packages/node/src/integrations/spotlight.ts @@ -11,7 +11,7 @@ type SpotlightConnectionOptions = { sidecarUrl?: string; }; -const INTEGRATION_NAME = 'Spotlight'; +export const INTEGRATION_NAME = 'Spotlight'; const _spotlightIntegration = ((options: Partial = {}) => { const _options = { diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index cab3ac8274d1..d294be1df572 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -17,7 +17,7 @@ import { setOpenTelemetryContextAsyncContextStrategy, setupEventContextTrace, } from '@sentry/opentelemetry'; -import type { Client, Integration, Options } from '@sentry/types'; +import type { Integration, Options } from '@sentry/types'; import { consoleSandbox, dropUndefinedKeys, @@ -36,7 +36,7 @@ import { modulesIntegration } from '../integrations/modules'; import { nativeNodeFetchIntegration } from '../integrations/node-fetch'; import { onUncaughtExceptionIntegration } from '../integrations/onuncaughtexception'; import { onUnhandledRejectionIntegration } from '../integrations/onunhandledrejection'; -import { spotlightIntegration } from '../integrations/spotlight'; +import { INTEGRATION_NAME as SPOTLIGHT_INTEGRATION_NAME, spotlightIntegration } from '../integrations/spotlight'; import { getAutoPerformanceIntegrations } from '../integrations/tracing'; import { makeNodeTransport } from '../transports'; import type { NodeClientOptions, NodeOptions } from '../types'; @@ -140,13 +140,19 @@ function _init( const scope = getCurrentScope(); scope.update(options.initialScope); + if (options.spotlight && options.integrations.some(({ name }) => name !== SPOTLIGHT_INTEGRATION_NAME)) { + options.integrations.push( + spotlightIntegration({ + sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined, + }), + ); + } + const client = new NodeClient(options); // The client is on the current scope, from where it generally is inherited getCurrentScope().setClient(client); - if (isEnabled(client)) { - client.init(); - } + client.init(); logger.log(`Running in ${isCjs() ? 'CommonJS' : 'ESM'} mode.`); @@ -158,20 +164,6 @@ function _init( updateScopeFromEnvVariables(); - if (options.spotlight) { - // force integrations to be setup even if no DSN was set - // If they have already been added before, they will be ignored anyhow - const integrations = client.getOptions().integrations; - for (const integration of integrations) { - client.addIntegration(integration); - } - client.addIntegration( - spotlightIntegration({ - sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined, - }), - ); - } - // If users opt-out of this, they _have_ to set up OpenTelemetry themselves // There is no way to use this SDK without OpenTelemetry! if (!options.skipOpenTelemetrySetup) { @@ -336,7 +328,3 @@ function startSessionTracking(): void { } }); } - -function isEnabled(client: Client): boolean { - return client.getOptions().enabled !== false && client.getTransport() !== undefined; -} diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 5179c1fdb70e..2c668593a822 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -211,6 +211,18 @@ export interface ClientOptions Date: Thu, 8 Aug 2024 14:01:57 +0100 Subject: [PATCH 07/10] bump size limit for next.js client by 30 bytes --- .size-limit.js | 2 +- packages/types/src/options.ts | 12 ------------ 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index 437e466a89e1..6369aa49e3e9 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -193,7 +193,7 @@ module.exports = [ import: createImport('init'), ignore: ['next/router', 'next/constants'], gzip: true, - limit: '38 KB', + limit: '38.03 KB', }, // SvelteKit SDK (ESM) { diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 2c668593a822..5179c1fdb70e 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -211,18 +211,6 @@ export interface ClientOptions Date: Thu, 8 Aug 2024 14:29:47 +0100 Subject: [PATCH 08/10] try to fix node 14 integration tests --- packages/node/src/integrations/spotlight.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/node/src/integrations/spotlight.ts b/packages/node/src/integrations/spotlight.ts index aa2b16b0d9d1..1021827312be 100644 --- a/packages/node/src/integrations/spotlight.ts +++ b/packages/node/src/integrations/spotlight.ts @@ -66,14 +66,16 @@ function connectToSpotlight(client: Client, options: Required { + if (res.statusCode && res.statusCode >= 200 && res.statusCode < 400) { + // Reset failed requests counter on success + failedRequests = 0; + } res.on('data', () => { // Drain socket }); res.on('end', () => { // Drain socket - // Reset failed requests counter on success - failedRequests = 0; }); res.setEncoding('utf8'); }, From ff233c781d6aee3a4f78b054fdc7559fd4c5b519 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 8 Aug 2024 14:47:17 +0100 Subject: [PATCH 09/10] align SpotlightBrowser fail reset with node version --- packages/browser/src/integrations/spotlight.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/browser/src/integrations/spotlight.ts b/packages/browser/src/integrations/spotlight.ts index 1fa106bdcdb8..75ed18e7f34d 100644 --- a/packages/browser/src/integrations/spotlight.ts +++ b/packages/browser/src/integrations/spotlight.ts @@ -52,8 +52,12 @@ function setupSidecarForwarding(client: Client, sidecarUrl: string): void { }, mode: 'cors', }).then( - // Reset fail count on success - () => (failCount = 0), + res => { + if (res.status >= 200 && res.status < 400) { + // Reset failed requests counter on success + failCount = 0; + } + }, err => { failCount++; logger.error( From f89533f47ddaca4eb4309d957a3ea49de4a7af61 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 9 Aug 2024 18:17:02 +0100 Subject: [PATCH 10/10] fix logic error --- packages/node/src/sdk/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index d294be1df572..1a20458802a0 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -140,7 +140,7 @@ function _init( const scope = getCurrentScope(); scope.update(options.initialScope); - if (options.spotlight && options.integrations.some(({ name }) => name !== SPOTLIGHT_INTEGRATION_NAME)) { + if (options.spotlight && !options.integrations.some(({ name }) => name === SPOTLIGHT_INTEGRATION_NAME)) { options.integrations.push( spotlightIntegration({ sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined,