ref(gatsby): Move SDK initialization logic to SDK (#4040)

This PR refactors how the Gatsby plugin initializes the SDK. The initialization was done in the `gatsby-browser` script, and the Gatsby SDK was initializing the React SDK. This approach forces to use that script to initialize it, without providing any alternative.

Author: iker-barriocanal

packages/gatsby/gatsby-browser.js

@@ -1,40 +1,16 @@
-const Sentry = require('@sentry/react');
-const Tracing = require('@sentry/tracing');
+const Sentry = require('@sentry/gatsby');
 
 exports.onClientEntry = function(_, pluginParams) {
   if (pluginParams === undefined) {
     return;
   }
 
-  pluginParams._metadata = pluginParams._metadata || {};
-  pluginParams._metadata.sdk = {
-    name: 'sentry.javascript.gatsby',
-    packages: [
-      {
-        name: 'npm:@sentry/gatsby',
-        version: Sentry.SDK_VERSION,
-      },
-    ],
-    version: Sentry.SDK_VERSION,
-  };
-
-  const integrations = [...(pluginParams.integrations || [])];
-
-  if (Tracing.hasTracingEnabled(pluginParams) && !integrations.some(ele => ele.name === 'BrowserTracing')) {
-    integrations.push(new Tracing.Integrations.BrowserTracing(pluginParams.browserTracingOptions));
-  }
-
-  Tracing.addExtensionMethods();
-
   Sentry.init({
-    autoSessionTracking: true,
-    environment: process.env.NODE_ENV || 'development',
     // eslint-disable-next-line no-undef
     release: __SENTRY_RELEASE__,
     // eslint-disable-next-line no-undef
     dsn: __SENTRY_DSN__,
     ...pluginParams,
-    integrations,
   });
 
   window.Sentry = Sentry;

packages/gatsby/package.json

@@ -33,8 +33,10 @@
     "gatsby": "^2.0.0 || ^3.0.0"
   },
   "devDependencies": {
-    "@sentry-internal/eslint-config-sdk": "6.13.3",
+    "@sentry-internal/eslint-config-sdk": "6.13.2",
+    "@sentry/types": "6.13.2",
     "@testing-library/react": "^10.4.9",
+    "@types/node": "^16.10.3",
     "jest": "^24.7.1",
     "npm-run-all": "^4.1.2",
     "prettier": "1.19.0",

packages/gatsby/src/index.ts

@@ -1 +1,3 @@
 export * from '@sentry/react';
+
+export { init } from './sdk';

packages/gatsby/src/sdk.ts

@@ -0,0 +1,33 @@
+import { init as reactInit, SDK_VERSION } from '@sentry/react';
+
+import { getIntegrationsFromOptions } from './utils/integrations';
+import { GatsbyOptions } from './utils/types';
+
+export const defaultOptions = {
+  autoSessionTracking: true,
+  environment: process.env.NODE_ENV || 'development',
+};
+
+/**
+ * Inits the Sentry Gatsby SDK.
+ */
+export function init(options: GatsbyOptions): void {
+  options._metadata = options._metadata || {};
+  options._metadata.sdk = options._metadata.sdk || {
+    name: 'sentry.javascript.gatsby',
+    packages: [
+      {
+        name: 'npm:@sentry/gatsby',
+        version: SDK_VERSION,
+      },
+    ],
+    version: SDK_VERSION,
+  };
+
+  const integrations = getIntegrationsFromOptions(options);
+  reactInit({
+    ...defaultOptions,
+    ...options,
+    integrations,
+  });
+}

packages/gatsby/src/utils/integrations.ts

@@ -0,0 +1,21 @@
+import * as Tracing from '@sentry/tracing';
+import { Integration } from '@sentry/types';
+
+import { GatsbyOptions } from './types';
+
+/**
+ * Returns the integrations to add to the SDK.
+ * If tracing is enabled, `BrowserTracing` is always present.
+ *
+ * @param options The options users have defined.
+ */
+export function getIntegrationsFromOptions(options: GatsbyOptions): Integration[] {
+  const integrations = [...(options.integrations || [])];
+  if (
+    Tracing.hasTracingEnabled(options) &&
+    !integrations.some(integration => integration.name === Tracing.Integrations.BrowserTracing.name)
+  ) {
+    integrations.push(new Tracing.Integrations.BrowserTracing());
+  }
+  return integrations;
+}

packages/gatsby/src/utils/types.ts

@@ -0,0 +1,3 @@
+import { BrowserOptions } from '@sentry/react';
+
+export type GatsbyOptions = BrowserOptions;

packages/gatsby/test/gatsby-browser.test.ts

@@ -7,8 +7,8 @@ const { onClientEntry } = require('../gatsby-browser');
 (global as any).__SENTRY_DSN__ = 'https://[email protected]/0';
 
 let sentryInit = jest.fn();
-jest.mock('@sentry/react', () => {
-  const original = jest.requireActual('@sentry/react');
+jest.mock('@sentry/gatsby', () => {
+  const original = jest.requireActual('@sentry/gatsby');
   return {
     ...original,
     init: (...args: any[]) => {
@@ -38,42 +38,23 @@ describe('onClientEntry', () => {
     (window as any).Sentry = undefined;
   });
 
-  it('inits Sentry by default', () => {
-    onClientEntry(undefined, {});
+  it.each([
+    [{}, ['dsn', 'release']],
+    [{ key: 'value' }, ['dsn', 'release', 'key']],
+  ])('inits Sentry by default', (pluginParams, expectedKeys) => {
+    onClientEntry(undefined, pluginParams);
     expect(sentryInit).toHaveBeenCalledTimes(1);
-    expect(sentryInit).toHaveBeenLastCalledWith({
-      dsn: (global as any).__SENTRY_DSN__,
-      environment: process.env.NODE_ENV,
-      integrations: [],
-      release: (global as any).__SENTRY_RELEASE__,
-      autoSessionTracking: true,
-      _metadata: {
-        sdk: {
-          name: 'sentry.javascript.gatsby',
-          packages: [
-            {
-              name: 'npm:@sentry/gatsby',
-              version: expect.any(String),
-            },
-          ],
-          version: expect.any(String),
-        },
-      },
-    });
+    const calledWith = sentryInit.mock.calls[0][0];
+    for (const key of expectedKeys) {
+      expect(calledWith[key]).toBeDefined();
+    }
   });
 
   it('sets window.Sentry', () => {
     onClientEntry(undefined, {});
     expect((window as any).Sentry).not.toBeUndefined();
   });
 
-  it('adds Tracing extension methods', () => {
-    onClientEntry(undefined, {});
-
-    expect(tracingAddExtensionMethods).toHaveBeenCalledTimes(1);
-    expect(tracingAddExtensionMethods).toHaveBeenLastCalledWith();
-  });
-
   it('sets a tracesSampleRate if defined as option', () => {
     onClientEntry(undefined, { tracesSampleRate: 0.5 });
     expect(sentryInit).toHaveBeenLastCalledWith(
@@ -93,25 +74,6 @@ describe('onClientEntry', () => {
     );
   });
 
-  it('adds `BrowserTracing` integration if tracesSampleRate is defined', () => {
-    onClientEntry(undefined, { tracesSampleRate: 0.5 });
-    expect(sentryInit).toHaveBeenLastCalledWith(
-      expect.objectContaining({
-        integrations: [expect.objectContaining({ name: 'BrowserTracing' })],
-      }),
-    );
-  });
-
-  it('adds `BrowserTracing` integration if tracesSampler is defined', () => {
-    const tracesSampler = jest.fn();
-    onClientEntry(undefined, { tracesSampler });
-    expect(sentryInit).toHaveBeenLastCalledWith(
-      expect.objectContaining({
-        integrations: [expect.objectContaining({ name: 'BrowserTracing' })],
-      }),
-    );
-  });
-
   it('only defines a single `BrowserTracing` integration', () => {
     const Tracing = jest.requireActual('@sentry/tracing');
     const integrations = [new Tracing.Integrations.BrowserTracing()];

packages/gatsby/test/index.test.ts

@@ -1,4 +1,4 @@
-import * as GatsbyIntegration from '../src';
+import * as GatsbyIntegration from '../src/index';
 
 describe('package', () => {
   it('exports init', () => {

packages/gatsby/test/sdk.test.ts

@@ -0,0 +1,92 @@
+import { init as reactInitRaw, SDK_VERSION } from '@sentry/react';
+import { Integrations } from '@sentry/tracing';
+import { Integration, Package } from '@sentry/types';
+
+import { defaultOptions, init as gatsbyInit } from '../src/sdk';
+import { GatsbyOptions } from '../src/utils/types';
+
+const reactInit = reactInitRaw as jest.Mock;
+jest.mock('@sentry/react');
+
+describe('Initialize React SDK', () => {
+  afterEach(() => reactInit.mockReset());
+
+  test('Default init props', () => {
+    gatsbyInit({});
+    expect(reactInit).toHaveBeenCalledTimes(1);
+    const calledWith = reactInit.mock.calls[0][0];
+    expect(calledWith).toMatchObject(defaultOptions);
+  });
+
+  test('Has correct SDK metadata', () => {
+    gatsbyInit({});
+    const calledWith = reactInit.mock.calls[0][0];
+    const sdkMetadata = calledWith._metadata.sdk;
+    expect(sdkMetadata.name).toStrictEqual('sentry.javascript.gatsby');
+    expect(sdkMetadata.version).toBe(SDK_VERSION);
+    expect(sdkMetadata.packages).toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "name": "npm:@sentry/gatsby",
+          "version": "6.13.3",
+        },
+      ]
+    `);
+  });
+
+  describe('Environment', () => {
+    test('process.env', () => {
+      gatsbyInit({});
+      expect(reactInit).toHaveBeenCalledTimes(1);
+      const callingObject = reactInit.mock.calls[0][0];
+      expect(callingObject.environment).toStrictEqual('test');
+    });
+
+    test('defined in the options', () => {
+      gatsbyInit({
+        environment: 'custom env!',
+      });
+      expect(reactInit).toHaveBeenCalledTimes(1);
+      const callingObject = reactInit.mock.calls[0][0];
+      expect(callingObject.environment).toStrictEqual('custom env!');
+    });
+  });
+
+  test('Has BrowserTracing if tracing enabled', () => {
+    gatsbyInit({ tracesSampleRate: 1 });
+    expect(reactInit).toHaveBeenCalledTimes(1);
+    const calledWith = reactInit.mock.calls[0][0];
+    const integrationNames: string[] = calledWith.integrations.map((integration: Integration) => integration.name);
+    expect(integrationNames.some(name => name === 'BrowserTracing')).toBe(true);
+  });
+});
+
+describe('Integrations from options', () => {
+  afterEach(() => reactInit.mockClear());
+
+  test.each([
+    ['tracing disabled, no integrations', {}, []],
+    ['tracing enabled, no integrations', { tracesSampleRate: 1 }, ['BrowserTracing']],
+    [
+      'tracing disabled, with Integrations.BrowserTracing',
+      { integrations: [new Integrations.BrowserTracing()] },
+      ['BrowserTracing'],
+    ],
+    [
+      'tracing enabled, with Integrations.BrowserTracing',
+      { tracesSampleRate: 1, integrations: [new Integrations.BrowserTracing()] },
+      ['BrowserTracing'],
+    ],
+    [
+      'tracing enabled, with another integration',
+      { tracesSampleRate: 1, integrations: [new Integrations.Express()] },
+      ['Express', 'BrowserTracing'],
+    ],
+    ['tracing disabled, with another integration', { integrations: [new Integrations.Express()] }, ['Express']],
+  ])('%s', (_testName, options: GatsbyOptions, expectedIntNames: string[]) => {
+    gatsbyInit(options);
+    const integrations: Integration[] = reactInit.mock.calls[0][0].integrations;
+    expect(integrations).toHaveLength(expectedIntNames.length);
+    integrations.map((integration, idx) => expect(integration.name).toStrictEqual(expectedIntNames[idx]));
+  });
+});