Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

## Unreleased

- fix(node): Fix Spotlight configuration precedence to match specification (#18195)
- feat(browser): Add environment variable support for Spotlight configuration ([#18198](https://github.com/getsentry/sentry-javascript/pull/18198))
- `SENTRY_SPOTLIGHT`, `PUBLIC_SENTRY_SPOTLIGHT`, `NEXT_PUBLIC_SENTRY_SPOTLIGHT`, `VITE_SENTRY_SPOTLIGHT`, `NUXT_PUBLIC_SENTRY_SPOTLIGHT`, `REACT_APP_SENTRY_SPOTLIGHT`, `VUE_APP_SENTRY_SPOTLIGHT`, and `GATSBY_SENTRY_SPOTLIGHT`
- fix(node): Fix Spotlight configuration precedence to match specification ([#18195](https://github.com/getsentry/sentry-javascript/pull/18195))

## 10.25.0

Expand Down
3 changes: 1 addition & 2 deletions packages/aws-serverless/src/init.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import type { Integration, Options } from '@sentry/core';
import { applySdkMetadata, debug, getSDKSource } from '@sentry/core';
import { applySdkMetadata, debug, envToBool, getSDKSource } from '@sentry/core';
import type { NodeClient, NodeOptions } from '@sentry/node';
import { getDefaultIntegrationsWithoutPerformance, initWithoutDefaultIntegrations } from '@sentry/node';
import { envToBool } from '@sentry/node-core';
import { DEBUG_BUILD } from './debug-build';
import { awsIntegration } from './integration/aws';
import { awsLambdaIntegration } from './integration/awslambda';
Expand Down
20 changes: 20 additions & 0 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,29 @@ type BrowserSpecificOptions = BrowserClientReplayOptions &
*
* Either set it to true, or provide a specific Spotlight Sidecar URL.
*
* Alternatively, you can configure Spotlight using environment variables (checked in this order):
* - PUBLIC_SENTRY_SPOTLIGHT (SvelteKit, Astro, Qwik)
* - NEXT_PUBLIC_SENTRY_SPOTLIGHT (Next.js)
* - VITE_SENTRY_SPOTLIGHT (Vite)
* - NUXT_PUBLIC_SENTRY_SPOTLIGHT (Nuxt)
* - REACT_APP_SENTRY_SPOTLIGHT (Create React App)
* - VUE_APP_SENTRY_SPOTLIGHT (Vue CLI)
* - GATSBY_SENTRY_SPOTLIGHT (Gatsby)
* - SENTRY_SPOTLIGHT (fallback for non-framework setups)
*
* Framework-specific vars have higher priority to support Docker Compose setups where
* backend uses SENTRY_SPOTLIGHT with Docker hostnames while frontend needs localhost.
*
* Precedence rules:
* - If this option is `false`, Spotlight is disabled (env vars ignored)
* - If this option is a string URL, that URL is used (env vars ignored)
* - If this option is `true` and env var is a URL, the env var URL is used
* - If this option is `undefined`, the env var value is used (if set)
*
* More details: https://spotlightjs.com/
*
* IMPORTANT: Only set this option to `true` while developing, not in production!
* Spotlight is automatically excluded from production bundles.
*/
spotlight?: boolean | string;
};
Expand Down
10 changes: 8 additions & 2 deletions packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getIntegrationsToSetup,
inboundFiltersIntegration,
initAndBind,
resolveSpotlightOptions,
stackParserFromStackParserOptions,
} from '@sentry/core';
import type { BrowserClientOptions, BrowserOptions } from './client';
Expand All @@ -19,6 +20,7 @@ import { spotlightBrowserIntegration } from './integrations/spotlight';
import { defaultStackParser } from './stack-parsers';
import { makeFetchTransport } from './transports/fetch';
import { checkAndWarnIfIsEmbeddedBrowserExtension } from './utils/detectBrowserExtension';
import { getSpotlightConfig } from './utils/spotlightConfig';

/** Get the default integrations for the browser SDK. */
export function getDefaultIntegrations(_options: Options): Integration[] {
Expand Down Expand Up @@ -95,11 +97,15 @@ export function init(options: BrowserOptions = {}): Client | undefined {
options.defaultIntegrations == null ? getDefaultIntegrations(options) : options.defaultIntegrations;

/* rollup-include-development-only */
if (options.spotlight) {
// Resolve Spotlight configuration with proper precedence
const envSpotlight = getSpotlightConfig();
const spotlightValue = resolveSpotlightOptions(options.spotlight, envSpotlight);

if (spotlightValue) {
if (!defaultIntegrations) {
defaultIntegrations = [];
}
const args = typeof options.spotlight === 'string' ? { sidecarUrl: options.spotlight } : undefined;
const args = typeof spotlightValue === 'string' ? { sidecarUrl: spotlightValue } : undefined;
defaultIntegrations.push(spotlightBrowserIntegration(args));
}
/* rollup-include-development-only-end */
Expand Down
27 changes: 27 additions & 0 deletions packages/browser/src/utils/env.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Safely gets an environment variable value with defensive guards for browser environments.
* Checks process.env which is transformed by most bundlers (Webpack, Vite, Rollup, Rspack, Parcel, etc.)
* at build time.
*
* Note: We don't check import.meta.env because:
* 1. Bundlers only replace static references like `import.meta.env.VITE_VAR`, not dynamic access
* 2. Dynamic access causes syntax errors in unsupported environments
* 3. Most bundlers transform process.env references anyway
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: My impression while doing some research for #18050 (comment) was what very few bundlers inject process.env into browser bundles. For example, this code will not work in Angular.
Vite instructs checking on import.meta.env, so not sure if it double-writes to process.env (my gut feeling is no).

Did you test this in the respective frameworks? Maybe I'm also misinformed and this works just fine 😅 Ideally we can add an e2e test for at least NextJS and some Vite-based framework app to be sure.

*
* @param key - The environment variable key to look up
* @returns The value of the environment variable or undefined if not found
*/
export function getEnvValue(key: string): string | undefined {
try {
if (typeof process !== 'undefined' && process.env) {
const value = process.env[key];
if (value !== undefined) {
return value;
}
}
} catch (e) {
// Silently ignore - process might not be accessible or might throw in some environments
}

return undefined;
}
63 changes: 63 additions & 0 deletions packages/browser/src/utils/spotlightConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { debug, envToBool } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
import { getEnvValue } from './env';

/**
* Environment variable keys to check for Spotlight configuration, in priority order.
* The first one found with a value will be used.
*
* IMPORTANT: Framework-specific variables (PUBLIC_*, NEXT_PUBLIC_*, etc.) are prioritized
* over the generic SENTRY_SPOTLIGHT to support Docker Compose setups where:
* - Backend services need SENTRY_SPOTLIGHT=http://host.internal.docker:8969/stream
* - Frontend code needs localhost (via framework-specific vars like NEXT_PUBLIC_SENTRY_SPOTLIGHT=http://localhost:8969/stream)
*
* SENTRY_SPOTLIGHT is kept as a fallback for:
* - Simple non-Docker setups
* - Remote Spotlight instances when no framework-specific var is set
*/
const SPOTLIGHT_ENV_KEYS = [
'PUBLIC_SENTRY_SPOTLIGHT', // SvelteKit, Astro, Qwik
'NEXT_PUBLIC_SENTRY_SPOTLIGHT', // Next.js
'VITE_SENTRY_SPOTLIGHT', // Vite
'NUXT_PUBLIC_SENTRY_SPOTLIGHT', // Nuxt
'REACT_APP_SENTRY_SPOTLIGHT', // Create React App
'VUE_APP_SENTRY_SPOTLIGHT', // Vue CLI
'GATSBY_SENTRY_SPOTLIGHT', // Gatsby
'SENTRY_SPOTLIGHT', // Fallback/base name - works in Parcel, Webpack, Rspack, Rollup, Rolldown, Node.js
] as const;

/**
* Gets the Spotlight configuration from environment variables.
* Checks multiple environment variable prefixes in priority order to support
* different bundlers and frameworks.
*
* @returns The resolved Spotlight configuration (boolean | string | undefined)
*/
export function getSpotlightConfig(): boolean | string | undefined {
for (const key of SPOTLIGHT_ENV_KEYS) {
const value = getEnvValue(key);

if (value !== undefined) {
// Try to parse as boolean first (strict mode)
const boolValue = envToBool(value, { strict: true });

if (boolValue !== null) {
// It's a valid boolean value
if (DEBUG_BUILD) {
debug.log(`[Spotlight] Found ${key}=${String(boolValue)} in environment variables`);
}
return boolValue;
}

// Not a boolean, treat as custom URL string
// Note: empty/whitespace strings are filtered by resolveSpotlightOptions
if (DEBUG_BUILD) {
debug.log(`[Spotlight] Found ${key}=${value} (custom URL) in environment variables`);
}
return value;
}
}

// No Spotlight configuration found in environment
return undefined;
Comment on lines 62 to 65
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: neat size trick: You can let the function return void and simply omit the return undefined here. Saves a couple of bytes and JS returns undefined anyway. 😅 (but feel free to ignore since this is shaken out for prod builds anyway)

}
148 changes: 148 additions & 0 deletions packages/browser/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,154 @@ describe('init', () => {
});
});

describe('Spotlight environment variable support', () => {
let originalProcess: typeof globalThis.process | undefined;

afterEach(() => {
if (originalProcess !== undefined) {
globalThis.process = originalProcess;
} else {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
delete (globalThis as any).process;
}
});

it('uses environment variable when options.spotlight is undefined', () => {
originalProcess = globalThis.process;
globalThis.process = {
env: {
SENTRY_SPOTLIGHT: 'true',
} as Record<string, string>,
} as NodeJS.Process;

// @ts-expect-error this is fine for testing
const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind').mockImplementationOnce(() => {});
const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, spotlight: undefined });
init(options);

const optionsPassed = initAndBindSpy.mock.calls[0]?.[1];
// Spotlight integration should be added
const spotlightIntegration = optionsPassed?.integrations.find((i: Integration) => i.name === 'SpotlightBrowser');
expect(spotlightIntegration).toBeDefined();
});

it('does not add Spotlight when environment variable is false', () => {
originalProcess = globalThis.process;
globalThis.process = {
env: {
SENTRY_SPOTLIGHT: 'false',
} as Record<string, string>,
} as NodeJS.Process;

// @ts-expect-error this is fine for testing
const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind').mockImplementationOnce(() => {});
const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, spotlight: undefined });
init(options);

const optionsPassed = initAndBindSpy.mock.calls[0]?.[1];
// Spotlight integration should NOT be added
const spotlightIntegration = optionsPassed?.integrations.find((i: Integration) => i.name === 'SpotlightBrowser');
expect(spotlightIntegration).toBeUndefined();
});

it('options.spotlight=false takes precedence over environment variable', () => {
originalProcess = globalThis.process;
globalThis.process = {
env: {
SENTRY_SPOTLIGHT: 'true',
} as Record<string, string>,
} as NodeJS.Process;

// @ts-expect-error this is fine for testing
const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind').mockImplementationOnce(() => {});
const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, spotlight: false });
init(options);

const optionsPassed = initAndBindSpy.mock.calls[0]?.[1];
// Spotlight integration should NOT be added even though env var is true
const spotlightIntegration = optionsPassed?.integrations.find((i: Integration) => i.name === 'SpotlightBrowser');
expect(spotlightIntegration).toBeUndefined();
});

it('options.spotlight=url takes precedence over environment variable', () => {
originalProcess = globalThis.process;
const customUrl = 'http://custom:1234/stream';
globalThis.process = {
env: {
SENTRY_SPOTLIGHT: 'http://env:5678/stream',
} as Record<string, string>,
} as NodeJS.Process;

// @ts-expect-error this is fine for testing
const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind').mockImplementationOnce(() => {});
const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, spotlight: customUrl });
init(options);

const optionsPassed = initAndBindSpy.mock.calls[0]?.[1];
// Spotlight integration should be added (we can't easily check the URL here without deeper inspection)
const spotlightIntegration = optionsPassed?.integrations.find((i: Integration) => i.name === 'SpotlightBrowser');
expect(spotlightIntegration).toBeDefined();
});

it('uses environment variable URL when options.spotlight=true', () => {
originalProcess = globalThis.process;
globalThis.process = {
env: {
SENTRY_SPOTLIGHT: 'http://env:5678/stream',
} as Record<string, string>,
} as NodeJS.Process;

// @ts-expect-error this is fine for testing
const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind').mockImplementationOnce(() => {});
const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, spotlight: true });
init(options);

const optionsPassed = initAndBindSpy.mock.calls[0]?.[1];
// Spotlight integration should be added
const spotlightIntegration = optionsPassed?.integrations.find((i: Integration) => i.name === 'SpotlightBrowser');
expect(spotlightIntegration).toBeDefined();
});

it('respects priority order: PUBLIC_SENTRY_SPOTLIGHT over SENTRY_SPOTLIGHT', () => {
originalProcess = globalThis.process;
globalThis.process = {
env: {
PUBLIC_SENTRY_SPOTLIGHT: 'true',
SENTRY_SPOTLIGHT: 'false',
} as Record<string, string>,
} as NodeJS.Process;

// @ts-expect-error this is fine for testing
const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind').mockImplementationOnce(() => {});
const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, spotlight: undefined });
init(options);

const optionsPassed = initAndBindSpy.mock.calls[0]?.[1];
// Spotlight integration should be added (PUBLIC_SENTRY_SPOTLIGHT=true wins)
const spotlightIntegration = optionsPassed?.integrations.find((i: Integration) => i.name === 'SpotlightBrowser');
expect(spotlightIntegration).toBeDefined();
});

it('uses framework-specific prefix when base is not set', () => {
originalProcess = globalThis.process;
globalThis.process = {
env: {
NEXT_PUBLIC_SENTRY_SPOTLIGHT: 'true',
} as Record<string, string>,
} as NodeJS.Process;

// @ts-expect-error this is fine for testing
const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind').mockImplementationOnce(() => {});
const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, spotlight: undefined });
init(options);

const optionsPassed = initAndBindSpy.mock.calls[0]?.[1];
// Spotlight integration should be added
const spotlightIntegration = optionsPassed?.integrations.find((i: Integration) => i.name === 'SpotlightBrowser');
expect(spotlightIntegration).toBeDefined();
});
});

it('returns a client from init', () => {
const client = init();
expect(client).not.toBeUndefined();
Expand Down
Loading
Loading