move node-version- and server-name-adding code to client

Author: lobsterkatie

packages/node/src/client.ts

@@ -2,6 +2,7 @@ import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
 import { SessionFlusher } from '@sentry/hub';
 import { Event, EventHint, Severity, SeverityLevel } from '@sentry/types';
 import { logger, resolvedSyncPromise } from '@sentry/utils';
+import * as os from 'os';
 import { TextEncoder } from 'util';
 
 import { eventFromMessage, eventFromUnknownInput } from './eventbuilder';
@@ -139,9 +140,15 @@ export class NodeClient extends BaseClient<NodeClientOptions> {
    */
   protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike<Event | null> {
     event.platform = event.platform || 'node';
-    if (this.getOptions().serverName) {
-      event.server_name = this.getOptions().serverName;
-    }
+    event.contexts = {
+      ...event.contexts,
+      runtime: {
+        name: 'node',
+        version: global.process.version,
+      },
+    };
+    event.server_name =
+      event.server_name || this.getOptions().serverName || global.process.env.SENTRY_NAME || os.hostname();
     return super._prepareEvent(event, hint, scope);
   }
 

packages/node/src/handlers.ts

@@ -14,7 +14,6 @@ import {
 import * as cookie from 'cookie';
 import * as domain from 'domain';
 import * as http from 'http';
-import * as os from 'os';
 import * as url from 'url';
 
 import { NodeClient } from './client';
@@ -290,10 +289,8 @@ export function extractRequestData(
 export interface ParseRequestOptions {
   ip?: boolean;
   request?: boolean | string[];
-  serverName?: boolean;
   transaction?: boolean | TransactionNamingScheme;
   user?: boolean | string[];
-  version?: boolean;
 }
 
 /**
@@ -309,23 +306,11 @@ export function parseRequest(event: Event, req: ExpressRequest, options?: ParseR
   options = {
     ip: false,
     request: true,
-    serverName: true,
     transaction: true,
     user: true,
-    version: true,
     ...options,
   };
 
-  if (options.version) {
-    event.contexts = {
-      ...event.contexts,
-      runtime: {
-        name: 'node',
-        version: global.process.version,
-      },
-    };
-  }
-
   if (options.request) {
     // if the option value is `true`, use the default set of keys by not passing anything to `extractRequestData()`
     const extractedRequestData = Array.isArray(options.request)
@@ -337,10 +322,6 @@ export function parseRequest(event: Event, req: ExpressRequest, options?: ParseR
     };
   }
 
-  if (options.serverName && !event.server_name) {
-    event.server_name = global.process.env.SENTRY_NAME || os.hostname();
-  }
-
   if (options.user) {
     const extractedUser = req.user && isPlainObject(req.user) ? extractUserData(req.user, options.user) : {};
 

packages/node/test/client.test.ts

@@ -1,4 +1,6 @@
 import { Scope, SessionFlusher } from '@sentry/hub';
+import { Event, EventHint } from '@sentry/types';
+import * as os from 'os';
 
 import { NodeClient } from '../src';
 import { getDefaultNodeClientOptions } from './helper/node-client-options';
@@ -187,6 +189,81 @@ describe('NodeClient', () => {
       expect(requestSession!.status).toEqual('ok');
     });
   });
+
+  describe('_prepareEvent', () => {
+    test('adds platform to event', () => {
+      const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN });
+      client = new NodeClient(options);
+
+      const event: Event = {};
+      const hint: EventHint = {};
+      (client as any)._prepareEvent(event, hint);
+
+      expect(event.platform).toEqual('node');
+    });
+
+    test('adds runtime context to event', () => {
+      const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN });
+      client = new NodeClient(options);
+
+      const event: Event = {};
+      const hint: EventHint = {};
+      (client as any)._prepareEvent(event, hint);
+
+      expect(event.contexts?.runtime).toEqual({
+        name: 'node',
+        version: process.version,
+      });
+    });
+
+    test('adds server name to event when value passed in options', () => {
+      const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, serverName: 'foo' });
+      client = new NodeClient(options);
+
+      const event: Event = {};
+      const hint: EventHint = {};
+      (client as any)._prepareEvent(event, hint);
+
+      expect(event.server_name).toEqual('foo');
+    });
+
+    test('adds server name to event when value given in env', () => {
+      const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN });
+      client = new NodeClient(options);
+      process.env.SENTRY_NAME = 'foo';
+
+      const event: Event = {};
+      const hint: EventHint = {};
+      (client as any)._prepareEvent(event, hint);
+
+      expect(event.server_name).toEqual('foo');
+
+      delete process.env.SENTRY_NAME;
+    });
+
+    test('adds hostname as event server name when no value given', () => {
+      const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN });
+      client = new NodeClient(options);
+
+      const event: Event = {};
+      const hint: EventHint = {};
+      (client as any)._prepareEvent(event, hint);
+
+      expect(event.server_name).toEqual(os.hostname());
+    });
+
+    test("doesn't clobber existing server name", () => {
+      const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, serverName: 'bar' });
+      client = new NodeClient(options);
+
+      const event: Event = { server_name: 'foo' };
+      const hint: EventHint = {};
+      (client as any)._prepareEvent(event, hint);
+
+      expect(event.server_name).toEqual('foo');
+      expect(event.server_name).not.toEqual('bar');
+    });
+  });
 });
 
 describe('flush/close', () => {

packages/node/test/handlers.test.ts

@@ -2,7 +2,7 @@ import * as sentryCore from '@sentry/core';
 import * as sentryHub from '@sentry/hub';
 import { Hub } from '@sentry/hub';
 import { Transaction } from '@sentry/tracing';
-import { Baggage, Runtime } from '@sentry/types';
+import { Baggage } from '@sentry/types';
 import { SentryError } from '@sentry/utils';
 import * as http from 'http';
 import * as net from 'net';
@@ -55,25 +55,6 @@ describe('parseRequest', () => {
     };
   });
 
-  describe('parseRequest.contexts runtime', () => {
-    test('runtime name must contain node', () => {
-      const parsedRequest: Event = parseRequest({}, mockReq as ExpressRequest);
-      expect((parsedRequest.contexts!.runtime as Runtime).name).toEqual('node');
-    });
-
-    test('runtime version must contain current node version', () => {
-      const parsedRequest: Event = parseRequest({}, mockReq as ExpressRequest);
-      expect((parsedRequest.contexts!.runtime as Runtime).version).toEqual(process.version);
-    });
-
-    test('runtime disbaled by options', () => {
-      const parsedRequest: Event = parseRequest({}, mockReq as ExpressRequest, {
-        version: false,
-      });
-      expect(parsedRequest).not.toHaveProperty('contexts.runtime');
-    });
-  });
-
   describe('parseRequest.user properties', () => {
     const DEFAULT_USER_KEYS = ['id', 'username', 'email'];
     const CUSTOM_USER_KEYS = ['custom_property'];

packages/serverless/src/awslambda.ts

@@ -180,11 +180,6 @@ function enhanceScopeWithEnvironmentData(scope: Scope, context: Context, startTi
   scope.setTag('server_name', process.env._AWS_XRAY_DAEMON_ADDRESS || process.env.SENTRY_NAME || hostname());
   scope.setTag('url', `awslambda:///${context.functionName}`);
 
-  scope.setContext('runtime', {
-    name: 'node',
-    version: global.process.version,
-  });
-
   scope.setContext('aws.lambda', {
     aws_request_id: context.awsRequestId,
     function_name: context.functionName,

packages/serverless/src/gcpfunction/general.ts

@@ -1,7 +1,6 @@
 import { Scope } from '@sentry/node';
 import { Context as SentryContext } from '@sentry/types';
 import type { Request, Response } from 'express';
-import { hostname } from 'os';
 
 export interface HttpFunction {
   (req: Request, res: Response): any; // eslint-disable-line @typescript-eslint/no-explicit-any
@@ -54,11 +53,6 @@ export interface WrapperOptions {
  * @param context event context
  */
 export function configureScopeWithContext(scope: Scope, context: Context): void {
-  scope.setContext('runtime', {
-    name: 'node',
-    version: global.process.version,
-  });
-  scope.setTag('server_name', process.env.SENTRY_NAME || hostname());
   scope.setContext('gcp.function.context', { ...context } as SentryContext);
 }
 

packages/serverless/test/awslambda.test.ts

@@ -46,8 +46,6 @@ function expectScopeSettings() {
   // @ts-ignore see "Why @ts-ignore" note
   expect(Sentry.fakeScope.setTag).toBeCalledWith('url', 'awslambda:///functionName');
   // @ts-ignore see "Why @ts-ignore" note
-  expect(Sentry.fakeScope.setContext).toBeCalledWith('runtime', { name: 'node', version: expect.anything() });
-  // @ts-ignore see "Why @ts-ignore" note
   expect(Sentry.fakeScope.setContext).toBeCalledWith(
     'aws.lambda',
     expect.objectContaining({
@@ -181,7 +179,7 @@ describe('AWSLambda', () => {
 
   describe('wrapHandler() on sync handler', () => {
     test('successful execution', async () => {
-      expect.assertions(10);
+      expect.assertions(9);
 
       const handler: Handler = (_event, _context, callback) => {
         callback(null, 42);
@@ -197,7 +195,7 @@ describe('AWSLambda', () => {
     });
 
     test('unsuccessful execution', async () => {
-      expect.assertions(10);
+      expect.assertions(9);
 
       const error = new Error('sorry');
       const handler: Handler = (_event, _context, callback) => {
@@ -264,7 +262,7 @@ describe('AWSLambda', () => {
     });
 
     test('capture error', async () => {
-      expect.assertions(10);
+      expect.assertions(9);
 
       const error = new Error('wat');
       const handler: Handler = (_event, _context, _callback) => {
@@ -294,7 +292,7 @@ describe('AWSLambda', () => {
 
   describe('wrapHandler() on async handler', () => {
     test('successful execution', async () => {
-      expect.assertions(10);
+      expect.assertions(9);
 
       const handler: Handler = async (_event, _context) => {
         return 42;
@@ -321,7 +319,7 @@ describe('AWSLambda', () => {
     });
 
     test('capture error', async () => {
-      expect.assertions(10);
+      expect.assertions(9);
 
       const error = new Error('wat');
       const handler: Handler = async (_event, _context) => {
@@ -359,7 +357,7 @@ describe('AWSLambda', () => {
 
   describe('wrapHandler() on async handler with a callback method (aka incorrect usage)', () => {
     test('successful execution', async () => {
-      expect.assertions(10);
+      expect.assertions(9);
 
       const handler: Handler = async (_event, _context, _callback) => {
         return 42;
@@ -386,7 +384,7 @@ describe('AWSLambda', () => {
     });
 
     test('capture error', async () => {
-      expect.assertions(10);
+      expect.assertions(9);
 
       const error = new Error('wat');
       const handler: Handler = async (_event, _context, _callback) => {

packages/serverless/test/gcpfunction.test.ts

@@ -212,7 +212,7 @@ describe('GCPFunction', () => {
   });
 
   test('wrapHttpFunction request data', async () => {
-    expect.assertions(7);
+    expect.assertions(6);
 
     const handler: HttpFunction = (_req, res) => {
       res.end();
@@ -223,7 +223,6 @@ describe('GCPFunction', () => {
     Sentry.fakeScope.addEventProcessor.mockImplementation(cb => cb(event));
     await handleHttp(wrappedHandler);
     expect(event.transaction).toEqual('POST /path');
-    expect(event.contexts?.runtime).toEqual({ name: 'node', version: expect.anything() });
     expect(event.request?.method).toEqual('POST');
     expect(event.request?.url).toEqual('http://hostname/path?q=query');
     expect(event.request?.query_string).toEqual('q=query');
@@ -362,16 +361,12 @@ describe('GCPFunction', () => {
   });
 
   test('wrapEventFunction scope data', async () => {
-    expect.assertions(3);
+    expect.assertions(1);
 
     const handler: EventFunction = (_data, _context) => 42;
     const wrappedHandler = wrapEventFunction(handler);
     await handleEvent(wrappedHandler);
     // @ts-ignore see "Why @ts-ignore" note
-    expect(Sentry.fakeScope.setContext).toBeCalledWith('runtime', { name: 'node', version: expect.anything() });
-    // @ts-ignore see "Why @ts-ignore" note
-    expect(Sentry.fakeScope.setTag).toBeCalledWith('server_name', expect.anything());
-    // @ts-ignore see "Why @ts-ignore" note
     expect(Sentry.fakeScope.setContext).toBeCalledWith('gcp.function.context', {
       eventType: 'event.type',
       resource: 'some.resource',
@@ -466,16 +461,12 @@ describe('GCPFunction', () => {
   });
 
   test('wrapCloudEventFunction scope data', async () => {
-    expect.assertions(3);
+    expect.assertions(1);
 
     const handler: CloudEventFunction = _context => 42;
     const wrappedHandler = wrapCloudEventFunction(handler);
     await handleCloudEvent(wrappedHandler);
     // @ts-ignore see "Why @ts-ignore" note
-    expect(Sentry.fakeScope.setContext).toBeCalledWith('runtime', { name: 'node', version: expect.anything() });
-    // @ts-ignore see "Why @ts-ignore" note
-    expect(Sentry.fakeScope.setTag).toBeCalledWith('server_name', expect.anything());
-    // @ts-ignore see "Why @ts-ignore" note
     expect(Sentry.fakeScope.setContext).toBeCalledWith('gcp.function.context', { type: 'event.type' });
   });