move sdk info setting to client in node and browser, add tests (#3279)

getsentry · Mar 3, 2021 · 5537110 · 5537110
1 parent 37dd210
commit 5537110
Show file tree

Hide file tree

Showing 6 changed files with 141 additions and 28 deletions.
diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts
@@ -1,4 +1,4 @@
-import { BaseClient, Scope } from '@sentry/core';
+import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
 import { Event, EventHint } from '@sentry/types';
 import { getGlobalObject, logger } from '@sentry/utils';
 
@@ -19,6 +19,18 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
    * @param options Configuration options for this SDK.
    */
   public constructor(options: BrowserOptions = {}) {
+    options._metadata = options._metadata || {};
+    options._metadata.sdk = options._metadata.sdk || {
+      name: 'sentry.javascript.browser',
+      packages: [
+        {
+          name: 'npm:@sentry/browser',
+          version: SDK_VERSION,
+        },
+      ],
+      version: SDK_VERSION,
+    };
+
     super(BrowserBackend, options);
   }
 

diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts
@@ -1,4 +1,4 @@
-import { getCurrentHub, initAndBind, Integrations as CoreIntegrations, SDK_VERSION } from '@sentry/core';
+import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
 import { addInstrumentationHandler, getGlobalObject, logger, SyncPromise } from '@sentry/utils';
 
 import { BrowserOptions } from './backend';
@@ -88,18 +88,6 @@ export function init(options: BrowserOptions = {}): void {
     options.autoSessionTracking = true;
   }
 
-  options._metadata = options._metadata || {};
-  options._metadata.sdk = {
-    name: 'sentry.javascript.browser',
-    packages: [
-      {
-        name: 'npm:@sentry/browser',
-        version: SDK_VERSION,
-      },
-    ],
-    version: SDK_VERSION,
-  };
-
   initAndBind(BrowserClient, options);
 
   if (options.autoSessionTracking) {

diff --git a/packages/browser/test/unit/index.test.ts b/packages/browser/test/unit/index.test.ts
@@ -1,3 +1,4 @@
+import { SDK_VERSION } from '@sentry/core';
 import { expect } from 'chai';
 import { SinonSpy, spy } from 'sinon';
 
@@ -177,11 +178,67 @@ describe('SentryBrowser initialization', () => {
     expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).to.equal('foobar');
     delete global.SENTRY_RELEASE;
   });
+
   it('should have initialization proceed as normal if window.SENTRY_RELEASE is not set', () => {
     // This is mostly a happy-path test to ensure that the initialization doesn't throw an error.
     init({ dsn });
     expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).to.be.undefined;
   });
+
+  describe('SDK metadata', () => {
+    it('should set SDK data when Sentry.init() is called', () => {
+      init({ dsn });
+
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const sdkData = (getCurrentHub().getClient() as any)._backend._transport._api.metadata?.sdk;
+
+      expect(sdkData.name).to.equal('sentry.javascript.browser');
+      expect(sdkData.packages[0].name).to.equal('npm:@sentry/browser');
+      expect(sdkData.packages[0].version).to.equal(SDK_VERSION);
+      expect(sdkData.version).to.equal(SDK_VERSION);
+    });
+
+    it('should set SDK data when instantiating a client directly', () => {
+      const client = new BrowserClient({ dsn });
+
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const sdkData = (client as any)._backend._transport._api.metadata?.sdk;
+
+      expect(sdkData.name).to.equal('sentry.javascript.browser');
+      expect(sdkData.packages[0].name).to.equal('npm:@sentry/browser');
+      expect(sdkData.packages[0].version).to.equal(SDK_VERSION);
+      expect(sdkData.version).to.equal(SDK_VERSION);
+    });
+
+    // wrapper packages (like @sentry/angular and @sentry/react) set their SDK data in their `init` methods, which are
+    // called before the client is instantiated, and we don't want to clobber that data
+    it("shouldn't overwrite SDK data that's already there", () => {
+      init({
+        dsn,
+        // this would normally be set by the wrapper SDK in init()
+        _metadata: {
+          sdk: {
+            name: 'sentry.javascript.angular',
+            packages: [
+              {
+                name: 'npm:@sentry/angular',
+                version: SDK_VERSION,
+              },
+            ],
+            version: SDK_VERSION,
+          },
+        },
+      });
+
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const sdkData = (getCurrentHub().getClient() as any)._backend._transport._api.metadata?.sdk;
+
+      expect(sdkData.name).to.equal('sentry.javascript.angular');
+      expect(sdkData.packages[0].name).to.equal('npm:@sentry/angular');
+      expect(sdkData.packages[0].version).to.equal(SDK_VERSION);
+      expect(sdkData.version).to.equal(SDK_VERSION);
+    });
+  });
 });
 
 describe('wrap()', () => {

diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts
@@ -1,4 +1,4 @@
-import { BaseClient, Scope } from '@sentry/core';
+import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
 import { Event, EventHint } from '@sentry/types';
 
 import { NodeBackend, NodeOptions } from './backend';
@@ -15,6 +15,18 @@ export class NodeClient extends BaseClient<NodeBackend, NodeOptions> {
    * @param options Configuration options for this SDK.
    */
   public constructor(options: NodeOptions) {
+    options._metadata = options._metadata || {};
+    options._metadata.sdk = options._metadata.sdk || {
+      name: 'sentry.javascript.node',
+      packages: [
+        {
+          name: 'npm:@sentry/node',
+          version: SDK_VERSION,
+        },
+      ],
+      version: SDK_VERSION,
+    };
+
     super(NodeBackend, options);
   }
 

diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts
@@ -1,4 +1,4 @@
-import { getCurrentHub, initAndBind, Integrations as CoreIntegrations, SDK_VERSION } from '@sentry/core';
+import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
 import { getMainCarrier, setHubOnCarrier } from '@sentry/hub';
 import { getGlobalObject } from '@sentry/utils';
 import * as domain from 'domain';
@@ -108,18 +108,6 @@ export function init(options: NodeOptions = {}): void {
     options.environment = process.env.SENTRY_ENVIRONMENT;
   }
 
-  options._metadata = options._metadata || {};
-  options._metadata.sdk = {
-    name: 'sentry.javascript.node',
-    packages: [
-      {
-        name: 'npm:@sentry/node',
-        version: SDK_VERSION,
-      },
-    ],
-    version: SDK_VERSION,
-  };
-
   // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
   if ((domain as any).active) {
     setHubOnCarrier(getMainCarrier(), getCurrentHub());

diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts
@@ -1,3 +1,4 @@
+import { SDK_VERSION } from '@sentry/core';
 import * as domain from 'domain';
 
 import {
@@ -282,4 +283,59 @@ describe('SentryNode initialization', () => {
     init({ dsn });
     expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).toBeUndefined();
   });
+
+  describe('SDK metadata', () => {
+    it('should set SDK data when Sentry.init() is called', () => {
+      init({ dsn });
+
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const sdkData = (getCurrentHub().getClient() as any)._backend._transport._api.metadata?.sdk;
+
+      expect(sdkData.name).toEqual('sentry.javascript.node');
+      expect(sdkData.packages[0].name).toEqual('npm:@sentry/node');
+      expect(sdkData.packages[0].version).toEqual(SDK_VERSION);
+      expect(sdkData.version).toEqual(SDK_VERSION);
+    });
+
+    it('should set SDK data when instantiating a client directly', () => {
+      const client = new NodeClient({ dsn });
+
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const sdkData = (client as any)._backend._transport._api.metadata?.sdk;
+
+      expect(sdkData.name).toEqual('sentry.javascript.node');
+      expect(sdkData.packages[0].name).toEqual('npm:@sentry/node');
+      expect(sdkData.packages[0].version).toEqual(SDK_VERSION);
+      expect(sdkData.version).toEqual(SDK_VERSION);
+    });
+
+    // wrapper packages (like @sentry/serverless) set their SDK data in their `init` methods, which are
+    // called before the client is instantiated, and we don't want to clobber that data
+    it("shouldn't overwrite SDK data that's already there", () => {
+      init({
+        dsn,
+        // this would normally be set by the wrapper SDK in init()
+        _metadata: {
+          sdk: {
+            name: 'sentry.javascript.serverless',
+            packages: [
+              {
+                name: 'npm:@sentry/serverless',
+                version: SDK_VERSION,
+              },
+            ],
+            version: SDK_VERSION,
+          },
+        },
+      });
+
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const sdkData = (getCurrentHub().getClient() as any)._backend._transport._api.metadata?.sdk;
+
+      expect(sdkData.name).toEqual('sentry.javascript.serverless');
+      expect(sdkData.packages[0].name).toEqual('npm:@sentry/serverless');
+      expect(sdkData.packages[0].version).toEqual(SDK_VERSION);
+      expect(sdkData.version).toEqual(SDK_VERSION);
+    });
+  });
 });