ref: merge global scope in prepareEvent

Author: mydea

packages/core/src/scope.ts

@@ -456,6 +456,7 @@ export class Scope implements ScopeInterface {
 
   /**
    * @inheritDoc
+   * @deprecated Use `getScopeData()` instead.
    */
   public getAttachments(): Attachment[] {
     const data = this.getScopeData();
@@ -472,7 +473,7 @@ export class Scope implements ScopeInterface {
   }
 
   /** @inheritDoc */
-  public getPerScopeData(): ScopeData {
+  public getScopeData(): ScopeData {
     const {
       _breadcrumbs,
       _attachments,
@@ -506,16 +507,6 @@ export class Scope implements ScopeInterface {
     };
   }
 
-  /** @inheritdoc */
-  public getScopeData(): ScopeData {
-    const data = getGlobalScope().getPerScopeData();
-    const scopeData = this.getPerScopeData();
-
-    mergeScopeData(data, scopeData);
-
-    return data;
-  }
-
   /**
    * Applies data from the scope to the event and runs all event processors on it.
    *

packages/core/src/utils/prepareEvent.ts

@@ -13,8 +13,8 @@ import { GLOBAL_OBJ, addExceptionMechanism, dateTimestampInSeconds, normalize, t
 
 import { DEFAULT_ENVIRONMENT } from '../constants';
 import { getGlobalEventProcessors, notifyEventProcessors } from '../eventProcessors';
-import { Scope } from '../scope';
-import { applyScopeDataToEvent } from './applyScopeDataToEvent';
+import { Scope, getGlobalScope } from '../scope';
+import { applyScopeDataToEvent, mergeScopeData } from './applyScopeDataToEvent';
 
 /**
  * This type makes sure that we get either a CaptureContext, OR an EventHint.
@@ -74,36 +74,32 @@ export function prepareEvent(
   }
 
   const clientEventProcessors = client && client.getEventProcessors ? client.getEventProcessors() : [];
-  // TODO (v8): Update this order to be: Global > Client > Scope
-  const eventProcessors = [
-    ...clientEventProcessors,
-    // eslint-disable-next-line deprecation/deprecation
-    ...getGlobalEventProcessors(),
-  ];
 
   // This should be the last thing called, since we want that
   // {@link Hub.addEventProcessor} gets the finished prepared event.
-  //
-  // We need to check for the existence of `finalScope.getAttachments`
-  // because `getAttachments` can be undefined if users are using an older version
-  // of `@sentry/core` that does not have the `getAttachments` method.
-  // See: https://github.com/getsentry/sentry-javascript/issues/5229
+  // Merge scope data together
+  const data = getGlobalScope().getScopeData();
+
   if (finalScope) {
-    // Collect attachments from the hint and scope
-    if (finalScope.getAttachments) {
-      const attachments = [...(hint.attachments || []), ...finalScope.getAttachments()];
+    const finalScopeData = finalScope.getScopeData();
+    mergeScopeData(data, finalScopeData);
+  }
 
-      if (attachments.length) {
-        hint.attachments = attachments;
-      }
-    }
+  const attachments = [...(hint.attachments || []), ...data.attachments];
+  if (attachments.length) {
+    hint.attachments = attachments;
+  }
 
-    const scopeData = finalScope.getScopeData();
-    applyScopeDataToEvent(prepared, scopeData);
+  applyScopeDataToEvent(prepared, data);
 
+  // TODO (v8): Update this order to be: Global > Client > Scope
+  const eventProcessors = [
+    ...clientEventProcessors,
+    // eslint-disable-next-line deprecation/deprecation
+    ...getGlobalEventProcessors(),
     // Run scope event processors _after_ all other processors
-    eventProcessors.push(...scopeData.eventProcessors);
-  }
+    ...data.eventProcessors,
+  ];
 
   const result = notifyEventProcessors(eventProcessors, prepared, hint);
 

packages/core/test/lib/base.test.ts

@@ -1,7 +1,7 @@
 import type { Client, Envelope, Event, Span, Transaction } from '@sentry/types';
 import { SentryError, SyncPromise, dsnToString, logger } from '@sentry/utils';
 
-import { Hub, Scope, makeSession } from '../../src';
+import { Hub, Scope, clearGlobalData, makeSession } from '../../src';
 import * as integrationModule from '../../src/integration';
 import { TestClient, getDefaultTestClientOptions } from '../mocks/client';
 import { AdHocIntegration, TestIntegration } from '../mocks/integration';
@@ -54,6 +54,7 @@ describe('BaseClient', () => {
   beforeEach(() => {
     TestClient.sendEventCalled = undefined;
     TestClient.instance = undefined;
+    clearGlobalData();
   });
 
   afterEach(() => {
@@ -756,7 +757,8 @@ describe('BaseClient', () => {
       expect(TestClient.instance!.event!).toEqual(
         expect.objectContaining({
           breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb],
-          contexts: normalizedObject,
+          // also has trace context from global scope
+          contexts: { ...normalizedObject, trace: expect.anything() },
           environment: 'production',
           event_id: '42',
           extra: normalizedObject,
@@ -805,7 +807,8 @@ describe('BaseClient', () => {
       expect(TestClient.instance!.event!).toEqual(
         expect.objectContaining({
           breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb],
-          contexts: normalizedObject,
+          // also has trace context from global scope
+          contexts: { ...normalizedObject, trace: expect.anything() },
           environment: 'production',
           event_id: '42',
           extra: normalizedObject,
@@ -859,7 +862,8 @@ describe('BaseClient', () => {
       expect(TestClient.instance!.event!).toEqual(
         expect.objectContaining({
           breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb],
-          contexts: normalizedObject,
+          // also has trace context from global scope
+          contexts: { ...normalizedObject, trace: expect.anything() },
           environment: 'production',
           event_id: '42',
           extra: normalizedObject,

packages/core/test/lib/prepareEvent.test.ts

@@ -1,8 +1,23 @@
-import type { Event, EventHint, ScopeContext } from '@sentry/types';
+import type {
+  Attachment,
+  Breadcrumb,
+  Client,
+  ClientOptions,
+  Event,
+  EventHint,
+  EventProcessor,
+  ScopeContext,
+} from '@sentry/types';
 import { GLOBAL_OBJ, createStackParser } from '@sentry/utils';
+import { clearGlobalData } from '../../src';
 
-import { Scope } from '../../src/scope';
-import { applyDebugIds, applyDebugMeta, parseEventHintOrCaptureContext } from '../../src/utils/prepareEvent';
+import { Scope, getGlobalScope } from '../../src/scope';
+import {
+  applyDebugIds,
+  applyDebugMeta,
+  parseEventHintOrCaptureContext,
+  prepareEvent,
+} from '../../src/utils/prepareEvent';
 
 describe('applyDebugIds', () => {
   afterEach(() => {
@@ -173,3 +188,170 @@ describe('parseEventHintOrCaptureContext', () => {
     });
   });
 });
+
+describe('prepareEvent', () => {
+  beforeEach(() => {
+    clearGlobalData();
+  });
+
+  it('works without any scope data', async () => {
+    const eventProcessor = jest.fn((a: unknown) => a) as EventProcessor;
+
+    const scope = new Scope();
+
+    const event = { message: 'foo' };
+
+    const options = {} as ClientOptions;
+    const client = {
+      getEventProcessors() {
+        return [eventProcessor];
+      },
+    } as Client;
+    const processedEvent = await prepareEvent(
+      options,
+      event,
+      {
+        integrations: [],
+      },
+      scope,
+      client,
+    );
+
+    expect(eventProcessor).toHaveBeenCalledWith(processedEvent, {
+      integrations: [],
+      // no attachments are added to hint
+    });
+
+    expect(processedEvent).toEqual({
+      timestamp: expect.any(Number),
+      event_id: expect.any(String),
+      environment: 'production',
+      message: 'foo',
+      sdkProcessingMetadata: {
+        propagationContext: {
+          spanId: expect.any(String),
+          traceId: expect.any(String),
+        },
+      },
+    });
+  });
+
+  it('merges scope data', async () => {
+    const breadcrumb1 = { message: '1', timestamp: 111 } as Breadcrumb;
+    const breadcrumb2 = { message: '2', timestamp: 222 } as Breadcrumb;
+    const breadcrumb3 = { message: '3', timestamp: 123 } as Breadcrumb;
+
+    const eventProcessor1 = jest.fn((a: unknown) => a) as EventProcessor;
+    const eventProcessor2 = jest.fn((b: unknown) => b) as EventProcessor;
+
+    const attachment1 = { filename: '1' } as Attachment;
+    const attachment2 = { filename: '2' } as Attachment;
+
+    const scope = new Scope();
+    scope.update({
+      user: { id: '1', email: '[email protected]' },
+      tags: { tag1: 'aa', tag2: 'aa' },
+      extra: { extra1: 'aa', extra2: 'aa' },
+      contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } },
+      propagationContext: { spanId: '1', traceId: '1' },
+      fingerprint: ['aa'],
+    });
+    scope.addBreadcrumb(breadcrumb1);
+    scope.addEventProcessor(eventProcessor1);
+    scope.addAttachment(attachment1);
+
+    const globalScope = getGlobalScope();
+
+    globalScope.addBreadcrumb(breadcrumb2);
+    globalScope.addEventProcessor(eventProcessor2);
+    globalScope.setSDKProcessingMetadata({ aa: 'aa' });
+    globalScope.addAttachment(attachment2);
+
+    const event = { message: 'foo', breadcrumbs: [breadcrumb3], fingerprint: ['dd'] };
+
+    const options = {} as ClientOptions;
+    const processedEvent = await prepareEvent(
+      options,
+      event,
+      {
+        integrations: [],
+      },
+      scope,
+    );
+
+    expect(eventProcessor1).toHaveBeenCalledTimes(1);
+    expect(eventProcessor2).toHaveBeenCalledTimes(1);
+
+    // Test that attachments are correctly merged
+    expect(eventProcessor1).toHaveBeenCalledWith(processedEvent, {
+      integrations: [],
+      attachments: [attachment2, attachment1],
+    });
+
+    expect(processedEvent).toEqual({
+      timestamp: expect.any(Number),
+      event_id: expect.any(String),
+      environment: 'production',
+      message: 'foo',
+      user: { id: '1', email: '[email protected]' },
+      tags: { tag1: 'aa', tag2: 'aa' },
+      extra: { extra1: 'aa', extra2: 'aa' },
+      contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } },
+      fingerprint: ['dd', 'aa'],
+      breadcrumbs: [breadcrumb3, breadcrumb2, breadcrumb1],
+      sdkProcessingMetadata: {
+        aa: 'aa',
+        propagationContext: {
+          spanId: '1',
+          traceId: '1',
+        },
+      },
+    });
+  });
+
+  it('works without a scope', async () => {
+    const breadcrumb1 = { message: '1', timestamp: 111 } as Breadcrumb;
+    const breadcrumb2 = { message: '2', timestamp: 222 } as Breadcrumb;
+
+    const eventProcessor1 = jest.fn((a: unknown) => a) as EventProcessor;
+
+    const attachment1 = { filename: '1' } as Attachment;
+    const attachment2 = { filename: '2' } as Attachment;
+
+    const globalScope = getGlobalScope();
+
+    globalScope.addBreadcrumb(breadcrumb1);
+    globalScope.addEventProcessor(eventProcessor1);
+    globalScope.setSDKProcessingMetadata({ aa: 'aa' });
+    globalScope.addAttachment(attachment1);
+
+    const event = { message: 'foo', breadcrumbs: [breadcrumb2], fingerprint: ['dd'] };
+
+    const options = {} as ClientOptions;
+    const processedEvent = await prepareEvent(options, event, {
+      integrations: [],
+      attachments: [attachment2],
+    });
+
+    expect(eventProcessor1).toHaveBeenCalledTimes(1);
+
+    // Test that attachments are correctly merged
+    expect(eventProcessor1).toHaveBeenCalledWith(processedEvent, {
+      integrations: [],
+      attachments: [attachment2, attachment1],
+    });
+
+    expect(processedEvent).toEqual({
+      timestamp: expect.any(Number),
+      event_id: expect.any(String),
+      environment: 'production',
+      message: 'foo',
+      fingerprint: ['dd'],
+      breadcrumbs: [breadcrumb2, breadcrumb1],
+      sdkProcessingMetadata: {
+        aa: 'aa',
+        propagationContext: globalScope.getPropagationContext(),
+      },
+    });
+  });
+});