feat(node): Move default option handling from init to NodeClient

Possibly relevant changes:

* `client.startClientReportTracking()` is now called in the NodeClient constructor - so unless `sendClientReports: false` is configured, this will always be setup now, even for manual clients.
* Env. vars will automatically be picked up and put on the current scope in the NodeClient constructor
* `Failed to register ESM hook` warning is now a `console.warn`, not using the logger - this means we do not need to manually enable the logger before we call `initAndBind` anymore.
* spotlight is now properly handled like a default integration

Author: mydea

packages/core/src/index.ts

@@ -160,7 +160,7 @@ export {
   isVueViewModel,
 } from './utils-hoist/is';
 export { isBrowser } from './utils-hoist/isBrowser';
-export { CONSOLE_LEVELS, consoleSandbox, logger, originalConsoleMethods } from './utils-hoist/logger';
+export { CONSOLE_LEVELS, consoleSandbox, logger, originalConsoleMethods, enableLogger } from './utils-hoist/logger';
 export type { Logger } from './utils-hoist/logger';
 export {
   addContextToFrame,

packages/core/src/sdk.ts

@@ -1,8 +1,7 @@
 import type { Client } from './client';
 import { getCurrentScope } from './currentScopes';
-import { DEBUG_BUILD } from './debug-build';
 import type { ClientOptions } from './types-hoist/options';
-import { consoleSandbox, logger } from './utils-hoist/logger';
+import { enableLogger } from './utils-hoist/logger';
 
 /** A class object that can instantiate Client objects. */
 export type ClientClass<F extends Client, O extends ClientOptions> = new (options: O) => F;
@@ -14,25 +13,14 @@ export type ClientClass<F extends Client, O extends ClientOptions> = new (option
  * @param clientClass The client class to instantiate.
  * @param options Options to pass to the client.
  */
-export function initAndBind<F extends Client, O extends ClientOptions>(
-  clientClass: ClientClass<F, O>,
-  options: O,
-): Client {
-  if (options.debug === true) {
-    if (DEBUG_BUILD) {
-      logger.enable();
-    } else {
-      // use `console.warn` rather than `logger.warn` since by non-debug bundles have all `logger.x` statements stripped
-      consoleSandbox(() => {
-        // eslint-disable-next-line no-console
-        console.warn('[Sentry] Cannot initialize SDK with `debug` option using a non-debug bundle.');
-      });
-    }
+export function initAndBind<F extends Client, O extends ClientOptions>(ClientClass: ClientClass<F, O>, options: O): F {
+  if (options.debug) {
+    enableLogger();
   }
   const scope = getCurrentScope();
   scope.update(options.initialScope);
 
-  const client = new clientClass(options);
+  const client = new ClientClass(options);
   setCurrentClient(client);
   client.init();
   return client;

packages/core/src/utils-hoist/logger.ts

@@ -100,3 +100,16 @@ function makeLogger(): Logger {
  * The logger is a singleton on the carrier, to ensure that a consistent logger is used throughout the SDK.
  */
 export const logger = getGlobalSingleton('logger', makeLogger);
+
+/** Enables the logger, or log a warning if DEBUG_BUILD is false. */
+export function enableLogger(): void {
+  if (DEBUG_BUILD) {
+    logger.enable();
+  } else {
+    // use `console.warn` rather than `logger.warn` since by non-debug bundles have all `logger.x` statements stripped
+    consoleSandbox(() => {
+      // eslint-disable-next-line no-console
+      console.warn('[Sentry] Cannot initialize SDK with `debug` option using a non-debug bundle.');
+    });
+  }
+}

packages/node/src/sdk/client.ts

@@ -3,12 +3,15 @@ import type { Tracer } from '@opentelemetry/api';
 import { trace } from '@opentelemetry/api';
 import { registerInstrumentations } from '@opentelemetry/instrumentation';
 import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
-import type { DynamicSamplingContext, Scope, ServerRuntimeClientOptions, TraceContext } from '@sentry/core';
+import type { DynamicSamplingContext, Scope, TraceContext } from '@sentry/core';
 import { _INTERNAL_flushLogsBuffer, applySdkMetadata, logger, SDK_VERSION, ServerRuntimeClient } from '@sentry/core';
 import { getTraceContextForScope } from '@sentry/opentelemetry';
 import { isMainThread, threadId } from 'worker_threads';
 import { DEBUG_BUILD } from '../debug-build';
 import type { NodeClientOptions } from '../types';
+import { isCjs } from '../utils/commonjs';
+import { envToBool } from '../utils/envToBool';
+import { getSentryRelease } from './api';
 
 const DEFAULT_CLIENT_REPORT_FLUSH_INTERVAL_MS = 60_000; // 60s was chosen arbitrarily
 
@@ -21,15 +24,9 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
   private _logOnExitFlushListener: (() => void) | undefined;
 
   public constructor(options: NodeClientOptions) {
-    const serverName = options.serverName || global.process.env.SENTRY_NAME || os.hostname();
-    const clientOptions: ServerRuntimeClientOptions = {
-      ...options,
-      platform: 'node',
-      runtime: { name: 'node', version: global.process.version },
-      serverName,
-    };
-
-    if (options.openTelemetryInstrumentations) {
+    const clientOptions = applyDefaultOptions(options);
+
+    if (clientOptions.openTelemetryInstrumentations) {
       registerInstrumentations({
         instrumentations: options.openTelemetryInstrumentations,
       });
@@ -40,19 +37,22 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
     logger.log(
       `Initializing Sentry: process: ${process.pid}, thread: ${isMainThread ? 'main' : `worker-${threadId}`}.`,
     );
+    logger.log(`Running in ${isCjs() ? 'CommonJS' : 'ESM'} mode.`);
 
     super(clientOptions);
 
+    this.startClientReportTracking();
+
     if (this.getOptions()._experiments?.enableLogs) {
       this._logOnExitFlushListener = () => {
         _INTERNAL_flushLogsBuffer(this);
       };
 
-      if (serverName) {
+      if (clientOptions.serverName) {
         this.on('beforeCaptureLog', log => {
           log.attributes = {
             ...log.attributes,
-            'server.address': serverName,
+            'server.address': clientOptions.serverName,
           };
         });
       }
@@ -154,3 +154,55 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
     return getTraceContextForScope(this, scope);
   }
 }
+
+function applyDefaultOptions<T extends Partial<NodeClientOptions>>(options: T): T {
+  const release = getRelease(options.release);
+  const spotlight =
+    options.spotlight ?? envToBool(process.env.SENTRY_SPOTLIGHT, { strict: true }) ?? process.env.SENTRY_SPOTLIGHT;
+  const tracesSampleRate = getTracesSampleRate(options.tracesSampleRate);
+  const serverName = options.serverName || global.process.env.SENTRY_NAME || os.hostname();
+
+  return {
+    platform: 'node',
+    runtime: { name: 'node', version: global.process.version },
+    serverName,
+    ...options,
+    dsn: options.dsn ?? process.env.SENTRY_DSN,
+    environment: options.environment ?? process.env.SENTRY_ENVIRONMENT,
+    sendClientReports: options.sendClientReports ?? true,
+    release,
+    tracesSampleRate,
+    spotlight,
+    debug: envToBool(options.debug ?? process.env.SENTRY_DEBUG),
+  };
+}
+
+function getRelease(release: NodeClientOptions['release']): string | undefined {
+  if (release !== undefined) {
+    return release;
+  }
+
+  const detectedRelease = getSentryRelease();
+  if (detectedRelease !== undefined) {
+    return detectedRelease;
+  }
+
+  return undefined;
+}
+
+/**
+ * Tries to get a `tracesSampleRate`, possibly extracted from the environment variables.
+ */
+export function getTracesSampleRate(tracesSampleRate: NodeClientOptions['tracesSampleRate']): number | undefined {
+  if (tracesSampleRate !== undefined) {
+    return tracesSampleRate;
+  }
+
+  const sampleRateFromEnv = process.env.SENTRY_TRACES_SAMPLE_RATE;
+  if (!sampleRateFromEnv) {
+    return undefined;
+  }
+
+  const parsed = parseFloat(sampleRateFromEnv);
+  return isFinite(parsed) ? parsed : undefined;
+}

packages/node/src/sdk/index.ts

@@ -1,12 +1,12 @@
-import type { Integration, Options } from '@sentry/core';
+import type { Integration } from '@sentry/core';
 import {
   consoleIntegration,
-  consoleSandbox,
   functionToStringIntegration,
   getCurrentScope,
   getIntegrationsToSetup,
   hasSpansEnabled,
   inboundFiltersIntegration,
+  initAndBind,
   linkedErrorsIntegration,
   logger,
   propagationContextFromHeaders,
@@ -30,14 +30,14 @@ import { nativeNodeFetchIntegration } from '../integrations/node-fetch';
 import { onUncaughtExceptionIntegration } from '../integrations/onuncaughtexception';
 import { onUnhandledRejectionIntegration } from '../integrations/onunhandledrejection';
 import { processSessionIntegration } from '../integrations/processSession';
-import { INTEGRATION_NAME as SPOTLIGHT_INTEGRATION_NAME, spotlightIntegration } from '../integrations/spotlight';
+import { spotlightIntegration } from '../integrations/spotlight';
 import { getAutoPerformanceIntegrations } from '../integrations/tracing';
 import { makeNodeTransport } from '../transports';
 import type { NodeClientOptions, NodeOptions } from '../types';
 import { isCjs } from '../utils/commonjs';
 import { envToBool } from '../utils/envToBool';
-import { defaultStackParser, getSentryRelease } from './api';
-import { NodeClient } from './client';
+import { defaultStackParser } from './api';
+import { getTracesSampleRate, NodeClient } from './client';
 import { initOpenTelemetry, maybeInitializeEsmLoader } from './initOtel';
 
 function getCjsOnlyIntegrations(): Integration[] {
@@ -74,9 +74,12 @@ export function getDefaultIntegrationsWithoutPerformance(): Integration[] {
 }
 
 /** Get the default integrations for the Node SDK. */
-export function getDefaultIntegrations(options: Options): Integration[] {
+export function getDefaultIntegrations(options: NodeOptions): Integration[] {
   return [
     ...getDefaultIntegrationsWithoutPerformance(),
+    ...(options.spotlight
+      ? [spotlightIntegration({ sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined })]
+      : []),
     // We only add performance integrations if tracing is enabled
     // Note that this means that without tracing enabled, e.g. `expressIntegration()` will not be added
     // This means that generally request isolation will work (because that is done by httpIntegration)
@@ -88,64 +91,37 @@ export function getDefaultIntegrations(options: Options): Integration[] {
 /**
  * Initialize Sentry for Node.
  */
-export function init(options: NodeOptions | undefined = {}): NodeClient | undefined {
+export function init(options: NodeOptions = {}): NodeClient | undefined {
   return _init(options, getDefaultIntegrations);
 }
 
 /**
  * Initialize Sentry for Node, without any integrations added by default.
  */
-export function initWithoutDefaultIntegrations(options: NodeOptions | undefined = {}): NodeClient {
+export function initWithoutDefaultIntegrations(options: NodeOptions = {}): NodeClient {
   return _init(options, () => []);
 }
 
 /**
- * Initialize Sentry for Node, without performance instrumentation.
+ * Initialize a Node client with the provided options and default integrations getter function.
+ * This is an internal method the SDK uses under the hood to set up things - you should not use this as a user!
+ * Instead, use `init()` to initialize the SDK.
+ *
+ * @hidden
+ * @internal
  */
 function _init(
-  _options: NodeOptions | undefined = {},
-  getDefaultIntegrationsImpl: (options: Options) => Integration[],
+  options: NodeOptions = {},
+  getDefaultIntegrationsImpl: (options: NodeOptions) => Integration[],
 ): NodeClient {
-  const options = getClientOptions(_options, getDefaultIntegrationsImpl);
-
-  if (options.debug === true) {
-    if (DEBUG_BUILD) {
-      logger.enable();
-    } else {
-      // use `console.warn` rather than `logger.warn` since by non-debug bundles have all `logger.x` statements stripped
-      consoleSandbox(() => {
-        // eslint-disable-next-line no-console
-        console.warn('[Sentry] Cannot initialize SDK with `debug` option using a non-debug bundle.');
-      });
-    }
-  }
-
   if (!isCjs() && options.registerEsmLoaderHooks !== false) {
     maybeInitializeEsmLoader();
   }
 
   setOpenTelemetryContextAsyncContextStrategy();
 
-  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);
-
-  client.init();
-
-  logger.log(`Running in ${isCjs() ? 'CommonJS' : 'ESM'} mode.`);
-
-  client.startClientReportTracking();
+  const clientOptions = getClientOptions(options, getDefaultIntegrationsImpl);
+  const client = initAndBind(NodeClient, clientOptions);
 
   updateScopeFromEnvVariables();
 
@@ -197,65 +173,29 @@ export function validateOpenTelemetrySetup(): void {
 
 function getClientOptions(
   options: NodeOptions,
-  getDefaultIntegrationsImpl: (options: Options) => Integration[],
+  getDefaultIntegrationsImpl: (options: NodeOptions) => Integration[],
 ): NodeClientOptions {
-  const release = getRelease(options.release);
-  const spotlight =
-    options.spotlight ?? envToBool(process.env.SENTRY_SPOTLIGHT, { strict: true }) ?? process.env.SENTRY_SPOTLIGHT;
-  const tracesSampleRate = getTracesSampleRate(options.tracesSampleRate);
-
-  const mergedOptions = {
+  // We need to make sure to extract the tracesSampleRate already here, before we pass it to `getDefaultIntegrationsImpl`
+  // As otherwise, the check for `hasSpansEnabled` may not work in all scenarios
+  const optionsWithTracesSampleRate = {
     ...options,
-    dsn: options.dsn ?? process.env.SENTRY_DSN,
-    environment: options.environment ?? process.env.SENTRY_ENVIRONMENT,
-    sendClientReports: options.sendClientReports ?? true,
-    transport: options.transport ?? makeNodeTransport,
-    stackParser: stackParserFromStackParserOptions(options.stackParser || defaultStackParser),
-    release,
-    tracesSampleRate,
-    spotlight,
-    debug: envToBool(options.debug ?? process.env.SENTRY_DEBUG),
+    tracesSampleRate: getTracesSampleRate(options.tracesSampleRate),
   };
 
   const integrations = options.integrations;
-  const defaultIntegrations = options.defaultIntegrations ?? getDefaultIntegrationsImpl(mergedOptions);
+  const defaultIntegrations = options.defaultIntegrations ?? getDefaultIntegrationsImpl(optionsWithTracesSampleRate);
 
   return {
-    ...mergedOptions,
+    ...optionsWithTracesSampleRate,
+    transport: options.transport ?? makeNodeTransport,
+    stackParser: stackParserFromStackParserOptions(options.stackParser || defaultStackParser),
     integrations: getIntegrationsToSetup({
       defaultIntegrations,
       integrations,
     }),
   };
 }
 
-function getRelease(release: NodeOptions['release']): string | undefined {
-  if (release !== undefined) {
-    return release;
-  }
-
-  const detectedRelease = getSentryRelease();
-  if (detectedRelease !== undefined) {
-    return detectedRelease;
-  }
-
-  return undefined;
-}
-
-function getTracesSampleRate(tracesSampleRate: NodeOptions['tracesSampleRate']): number | undefined {
-  if (tracesSampleRate !== undefined) {
-    return tracesSampleRate;
-  }
-
-  const sampleRateFromEnv = process.env.SENTRY_TRACES_SAMPLE_RATE;
-  if (!sampleRateFromEnv) {
-    return undefined;
-  }
-
-  const parsed = parseFloat(sampleRateFromEnv);
-  return isFinite(parsed) ? parsed : undefined;
-}
-
 /**
  * Update scope and propagation context based on environmental variables.
  *

packages/node/src/sdk/initOtel.ts

@@ -7,7 +7,7 @@ import {
   ATTR_SERVICE_VERSION,
   SEMRESATTRS_SERVICE_NAMESPACE,
 } from '@opentelemetry/semantic-conventions';
-import { consoleSandbox, GLOBAL_OBJ, logger, SDK_VERSION } from '@sentry/core';
+import { consoleSandbox, enableLogger, GLOBAL_OBJ, logger, SDK_VERSION } from '@sentry/core';
 import { SentryPropagator, SentrySampler, SentrySpanProcessor } from '@sentry/opentelemetry';
 import { createAddHookMessageChannel } from 'import-in-the-middle';
 import moduleModule from 'module';
@@ -52,7 +52,10 @@ export function maybeInitializeEsmLoader(): void {
           transferList: [addHookMessagePort],
         });
       } catch (error) {
-        logger.warn('Failed to register ESM hook', error);
+        consoleSandbox(() => {
+          // eslint-disable-next-line no-console
+          console.warn('Failed to register ESM hook', error);
+        });
       }
     }
   } else {
@@ -79,7 +82,7 @@ export function preloadOpenTelemetry(options: NodePreloadOptions = {}): void {
   const { debug } = options;
 
   if (debug) {
-    logger.enable();
+    enableLogger();
     setupOpenTelemetryLogger();
   }
 

packages/node/test/sdk/client.test.ts

@@ -23,7 +23,7 @@ describe('NodeClient', () => {
     cleanupOtel();
   });
 
-  it('sets correct metadata', () => {
+  it('sets correct metadata & default options', () => {
     const options = getDefaultNodeClientOptions();
     const client = new NodeClient(options);
 
@@ -48,6 +48,8 @@ describe('NodeClient', () => {
       runtime: { name: 'node', version: expect.any(String) },
       serverName: expect.any(String),
       tracesSampleRate: 1,
+      debug: false,
+      sendClientReports: true,
     });
   });
 

packages/node/test/sdk/init.test.ts

@@ -123,6 +123,23 @@ describe('init()', () => {
         }),
       );
     });
+
+    it('does not install spotlight integration by default', () => {
+      const client = init({
+        dsn: PUBLIC_DSN,
+      });
+
+      expect(client!.getIntegrationByName('Spotlight')).toBeUndefined();
+    });
+
+    it('installs spotlight integration if spotlight=true is configured', () => {
+      const client = init({
+        dsn: PUBLIC_DSN,
+        spotlight: true,
+      });
+
+      expect(client!.getIntegrationByName('Spotlight')).toBeDefined();
+    });
   });
 
   describe('OpenTelemetry', () => {

packages/profiling-node/test/integration.test.ts

@@ -34,6 +34,7 @@ function makeLegacyContinuousProfilingClient(): [Sentry.NodeClient, Transport] {
     stackParser: Sentry.defaultStackParser,
     tracesSampleRate: 1,
     debug: true,
+    sendClientReports: false,
     environment: 'test-environment',
     dsn: 'https://7fa19397baaf433f919fbe02228d5470@o1137848.ingest.sentry.io/6625302',
     integrations: [integration],
@@ -55,6 +56,7 @@ function makeCurrentSpanProfilingClient(options: Partial<NodeClientOptions> = {}
     stackParser: Sentry.defaultStackParser,
     tracesSampleRate: 1,
     debug: true,
+    sendClientReports: false,
     environment: 'test-environment',
     dsn: 'https://7fa19397baaf433f919fbe02228d5470@o1137848.ingest.sentry.io/6625302',
     integrations: [integration],
@@ -84,6 +86,7 @@ function makeClientOptions(
     stackParser: Sentry.defaultStackParser,
     integrations: [_nodeProfilingIntegration()],
     debug: true,
+    sendClientReports: false,
     transport: _opts =>
       Sentry.makeNodeTransport({
         url: 'https://7fa19397baaf433f919fbe02228d5470@o1137848.ingest.sentry.io/6625302',