From a8b657f95556a8c04ec95ed54717245bc1a1a11f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 19 Dec 2023 17:14:56 +0100 Subject: [PATCH 1/4] feat(core): Add `getGlobalScope()` method This scope lives in module scope and is applied to _all_ events. --- packages/core/src/globals.ts | 22 ++ packages/core/src/index.ts | 5 +- packages/core/src/scope.ts | 34 ++- .../core/src/utils/applyScopeDataToEvent.ts | 98 +++++++++ packages/core/test/lib/scope.test.ts | 204 ++++++++++++++++++ .../lib/utils/applyScopeDataToEvent.test.ts | 201 +++++++++++++++++ packages/hub/test/scope.test.ts | 36 ++-- packages/node-experimental/src/sdk/scope.ts | 153 ++----------- packages/node-experimental/src/sdk/types.ts | 1 - .../test/integration/scope.test.ts | 15 +- .../node-experimental/test/sdk/scope.test.ts | 197 +---------------- packages/types/src/scope.ts | 3 + 12 files changed, 605 insertions(+), 364 deletions(-) create mode 100644 packages/core/src/globals.ts create mode 100644 packages/core/test/lib/scope.test.ts create mode 100644 packages/core/test/lib/utils/applyScopeDataToEvent.test.ts diff --git a/packages/core/src/globals.ts b/packages/core/src/globals.ts new file mode 100644 index 000000000000..e889b4fa0709 --- /dev/null +++ b/packages/core/src/globals.ts @@ -0,0 +1,22 @@ +import type { Scope } from '@sentry/types'; + +interface GlobalData { + globalScope?: Scope; +} + +const GLOBAL_DATA: GlobalData = {}; + +/** + * Get the global data. + */ +export function getGlobalData(): GlobalData { + return GLOBAL_DATA; +} + +/** + * Reset all global data. + * Mostly useful for tests. + */ +export function clearGlobalData(): void { + delete GLOBAL_DATA.globalScope; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 08f18f38ade6..400aac801ebb 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -42,7 +42,8 @@ export { } from './hub'; export { makeSession, closeSession, updateSession } from './session'; export { SessionFlusher } from './sessionflusher'; -export { Scope } from './scope'; +export { Scope, getGlobalScope } from './scope'; +export { clearGlobalData, getGlobalData } from './globals'; export { notifyEventProcessors, // eslint-disable-next-line deprecation/deprecation @@ -63,7 +64,7 @@ export { convertIntegrationFnToClass, } from './integration'; export { FunctionToString, InboundFilters, LinkedErrors } from './integrations'; -export { applyScopeDataToEvent } from './utils/applyScopeDataToEvent'; +export { applyScopeDataToEvent, mergeScopeData } from './utils/applyScopeDataToEvent'; export { prepareEvent } from './utils/prepareEvent'; export { createCheckInEnvelope } from './checkin'; export { hasTracingEnabled } from './utils/hasTracingEnabled'; diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 599b5c0f8d57..7919e67e2b18 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -23,11 +23,12 @@ import type { Transaction, User, } from '@sentry/types'; -import { arrayify, dateTimestampInSeconds, isPlainObject, uuid4 } from '@sentry/utils'; +import { dateTimestampInSeconds, isPlainObject, uuid4 } from '@sentry/utils'; import { getGlobalEventProcessors, notifyEventProcessors } from './eventProcessors'; +import { getGlobalData } from './globals'; import { updateSession } from './session'; -import { applyScopeDataToEvent } from './utils/applyScopeDataToEvent'; +import { applyScopeDataToEvent, mergeScopeData } from './utils/applyScopeDataToEvent'; /** * Default value for maximum number of breadcrumbs added to an event. @@ -457,7 +458,9 @@ export class Scope implements ScopeInterface { * @inheritDoc */ public getAttachments(): Attachment[] { - return this._attachments; + const data = this.getScopeData(); + + return data.attachments; } /** @@ -469,7 +472,7 @@ export class Scope implements ScopeInterface { } /** @inheritDoc */ - public getScopeData(): ScopeData { + public getPerScopeData(): ScopeData { const { _breadcrumbs, _attachments, @@ -503,6 +506,16 @@ 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. * @@ -570,6 +583,19 @@ export class Scope implements ScopeInterface { } } +/** + * Get the global scope. + * This scope is applied to _all_ events. + */ +export function getGlobalScope(): ScopeInterface { + const globalData = getGlobalData(); + if (!globalData.globalScope) { + globalData.globalScope = new Scope(); + } + + return globalData.globalScope; +} + function generatePropagationContext(): PropagationContext { return { traceId: uuid4(), diff --git a/packages/core/src/utils/applyScopeDataToEvent.ts b/packages/core/src/utils/applyScopeDataToEvent.ts index cc63e7c26cb6..1dab87aea866 100644 --- a/packages/core/src/utils/applyScopeDataToEvent.ts +++ b/packages/core/src/utils/applyScopeDataToEvent.ts @@ -22,6 +22,104 @@ export function applyScopeDataToEvent(event: Event, data: ScopeData): void { applySdkMetadataToEvent(event, sdkProcessingMetadata, propagationContext); } +/** Merge data of two scopes together. */ +export function mergeScopeData(data: ScopeData, mergeData: ScopeData): void { + const { + extra, + tags, + user, + contexts, + level, + sdkProcessingMetadata, + breadcrumbs, + fingerprint, + eventProcessors, + attachments, + propagationContext, + transactionName, + span, + } = mergeData; + + mergePropOverwrite(data, 'extra', extra); + mergePropOverwrite(data, 'tags', tags); + mergePropOverwrite(data, 'user', user); + mergePropOverwrite(data, 'contexts', contexts); + mergePropOverwrite(data, 'sdkProcessingMetadata', sdkProcessingMetadata); + + if (level) { + data.level = level; + } + + if (transactionName) { + data.transactionName = transactionName; + } + + if (span) { + data.span = span; + } + + if (breadcrumbs.length) { + data.breadcrumbs = [...data.breadcrumbs, ...breadcrumbs]; + } + + if (fingerprint.length) { + data.fingerprint = [...data.fingerprint, ...fingerprint]; + } + + if (eventProcessors.length) { + data.eventProcessors = [...data.eventProcessors, ...eventProcessors]; + } + + if (attachments.length) { + data.attachments = [...data.attachments, ...attachments]; + } + + data.propagationContext = { ...data.propagationContext, ...propagationContext }; +} + +/** + * Merge properties, overwriting existing keys. + * Exported only for tests. + */ +export function mergePropOverwrite< + Prop extends 'extra' | 'tags' | 'user' | 'contexts' | 'sdkProcessingMetadata', + Data extends ScopeData | Event, +>(data: Data, prop: Prop, mergeVal: Data[Prop]): void { + if (mergeVal && Object.keys(mergeVal).length) { + data[prop] = { ...data[prop], ...mergeVal }; + } +} + +/** + * Merge properties, keeping existing keys. + * Exported only for tests. + */ +export function mergePropKeep< + Prop extends 'extra' | 'tags' | 'user' | 'contexts' | 'sdkProcessingMetadata', + Data extends ScopeData | Event, +>(data: Data, prop: Prop, mergeVal: Data[Prop]): void { + if (mergeVal && Object.keys(mergeVal).length) { + data[prop] = { ...mergeVal, ...data[prop] }; + } +} + +/** Exported only for tests */ +export function mergeArray( + event: Event, + prop: Prop, + mergeVal: ScopeData[Prop], +): void { + const prevVal = event[prop]; + // If we are not merging any new values, + // we only need to proceed if there was an empty array before (as we want to replace it with undefined) + if (!mergeVal.length && (!prevVal || prevVal.length)) { + return; + } + + const merged = [...(prevVal || []), ...mergeVal] as ScopeData[Prop]; + event[prop] = merged.length ? merged : undefined; +} + function applyDataToEvent(event: Event, data: ScopeData): void { const { extra, tags, user, contexts, level, transactionName } = data; diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts new file mode 100644 index 000000000000..860e6caf7980 --- /dev/null +++ b/packages/core/test/lib/scope.test.ts @@ -0,0 +1,204 @@ +import { applyScopeDataToEvent } from '@sentry/core'; +import type { Attachment, Breadcrumb, EventProcessor } from '@sentry/types'; +import { clearGlobalData } from '../../src/globals'; +import { Scope, getGlobalScope } from '../../src/scope'; + +describe('Unit | Scope', () => { + beforeEach(() => { + clearGlobalData(); + }); + + it('allows to create & update a scope', () => { + const scope = new Scope(); + + expect(scope.getScopeData()).toEqual({ + breadcrumbs: [], + attachments: [], + contexts: {}, + tags: {}, + extra: {}, + user: {}, + level: undefined, + fingerprint: [], + eventProcessors: [], + propagationContext: { + traceId: expect.any(String), + spanId: expect.any(String), + }, + sdkProcessingMetadata: {}, + }); + + scope.update({ + tags: { foo: 'bar' }, + extra: { foo2: 'bar2' }, + }); + + expect(scope.getScopeData()).toEqual({ + breadcrumbs: [], + attachments: [], + contexts: {}, + tags: { + foo: 'bar', + }, + extra: { + foo2: 'bar2', + }, + user: {}, + level: undefined, + fingerprint: [], + eventProcessors: [], + propagationContext: { + traceId: expect.any(String), + spanId: expect.any(String), + }, + sdkProcessingMetadata: {}, + }); + }); + + it('allows to clone a scope', () => { + const scope = new Scope(); + + scope.update({ + tags: { foo: 'bar' }, + extra: { foo2: 'bar2' }, + }); + + const newScope = scope.clone(); + expect(newScope).toBeInstanceOf(Scope); + expect(newScope).not.toBe(scope); + + expect(newScope.getScopeData()).toEqual({ + breadcrumbs: [], + attachments: [], + contexts: {}, + tags: { + foo: 'bar', + }, + extra: { + foo2: 'bar2', + }, + user: {}, + level: undefined, + fingerprint: [], + eventProcessors: [], + propagationContext: { + traceId: expect.any(String), + spanId: expect.any(String), + }, + sdkProcessingMetadata: {}, + }); + }); + + describe('global scope', () => { + beforeEach(() => { + clearGlobalData(); + }); + + it('works', () => { + const globalScope = getGlobalScope(); + expect(globalScope).toBeDefined(); + expect(globalScope).toBeInstanceOf(Scope); + + // Repeatedly returns the same instance + expect(getGlobalScope()).toBe(globalScope); + + globalScope.setTag('tag1', 'val1'); + globalScope.setTag('tag2', 'val2'); + + expect(globalScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + }); + }); + + describe('applyScopeDataToEvent', () => { + it('works without any data', async () => { + const scope = new Scope(); + + const event = { message: 'foo' }; + applyScopeDataToEvent(event, scope.getScopeData()); + + expect(event).toEqual({ + 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: '4', timestamp: 333 } as Breadcrumb; + + const eventProcessor1 = jest.fn((a: unknown) => a) as EventProcessor; + const eventProcessor2 = jest.fn((b: unknown) => b) as EventProcessor; + + const scope = new Scope(); + scope.update({ + user: { id: '1', email: 'test@example.com' }, + 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.setSDKProcessingMetadata({ aa: 'aa' }); + + const globalScope = getGlobalScope(); + + globalScope.addBreadcrumb(breadcrumb2); + globalScope.addEventProcessor(eventProcessor2); + globalScope.setSDKProcessingMetadata({ bb: 'bb' }); + + const event = { message: 'foo', breadcrumbs: [breadcrumb3], fingerprint: ['dd'] }; + + applyScopeDataToEvent(event, scope.getScopeData()); + + expect(event).toEqual({ + message: 'foo', + user: { id: '1', email: 'test@example.com' }, + 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', + bb: 'bb', + propagationContext: { + spanId: '1', + traceId: '1', + }, + }, + }); + }); + }); + + describe('getAttachments', () => { + it('works without any data', async () => { + const scope = new Scope(); + + const actual = scope.getAttachments(); + expect(actual).toEqual([]); + }); + + it('merges attachments data', async () => { + const attachment1 = { filename: '1' } as Attachment; + const attachment2 = { filename: '2' } as Attachment; + + const scope = new Scope(); + scope.addAttachment(attachment1); + + const globalScope = getGlobalScope(); + + globalScope.addAttachment(attachment2); + + const actual = scope.getAttachments(); + expect(actual).toEqual([attachment2, attachment1]); + }); + }); +}); diff --git a/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts b/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts new file mode 100644 index 000000000000..f4fa38ee8b8b --- /dev/null +++ b/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts @@ -0,0 +1,201 @@ +import type { Attachment, Breadcrumb, EventProcessor, ScopeData } from '@sentry/types'; +import { + mergeArray, + mergePropKeep, + mergePropOverwrite, + mergeScopeData, +} from '../../../src/utils/applyScopeDataToEvent'; + +describe('mergeArray', () => { + it.each([ + [[], [], undefined], + [undefined, [], undefined], + [['a'], [], ['a']], + [['a'], ['b', 'c'], ['a', 'b', 'c']], + [[], ['b', 'c'], ['b', 'c']], + [undefined, ['b', 'c'], ['b', 'c']], + ])('works with %s and %s', (a, b, expected) => { + const data = { fingerprint: a }; + mergeArray(data, 'fingerprint', b); + expect(data.fingerprint).toEqual(expected); + }); + + it('does not mutate the original array if no changes are made', () => { + const fingerprint = ['a']; + const data = { fingerprint }; + mergeArray(data, 'fingerprint', []); + expect(data.fingerprint).toBe(fingerprint); + }); +}); + +describe('mergePropKeep', () => { + it.each([ + [{}, {}, {}], + [{ a: 'aa' }, {}, { a: 'aa' }], + [{ a: 'aa' }, { b: 'bb' }, { a: 'aa', b: 'bb' }], + // Does not overwrite existing keys + [{ a: 'aa' }, { b: 'bb', a: 'cc' }, { a: 'aa', b: 'bb' }], + ])('works with %s and %s', (a, b, expected) => { + const data = { tags: a } as unknown as ScopeData; + mergePropKeep(data, 'tags', b); + expect(data.tags).toEqual(expected); + }); + + it('does not deep merge', () => { + const data = { + contexts: { + app: { app_version: 'v1' }, + culture: { display_name: 'name1' }, + }, + } as unknown as ScopeData; + mergePropKeep(data, 'contexts', { + os: { name: 'os1' }, + app: { app_name: 'name1' }, + }); + expect(data.contexts).toEqual({ + os: { name: 'os1' }, + culture: { display_name: 'name1' }, + app: { app_version: 'v1' }, + }); + }); + + it('does not mutate the original object if no changes are made', () => { + const tags = { a: 'aa' }; + const data = { tags } as unknown as ScopeData; + mergePropKeep(data, 'tags', {}); + expect(data.tags).toBe(tags); + }); +}); + +describe('mergePropOverwrite', () => { + it.each([ + [{}, {}, {}], + [{ a: 'aa' }, {}, { a: 'aa' }], + [{ a: 'aa' }, { b: 'bb' }, { a: 'aa', b: 'bb' }], + // overwrites existing keys + [{ a: 'aa' }, { b: 'bb', a: 'cc' }, { a: 'cc', b: 'bb' }], + ])('works with %s and %s', (a, b, expected) => { + const data = { tags: a } as unknown as ScopeData; + mergePropOverwrite(data, 'tags', b); + expect(data.tags).toEqual(expected); + }); + + it('does not deep merge', () => { + const data = { + contexts: { + app: { app_version: 'v1' }, + culture: { display_name: 'name1' }, + }, + } as unknown as ScopeData; + mergePropOverwrite(data, 'contexts', { + os: { name: 'os1' }, + app: { app_name: 'name1' }, + }); + expect(data.contexts).toEqual({ + os: { name: 'os1' }, + culture: { display_name: 'name1' }, + app: { app_name: 'name1' }, + }); + }); + + it('does not mutate the original object if no changes are made', () => { + const tags = { a: 'aa' }; + const data = { tags } as unknown as ScopeData; + mergePropOverwrite(data, 'tags', {}); + expect(data.tags).toBe(tags); + }); +}); + +describe('mergeScopeData', () => { + it('works with empty data', () => { + const data1: ScopeData = { + eventProcessors: [], + breadcrumbs: [], + user: {}, + tags: {}, + extra: {}, + contexts: {}, + attachments: [], + propagationContext: { spanId: '1', traceId: '1' }, + sdkProcessingMetadata: {}, + fingerprint: [], + }; + const data2: ScopeData = { + eventProcessors: [], + breadcrumbs: [], + user: {}, + tags: {}, + extra: {}, + contexts: {}, + attachments: [], + propagationContext: { spanId: '1', traceId: '1' }, + sdkProcessingMetadata: {}, + fingerprint: [], + }; + mergeScopeData(data1, data2); + expect(data1).toEqual({ + eventProcessors: [], + breadcrumbs: [], + user: {}, + tags: {}, + extra: {}, + contexts: {}, + attachments: [], + propagationContext: { spanId: '1', traceId: '1' }, + sdkProcessingMetadata: {}, + fingerprint: [], + }); + }); + + it('merges data correctly', () => { + const attachment1 = { filename: '1' } as Attachment; + const attachment2 = { filename: '2' } as Attachment; + const attachment3 = { filename: '3' } as Attachment; + + const breadcrumb1 = { message: '1' } as Breadcrumb; + const breadcrumb2 = { message: '2' } as Breadcrumb; + const breadcrumb3 = { message: '3' } as Breadcrumb; + + const eventProcessor1 = ((a: unknown) => null) as EventProcessor; + const eventProcessor2 = ((b: unknown) => null) as EventProcessor; + const eventProcessor3 = ((c: unknown) => null) as EventProcessor; + + const data1: ScopeData = { + eventProcessors: [eventProcessor1], + breadcrumbs: [breadcrumb1], + user: { id: '1', email: 'test@example.com' }, + tags: { tag1: 'aa', tag2: 'aa' }, + extra: { extra1: 'aa', extra2: 'aa' }, + contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } }, + attachments: [attachment1], + propagationContext: { spanId: '1', traceId: '1' }, + sdkProcessingMetadata: { aa: 'aa', bb: 'aa' }, + fingerprint: ['aa', 'bb'], + }; + const data2: ScopeData = { + eventProcessors: [eventProcessor2, eventProcessor3], + breadcrumbs: [breadcrumb2, breadcrumb3], + user: { id: '2', name: 'foo' }, + tags: { tag2: 'bb', tag3: 'bb' }, + extra: { extra2: 'bb', extra3: 'bb' }, + contexts: { os: { name: 'os2' } }, + attachments: [attachment2, attachment3], + propagationContext: { spanId: '2', traceId: '2' }, + sdkProcessingMetadata: { bb: 'bb', cc: 'bb' }, + fingerprint: ['cc'], + }; + mergeScopeData(data1, data2); + expect(data1).toEqual({ + eventProcessors: [eventProcessor1, eventProcessor2, eventProcessor3], + breadcrumbs: [breadcrumb1, breadcrumb2, breadcrumb3], + user: { id: '2', name: 'foo', email: 'test@example.com' }, + tags: { tag1: 'aa', tag2: 'bb', tag3: 'bb' }, + extra: { extra1: 'aa', extra2: 'bb', extra3: 'bb' }, + contexts: { os: { name: 'os2' }, culture: { display_name: 'name1' } }, + attachments: [attachment1, attachment2, attachment3], + propagationContext: { spanId: '2', traceId: '2' }, + sdkProcessingMetadata: { aa: 'aa', bb: 'bb', cc: 'bb' }, + fingerprint: ['aa', 'bb', 'cc'], + }); + }); +}); diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts index a039a839dcc6..a2be18b0526d 100644 --- a/packages/hub/test/scope.test.ts +++ b/packages/hub/test/scope.test.ts @@ -237,8 +237,6 @@ describe('Scope', () => { describe('applyToEvent', () => { test('basic usage', async () => { - expect.assertions(9); - const scope = new Scope(); scope.setExtra('a', 2); scope.setTag('a', 'b'); @@ -251,20 +249,20 @@ describe('Scope', () => { scope.setSDKProcessingMetadata({ dogs: 'are great!' }); const event: Event = {}; - return scope.applyToEvent(event).then(processedEvent => { - expect(processedEvent!.extra).toEqual({ a: 2 }); - expect(processedEvent!.tags).toEqual({ a: 'b' }); - expect(processedEvent!.user).toEqual({ id: '1' }); - expect(processedEvent!.fingerprint).toEqual(['abcd']); - expect(processedEvent!.level).toEqual('warning'); - expect(processedEvent!.transaction).toEqual('/abc'); - expect(processedEvent!.breadcrumbs![0]).toHaveProperty('message', 'test'); - expect(processedEvent!.contexts).toEqual({ os: { id: '1' } }); - expect(processedEvent!.sdkProcessingMetadata).toEqual({ - dogs: 'are great!', - // @ts-expect-error accessing private property for test - propagationContext: scope._propagationContext, - }); + const processedEvent = await scope.applyToEvent(event); + + expect(processedEvent!.extra).toEqual({ a: 2 }); + expect(processedEvent!.tags).toEqual({ a: 'b' }); + expect(processedEvent!.user).toEqual({ id: '1' }); + expect(processedEvent!.fingerprint).toEqual(['abcd']); + expect(processedEvent!.level).toEqual('warning'); + expect(processedEvent!.transaction).toEqual('/abc'); + expect(processedEvent!.breadcrumbs![0]).toHaveProperty('message', 'test'); + expect(processedEvent!.contexts).toEqual({ os: { id: '1' } }); + expect(processedEvent!.sdkProcessingMetadata).toEqual({ + dogs: 'are great!', + // @ts-expect-error accessing private property for test + propagationContext: scope._propagationContext, }); }); @@ -360,7 +358,6 @@ describe('Scope', () => { }); test('adds trace context', async () => { - expect.assertions(1); const scope = new Scope(); const span = { fake: 'span', @@ -368,9 +365,8 @@ describe('Scope', () => { } as any; scope.setSpan(span); const event: Event = {}; - return scope.applyToEvent(event).then(processedEvent => { - expect((processedEvent!.contexts!.trace as any).a).toEqual('b'); - }); + const processedEvent = await scope.applyToEvent(event); + expect((processedEvent!.contexts!.trace as any).a).toEqual('b'); }); test('existing trace context in event should take precedence', async () => { diff --git a/packages/node-experimental/src/sdk/scope.ts b/packages/node-experimental/src/sdk/scope.ts index 5bf220708a6a..cb1247ed5b31 100644 --- a/packages/node-experimental/src/sdk/scope.ts +++ b/packages/node-experimental/src/sdk/scope.ts @@ -1,5 +1,6 @@ +import { getGlobalData, mergeScopeData } from '@sentry/core'; import { OpenTelemetryScope } from '@sentry/opentelemetry'; -import type { Attachment, Breadcrumb, Client, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; +import type { Breadcrumb, Client, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; import { uuid4 } from '@sentry/utils'; import { getGlobalCarrier } from './globals'; @@ -18,15 +19,25 @@ export function setCurrentScope(scope: Scope): void { getScopes().scope = scope; } -/** Get the global scope. */ +/** + * Get the global scope. + * We overwrite this from the core implementation to make sure we get the correct Scope class. + */ export function getGlobalScope(): Scope { - const carrier = getGlobalCarrier(); + const globalData = getGlobalData(); + + if (!globalData.globalScope) { + globalData.globalScope = new Scope(); + } - if (!carrier.globalScope) { - carrier.globalScope = new Scope(); + // If we have a default Scope here by chance, make sure to "upgrade" it to our custom Scope + if (!(globalData.globalScope instanceof Scope)) { + const oldScope = globalData.globalScope; + globalData.globalScope = new Scope(); + globalData.globalScope.update(oldScope); } - return carrier.globalScope as Scope; + return globalData.globalScope as Scope; } /** Get the currently active isolation scope. */ @@ -104,13 +115,6 @@ export class Scope extends OpenTelemetryScope implements ScopeInterface { return this._client; } - /** @inheritdoc */ - public getAttachments(): Attachment[] { - const data = this.getScopeData(); - - return data.attachments; - } - /** Capture an exception for this scope. */ public captureException(exception: unknown, hint?: EventHint): string { const eventId = hint && hint.event_id ? hint.event_id : uuid4(); @@ -183,37 +187,6 @@ export class Scope extends OpenTelemetryScope implements ScopeInterface { return this._addBreadcrumb(breadcrumb, maxBreadcrumbs); } - /** Get all relevant data for this scope. */ - public getPerScopeData(): ScopeData { - const { - _breadcrumbs, - _attachments, - _contexts, - _tags, - _extra, - _user, - _level, - _fingerprint, - _eventProcessors, - _propagationContext, - _sdkProcessingMetadata, - } = this; - - return { - breadcrumbs: _breadcrumbs, - attachments: _attachments, - contexts: _contexts, - tags: _tags, - extra: _extra, - user: _user, - level: _level, - fingerprint: _fingerprint || [], - eventProcessors: _eventProcessors, - propagationContext: _propagationContext, - sdkProcessingMetadata: _sdkProcessingMetadata, - }; - } - /** @inheritdoc */ public getScopeData(): ScopeData { const data = getGlobalScope().getPerScopeData(); @@ -221,8 +194,8 @@ export class Scope extends OpenTelemetryScope implements ScopeInterface { const scopeData = this.getPerScopeData(); // Merge data together, in order - mergeData(data, isolationScopeData); - mergeData(data, scopeData); + mergeScopeData(data, isolationScopeData); + mergeScopeData(data, scopeData); return data; } @@ -233,94 +206,6 @@ export class Scope extends OpenTelemetryScope implements ScopeInterface { } } -/** Exported only for tests */ -export function mergeData(data: ScopeData, mergeData: ScopeData): void { - const { - extra, - tags, - user, - contexts, - level, - sdkProcessingMetadata, - breadcrumbs, - fingerprint, - eventProcessors, - attachments, - propagationContext, - } = mergeData; - - mergePropOverwrite(data, 'extra', extra); - mergePropOverwrite(data, 'tags', tags); - mergePropOverwrite(data, 'user', user); - mergePropOverwrite(data, 'contexts', contexts); - mergePropOverwrite(data, 'sdkProcessingMetadata', sdkProcessingMetadata); - - if (level) { - data.level = level; - } - - if (breadcrumbs.length) { - data.breadcrumbs = [...data.breadcrumbs, ...breadcrumbs]; - } - - if (fingerprint.length) { - data.fingerprint = [...data.fingerprint, ...fingerprint]; - } - - if (eventProcessors.length) { - data.eventProcessors = [...data.eventProcessors, ...eventProcessors]; - } - - if (attachments.length) { - data.attachments = [...data.attachments, ...attachments]; - } - - data.propagationContext = { ...data.propagationContext, ...propagationContext }; -} - -/** - * Merge properties, overwriting existing keys. - * Exported only for tests. - */ -export function mergePropOverwrite< - Prop extends 'extra' | 'tags' | 'user' | 'contexts' | 'sdkProcessingMetadata', - Data extends ScopeData | Event, ->(data: Data, prop: Prop, mergeVal: Data[Prop]): void { - if (mergeVal && Object.keys(mergeVal).length) { - data[prop] = { ...data[prop], ...mergeVal }; - } -} - -/** - * Merge properties, keeping existing keys. - * Exported only for tests. - */ -export function mergePropKeep< - Prop extends 'extra' | 'tags' | 'user' | 'contexts' | 'sdkProcessingMetadata', - Data extends ScopeData | Event, ->(data: Data, prop: Prop, mergeVal: Data[Prop]): void { - if (mergeVal && Object.keys(mergeVal).length) { - data[prop] = { ...mergeVal, ...data[prop] }; - } -} - -/** Exported only for tests */ -export function mergeArray( - event: Event, - prop: Prop, - mergeVal: ScopeData[Prop], -): void { - const prevVal = event[prop]; - // If we are not merging any new values, - // we only need to proceed if there was an empty array before (as we want to replace it with undefined) - if (!mergeVal.length && (!prevVal || prevVal.length)) { - return; - } - - const merged = [...(prevVal || []), ...mergeVal] as ScopeData[Prop]; - event[prop] = merged.length ? merged : undefined; -} - function getScopes(): CurrentScopes { const carrier = getGlobalCarrier(); diff --git a/packages/node-experimental/src/sdk/types.ts b/packages/node-experimental/src/sdk/types.ts index 773c404d65ce..140056e583ca 100644 --- a/packages/node-experimental/src/sdk/types.ts +++ b/packages/node-experimental/src/sdk/types.ts @@ -74,7 +74,6 @@ export interface AsyncContextStrategy { } export interface SentryCarrier { - globalScope?: Scope; scopes?: CurrentScopes; acs?: AsyncContextStrategy; diff --git a/packages/node-experimental/test/integration/scope.test.ts b/packages/node-experimental/test/integration/scope.test.ts index 57be6126bcae..d5f1d3156a11 100644 --- a/packages/node-experimental/test/integration/scope.test.ts +++ b/packages/node-experimental/test/integration/scope.test.ts @@ -1,3 +1,4 @@ +import { clearGlobalData } from '@sentry/core'; import { getCurrentHub, getSpanScope } from '@sentry/opentelemetry'; import * as Sentry from '../../src/'; @@ -230,7 +231,7 @@ describe('Integration | Scope', () => { describe('global scope', () => { beforeEach(() => { - resetGlobals(); + clearGlobalData(); }); it('works before calling init', () => { @@ -245,14 +246,14 @@ describe('Integration | Scope', () => { globalScope.setTag('tag1', 'val1'); globalScope.setTag('tag2', 'val2'); - expect(globalScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(globalScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); // Now when we call init, the global scope remains intact Sentry.init({ dsn: 'https://username@domain/123', defaultIntegrations: false }); expect(globalScope.getClient()).toBeDefined(); expect(Sentry.getGlobalScope()).toBe(globalScope); - expect(globalScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(globalScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); }); it('is applied to events', async () => { @@ -303,7 +304,7 @@ describe('Integration | Scope', () => { isolationScope.setTag('tag1', 'val1'); isolationScope.setTag('tag2', 'val2'); - expect(isolationScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(isolationScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); // Now when we call init, the isolation scope remains intact Sentry.init({ dsn: 'https://username@domain/123', defaultIntegrations: false }); @@ -311,7 +312,7 @@ describe('Integration | Scope', () => { // client is only attached to global scope by default expect(isolationScope.getClient()).toBeUndefined(); expect(Sentry.getIsolationScope()).toBe(isolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(isolationScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); }); it('is applied to events', async () => { @@ -367,13 +368,13 @@ describe('Integration | Scope', () => { expect(newIsolationScope).not.toBe(initialIsolationScope); // Data is forked off original isolation scope - expect(newIsolationScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(newIsolationScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); newIsolationScope.setTag('tag3', 'val3'); Sentry.captureException(error); }); - expect(initialIsolationScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(initialIsolationScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); await client.flush(); diff --git a/packages/node-experimental/test/sdk/scope.test.ts b/packages/node-experimental/test/sdk/scope.test.ts index 9fb5c02bea69..d1cc12b7463b 100644 --- a/packages/node-experimental/test/sdk/scope.test.ts +++ b/packages/node-experimental/test/sdk/scope.test.ts @@ -1,8 +1,7 @@ import { applyScopeDataToEvent } from '@sentry/core'; import type { Attachment, Breadcrumb, Client, EventProcessor } from '@sentry/types'; import { Scope, getIsolationScope } from '../../src'; -import { getGlobalScope, mergeArray, mergeData, mergePropKeep, mergePropOverwrite } from '../../src/sdk/scope'; -import type { ScopeData } from '../../src/sdk/types'; +import { getGlobalScope } from '../../src/sdk/scope'; import { mockSdkInit, resetGlobals } from '../helpers/mockSdkInit'; describe('Unit | Scope', () => { @@ -109,200 +108,6 @@ describe('Unit | Scope', () => { expect(scope['_getIsolationScope']()).toBe(customIsolationScope); }); - describe('mergeArray', () => { - it.each([ - [[], [], undefined], - [undefined, [], undefined], - [['a'], [], ['a']], - [['a'], ['b', 'c'], ['a', 'b', 'c']], - [[], ['b', 'c'], ['b', 'c']], - [undefined, ['b', 'c'], ['b', 'c']], - ])('works with %s and %s', (a, b, expected) => { - const data = { fingerprint: a }; - mergeArray(data, 'fingerprint', b); - expect(data.fingerprint).toEqual(expected); - }); - - it('does not mutate the original array if no changes are made', () => { - const fingerprint = ['a']; - const data = { fingerprint }; - mergeArray(data, 'fingerprint', []); - expect(data.fingerprint).toBe(fingerprint); - }); - }); - - describe('mergePropKeep', () => { - it.each([ - [{}, {}, {}], - [{ a: 'aa' }, {}, { a: 'aa' }], - [{ a: 'aa' }, { b: 'bb' }, { a: 'aa', b: 'bb' }], - // Does not overwrite existing keys - [{ a: 'aa' }, { b: 'bb', a: 'cc' }, { a: 'aa', b: 'bb' }], - ])('works with %s and %s', (a, b, expected) => { - const data = { tags: a } as unknown as ScopeData; - mergePropKeep(data, 'tags', b); - expect(data.tags).toEqual(expected); - }); - - it('does not deep merge', () => { - const data = { - contexts: { - app: { app_version: 'v1' }, - culture: { display_name: 'name1' }, - }, - } as unknown as ScopeData; - mergePropKeep(data, 'contexts', { - os: { name: 'os1' }, - app: { app_name: 'name1' }, - }); - expect(data.contexts).toEqual({ - os: { name: 'os1' }, - culture: { display_name: 'name1' }, - app: { app_version: 'v1' }, - }); - }); - - it('does not mutate the original object if no changes are made', () => { - const tags = { a: 'aa' }; - const data = { tags } as unknown as ScopeData; - mergePropKeep(data, 'tags', {}); - expect(data.tags).toBe(tags); - }); - }); - - describe('mergePropOverwrite', () => { - it.each([ - [{}, {}, {}], - [{ a: 'aa' }, {}, { a: 'aa' }], - [{ a: 'aa' }, { b: 'bb' }, { a: 'aa', b: 'bb' }], - // overwrites existing keys - [{ a: 'aa' }, { b: 'bb', a: 'cc' }, { a: 'cc', b: 'bb' }], - ])('works with %s and %s', (a, b, expected) => { - const data = { tags: a } as unknown as ScopeData; - mergePropOverwrite(data, 'tags', b); - expect(data.tags).toEqual(expected); - }); - - it('does not deep merge', () => { - const data = { - contexts: { - app: { app_version: 'v1' }, - culture: { display_name: 'name1' }, - }, - } as unknown as ScopeData; - mergePropOverwrite(data, 'contexts', { - os: { name: 'os1' }, - app: { app_name: 'name1' }, - }); - expect(data.contexts).toEqual({ - os: { name: 'os1' }, - culture: { display_name: 'name1' }, - app: { app_name: 'name1' }, - }); - }); - - it('does not mutate the original object if no changes are made', () => { - const tags = { a: 'aa' }; - const data = { tags } as unknown as ScopeData; - mergePropOverwrite(data, 'tags', {}); - expect(data.tags).toBe(tags); - }); - }); - - describe('mergeData', () => { - it('works with empty data', () => { - const data1: ScopeData = { - eventProcessors: [], - breadcrumbs: [], - user: {}, - tags: {}, - extra: {}, - contexts: {}, - attachments: [], - propagationContext: { spanId: '1', traceId: '1' }, - sdkProcessingMetadata: {}, - fingerprint: [], - }; - const data2: ScopeData = { - eventProcessors: [], - breadcrumbs: [], - user: {}, - tags: {}, - extra: {}, - contexts: {}, - attachments: [], - propagationContext: { spanId: '1', traceId: '1' }, - sdkProcessingMetadata: {}, - fingerprint: [], - }; - mergeData(data1, data2); - expect(data1).toEqual({ - eventProcessors: [], - breadcrumbs: [], - user: {}, - tags: {}, - extra: {}, - contexts: {}, - attachments: [], - propagationContext: { spanId: '1', traceId: '1' }, - sdkProcessingMetadata: {}, - fingerprint: [], - }); - }); - - it('merges data correctly', () => { - const attachment1 = { filename: '1' } as Attachment; - const attachment2 = { filename: '2' } as Attachment; - const attachment3 = { filename: '3' } as Attachment; - - const breadcrumb1 = { message: '1' } as Breadcrumb; - const breadcrumb2 = { message: '2' } as Breadcrumb; - const breadcrumb3 = { message: '3' } as Breadcrumb; - - const eventProcessor1 = ((a: unknown) => null) as EventProcessor; - const eventProcessor2 = ((b: unknown) => null) as EventProcessor; - const eventProcessor3 = ((c: unknown) => null) as EventProcessor; - - const data1: ScopeData = { - eventProcessors: [eventProcessor1], - breadcrumbs: [breadcrumb1], - user: { id: '1', email: 'test@example.com' }, - tags: { tag1: 'aa', tag2: 'aa' }, - extra: { extra1: 'aa', extra2: 'aa' }, - contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } }, - attachments: [attachment1], - propagationContext: { spanId: '1', traceId: '1' }, - sdkProcessingMetadata: { aa: 'aa', bb: 'aa' }, - fingerprint: ['aa', 'bb'], - }; - const data2: ScopeData = { - eventProcessors: [eventProcessor2, eventProcessor3], - breadcrumbs: [breadcrumb2, breadcrumb3], - user: { id: '2', name: 'foo' }, - tags: { tag2: 'bb', tag3: 'bb' }, - extra: { extra2: 'bb', extra3: 'bb' }, - contexts: { os: { name: 'os2' } }, - attachments: [attachment2, attachment3], - propagationContext: { spanId: '2', traceId: '2' }, - sdkProcessingMetadata: { bb: 'bb', cc: 'bb' }, - fingerprint: ['cc'], - }; - mergeData(data1, data2); - expect(data1).toEqual({ - eventProcessors: [eventProcessor1, eventProcessor2, eventProcessor3], - breadcrumbs: [breadcrumb1, breadcrumb2, breadcrumb3], - user: { id: '2', name: 'foo', email: 'test@example.com' }, - tags: { tag1: 'aa', tag2: 'bb', tag3: 'bb' }, - extra: { extra1: 'aa', extra2: 'bb', extra3: 'bb' }, - contexts: { os: { name: 'os2' }, culture: { display_name: 'name1' } }, - attachments: [attachment1, attachment2, attachment3], - propagationContext: { spanId: '2', traceId: '2' }, - sdkProcessingMetadata: { aa: 'aa', bb: 'bb', cc: 'bb' }, - fingerprint: ['aa', 'bb', 'cc'], - }); - }); - }); - describe('applyToEvent', () => { it('works without any data', async () => { mockSdkInit(); diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts index 50ef4da5987f..8cee38ca50ab 100644 --- a/packages/types/src/scope.ts +++ b/packages/types/src/scope.ts @@ -53,6 +53,9 @@ export interface Scope { /** Get the data of this scope, which is applied to an event during processing. */ getScopeData(): ScopeData; + /** Get the data of this scope only, ignoring all other related scopes. */ + getPerScopeData(): ScopeData; + /** * Updates user context information for future events. * From 783c49fb67a514ce61f8b68a0adc6581d56ff67a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 20 Dec 2023 11:30:03 +0100 Subject: [PATCH 2/4] ref: merge global scope in `prepareEvent` --- packages/core/src/scope.ts | 13 +- packages/core/src/utils/prepareEvent.ts | 44 ++-- packages/core/test/lib/base.test.ts | 12 +- packages/core/test/lib/prepareEvent.test.ts | 188 +++++++++++++++++- packages/core/test/lib/scope.test.ts | 31 +-- packages/node-experimental/src/sdk/scope.ts | 21 +- .../test/integration/scope.test.ts | 12 +- .../node-experimental/test/sdk/scope.test.ts | 102 ++++++---- packages/types/src/scope.ts | 3 - 9 files changed, 309 insertions(+), 117 deletions(-) diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 7919e67e2b18..9931f8d94279 100644 --- a/packages/core/src/scope.ts +++ b/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. * diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 76307b4f45e6..a4584c34066a 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/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); diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index baeb8bfd0044..9ba1d60efeab 100644 --- a/packages/core/test/lib/base.test.ts +++ b/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, diff --git a/packages/core/test/lib/prepareEvent.test.ts b/packages/core/test/lib/prepareEvent.test.ts index b98305dea604..9e2f69e0f654 100644 --- a/packages/core/test/lib/prepareEvent.test.ts +++ b/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: 'test@example.com' }, + 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: 'test@example.com' }, + 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(), + }, + }); + }); +}); diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts index 860e6caf7980..3bcdbf4022ce 100644 --- a/packages/core/test/lib/scope.test.ts +++ b/packages/core/test/lib/scope.test.ts @@ -127,13 +127,9 @@ describe('Unit | Scope', () => { }); }); - it('merges scope data', async () => { + it('works with data', async () => { const breadcrumb1 = { message: '1', timestamp: 111 } as Breadcrumb; - const breadcrumb2 = { message: '2', timestamp: 222 } as Breadcrumb; - const breadcrumb3 = { message: '4', timestamp: 333 } as Breadcrumb; - - const eventProcessor1 = jest.fn((a: unknown) => a) as EventProcessor; - const eventProcessor2 = jest.fn((b: unknown) => b) as EventProcessor; + const breadcrumb2 = { message: '1', timestamp: 111 } as Breadcrumb; const scope = new Scope(); scope.update({ @@ -145,16 +141,9 @@ describe('Unit | Scope', () => { fingerprint: ['aa'], }); scope.addBreadcrumb(breadcrumb1); - scope.addEventProcessor(eventProcessor1); scope.setSDKProcessingMetadata({ aa: 'aa' }); - const globalScope = getGlobalScope(); - - globalScope.addBreadcrumb(breadcrumb2); - globalScope.addEventProcessor(eventProcessor2); - globalScope.setSDKProcessingMetadata({ bb: 'bb' }); - - const event = { message: 'foo', breadcrumbs: [breadcrumb3], fingerprint: ['dd'] }; + const event = { message: 'foo', breadcrumbs: [breadcrumb2], fingerprint: ['dd'] }; applyScopeDataToEvent(event, scope.getScopeData()); @@ -165,10 +154,9 @@ describe('Unit | Scope', () => { extra: { extra1: 'aa', extra2: 'aa' }, contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } }, fingerprint: ['dd', 'aa'], - breadcrumbs: [breadcrumb3, breadcrumb2, breadcrumb1], + breadcrumbs: [breadcrumb2, breadcrumb1], sdkProcessingMetadata: { aa: 'aa', - bb: 'bb', propagationContext: { spanId: '1', traceId: '1', @@ -179,6 +167,7 @@ describe('Unit | Scope', () => { }); describe('getAttachments', () => { + /* eslint-disable deprecation/deprecation */ it('works without any data', async () => { const scope = new Scope(); @@ -186,19 +175,17 @@ describe('Unit | Scope', () => { expect(actual).toEqual([]); }); - it('merges attachments data', async () => { + it('works with attachments', async () => { const attachment1 = { filename: '1' } as Attachment; const attachment2 = { filename: '2' } as Attachment; const scope = new Scope(); scope.addAttachment(attachment1); - - const globalScope = getGlobalScope(); - - globalScope.addAttachment(attachment2); + scope.addAttachment(attachment2); const actual = scope.getAttachments(); - expect(actual).toEqual([attachment2, attachment1]); + expect(actual).toEqual([attachment1, attachment2]); }); + /* eslint-enable deprecation/deprecation */ }); }); diff --git a/packages/node-experimental/src/sdk/scope.ts b/packages/node-experimental/src/sdk/scope.ts index cb1247ed5b31..72b558c02e1f 100644 --- a/packages/node-experimental/src/sdk/scope.ts +++ b/packages/node-experimental/src/sdk/scope.ts @@ -187,14 +187,27 @@ export class Scope extends OpenTelemetryScope implements ScopeInterface { return this._addBreadcrumb(breadcrumb, maxBreadcrumbs); } + /** Get scope data for this scope only. */ + public getOwnScopeData(): ScopeData { + return super.getScopeData(); + } + /** @inheritdoc */ public getScopeData(): ScopeData { - const data = getGlobalScope().getPerScopeData(); - const isolationScopeData = this._getIsolationScope().getPerScopeData(); - const scopeData = this.getPerScopeData(); + const globalScope = getGlobalScope(); + const isolationScope = this._getIsolationScope(); + + // Special case: If this is the global/isolation scope, no need to merge other data in here + if (this === globalScope || this === isolationScope) { + return this.getOwnScopeData(); + } + + // Global scope is applied anyhow in prepareEvent, + // but we need to merge the isolation scope in here + const data = isolationScope.getOwnScopeData(); + const scopeData = this.getOwnScopeData(); // Merge data together, in order - mergeScopeData(data, isolationScopeData); mergeScopeData(data, scopeData); return data; diff --git a/packages/node-experimental/test/integration/scope.test.ts b/packages/node-experimental/test/integration/scope.test.ts index d5f1d3156a11..9604aa3c1ac3 100644 --- a/packages/node-experimental/test/integration/scope.test.ts +++ b/packages/node-experimental/test/integration/scope.test.ts @@ -246,14 +246,14 @@ describe('Integration | Scope', () => { globalScope.setTag('tag1', 'val1'); globalScope.setTag('tag2', 'val2'); - expect(globalScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(globalScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); // Now when we call init, the global scope remains intact Sentry.init({ dsn: 'https://username@domain/123', defaultIntegrations: false }); expect(globalScope.getClient()).toBeDefined(); expect(Sentry.getGlobalScope()).toBe(globalScope); - expect(globalScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(globalScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); }); it('is applied to events', async () => { @@ -304,7 +304,7 @@ describe('Integration | Scope', () => { isolationScope.setTag('tag1', 'val1'); isolationScope.setTag('tag2', 'val2'); - expect(isolationScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(isolationScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); // Now when we call init, the isolation scope remains intact Sentry.init({ dsn: 'https://username@domain/123', defaultIntegrations: false }); @@ -312,7 +312,7 @@ describe('Integration | Scope', () => { // client is only attached to global scope by default expect(isolationScope.getClient()).toBeUndefined(); expect(Sentry.getIsolationScope()).toBe(isolationScope); - expect(isolationScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(isolationScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); }); it('is applied to events', async () => { @@ -368,13 +368,13 @@ describe('Integration | Scope', () => { expect(newIsolationScope).not.toBe(initialIsolationScope); // Data is forked off original isolation scope - expect(newIsolationScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(newIsolationScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); newIsolationScope.setTag('tag3', 'val3'); Sentry.captureException(error); }); - expect(initialIsolationScope.getPerScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); + expect(initialIsolationScope.getScopeData().tags).toEqual({ tag1: 'val1', tag2: 'val2' }); await client.flush(); diff --git a/packages/node-experimental/test/sdk/scope.test.ts b/packages/node-experimental/test/sdk/scope.test.ts index d1cc12b7463b..c38437121959 100644 --- a/packages/node-experimental/test/sdk/scope.test.ts +++ b/packages/node-experimental/test/sdk/scope.test.ts @@ -1,5 +1,5 @@ -import { applyScopeDataToEvent } from '@sentry/core'; -import type { Attachment, Breadcrumb, Client, EventProcessor } from '@sentry/types'; +import { prepareEvent } from '@sentry/core'; +import type { Attachment, Breadcrumb, Client, ClientOptions, EventProcessor } from '@sentry/types'; import { Scope, getIsolationScope } from '../../src'; import { getGlobalScope } from '../../src/sdk/scope'; import { mockSdkInit, resetGlobals } from '../helpers/mockSdkInit'; @@ -108,16 +108,41 @@ describe('Unit | Scope', () => { expect(scope['_getIsolationScope']()).toBe(customIsolationScope); }); - describe('applyToEvent', () => { - it('works without any data', async () => { + describe('prepareEvent', () => { + it('works without any scope data', async () => { mockSdkInit(); + const eventProcessor = jest.fn((a: unknown) => a) as EventProcessor; + const scope = new Scope(); const event = { message: 'foo' }; - applyScopeDataToEvent(event, scope.getScopeData()); - expect(event).toEqual({ + 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: { @@ -140,6 +165,10 @@ describe('Unit | Scope', () => { const eventProcessor2 = jest.fn((b: unknown) => b) as EventProcessor; const eventProcessor3 = jest.fn((c: unknown) => c) as EventProcessor; + const attachment1 = { filename: '1' } as Attachment; + const attachment2 = { filename: '2' } as Attachment; + const attachment3 = { filename: '3' } as Attachment; + const scope = new Scope(); scope.update({ user: { id: '1', email: 'test@example.com' }, @@ -151,6 +180,7 @@ describe('Unit | Scope', () => { }); scope.addBreadcrumb(breadcrumb1); scope.addEventProcessor(eventProcessor1); + scope.addAttachment(attachment1); const globalScope = getGlobalScope(); const isolationScope = getIsolationScope(); @@ -158,16 +188,39 @@ describe('Unit | Scope', () => { globalScope.addBreadcrumb(breadcrumb2); globalScope.addEventProcessor(eventProcessor2); globalScope.setSDKProcessingMetadata({ aa: 'aa' }); + globalScope.addAttachment(attachment2); isolationScope.addBreadcrumb(breadcrumb3); isolationScope.addEventProcessor(eventProcessor3); - globalScope.setSDKProcessingMetadata({ bb: 'bb' }); + isolationScope.setSDKProcessingMetadata({ bb: 'bb' }); + isolationScope.addAttachment(attachment3); const event = { message: 'foo', breadcrumbs: [breadcrumb4], fingerprint: ['dd'] }; - applyScopeDataToEvent(event, scope.getScopeData()); + const options = {} as ClientOptions; + const processedEvent = await prepareEvent( + options, + event, + { + integrations: [], + }, + scope, + ); + + expect(eventProcessor1).toHaveBeenCalledTimes(1); + expect(eventProcessor2).toHaveBeenCalledTimes(1); + expect(eventProcessor3).toHaveBeenCalledTimes(1); + + // Test that attachments are correctly merged + expect(eventProcessor1).toHaveBeenCalledWith(processedEvent, { + integrations: [], + attachments: [attachment2, attachment3, attachment1], + }); - expect(event).toEqual({ + expect(processedEvent).toEqual({ + timestamp: expect.any(Number), + event_id: expect.any(String), + environment: 'production', message: 'foo', user: { id: '1', email: 'test@example.com' }, tags: { tag1: 'aa', tag2: 'aa' }, @@ -186,35 +239,4 @@ describe('Unit | Scope', () => { }); }); }); - - describe('getAttachments', () => { - it('works without any data', async () => { - mockSdkInit(); - - const scope = new Scope(); - - const actual = scope.getAttachments(); - expect(actual).toEqual([]); - }); - - it('merges attachments data', async () => { - mockSdkInit(); - - const attachment1 = { filename: '1' } as Attachment; - const attachment2 = { filename: '2' } as Attachment; - const attachment3 = { filename: '3' } as Attachment; - - const scope = new Scope(); - scope.addAttachment(attachment1); - - const globalScope = getGlobalScope(); - const isolationScope = getIsolationScope(); - - globalScope.addAttachment(attachment2); - isolationScope.addAttachment(attachment3); - - const actual = scope.getAttachments(); - expect(actual).toEqual([attachment2, attachment3, attachment1]); - }); - }); }); diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts index 8cee38ca50ab..50ef4da5987f 100644 --- a/packages/types/src/scope.ts +++ b/packages/types/src/scope.ts @@ -53,9 +53,6 @@ export interface Scope { /** Get the data of this scope, which is applied to an event during processing. */ getScopeData(): ScopeData; - /** Get the data of this scope only, ignoring all other related scopes. */ - getPerScopeData(): ScopeData; - /** * Updates user context information for future events. * From 94ae54c337c509bd5b8819e155dbb1b2a0100d1b Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 21 Dec 2023 09:47:04 +0100 Subject: [PATCH 3/4] ref: avoid globals.ts for scope --- packages/core/src/globals.ts | 22 ----------------- packages/core/src/index.ts | 3 +-- packages/core/src/scope.ts | 24 ++++++++++++++----- packages/core/test/lib/base.test.ts | 4 ++-- packages/core/test/lib/prepareEvent.test.ts | 4 ++-- packages/core/test/lib/scope.test.ts | 11 ++++----- .../src/otel/asyncContextStrategy.ts | 2 +- packages/node-experimental/src/sdk/scope.ts | 20 +++++++--------- .../test/integration/scope.test.ts | 4 ++-- 9 files changed, 40 insertions(+), 54 deletions(-) delete mode 100644 packages/core/src/globals.ts diff --git a/packages/core/src/globals.ts b/packages/core/src/globals.ts deleted file mode 100644 index e889b4fa0709..000000000000 --- a/packages/core/src/globals.ts +++ /dev/null @@ -1,22 +0,0 @@ -import type { Scope } from '@sentry/types'; - -interface GlobalData { - globalScope?: Scope; -} - -const GLOBAL_DATA: GlobalData = {}; - -/** - * Get the global data. - */ -export function getGlobalData(): GlobalData { - return GLOBAL_DATA; -} - -/** - * Reset all global data. - * Mostly useful for tests. - */ -export function clearGlobalData(): void { - delete GLOBAL_DATA.globalScope; -} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 400aac801ebb..dbf18135ef67 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -42,8 +42,7 @@ export { } from './hub'; export { makeSession, closeSession, updateSession } from './session'; export { SessionFlusher } from './sessionflusher'; -export { Scope, getGlobalScope } from './scope'; -export { clearGlobalData, getGlobalData } from './globals'; +export { Scope, getGlobalScope, setGlobalScope } from './scope'; export { notifyEventProcessors, // eslint-disable-next-line deprecation/deprecation diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 9931f8d94279..08b77c4c4108 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -26,15 +26,20 @@ import type { import { dateTimestampInSeconds, isPlainObject, uuid4 } from '@sentry/utils'; import { getGlobalEventProcessors, notifyEventProcessors } from './eventProcessors'; -import { getGlobalData } from './globals'; import { updateSession } from './session'; -import { applyScopeDataToEvent, mergeScopeData } from './utils/applyScopeDataToEvent'; +import { applyScopeDataToEvent } from './utils/applyScopeDataToEvent'; /** * Default value for maximum number of breadcrumbs added to an event. */ const DEFAULT_MAX_BREADCRUMBS = 100; +/** + * The global scope is kept in this module. + * When accessing this via `getGlobalScope()` we'll make sure to set one if none is currently present. + */ +let globalScope: ScopeInterface | undefined; + /** * Holds additional event information. {@link Scope.applyToEvent} will be * called by the client before an event will be sent. @@ -579,12 +584,19 @@ export class Scope implements ScopeInterface { * This scope is applied to _all_ events. */ export function getGlobalScope(): ScopeInterface { - const globalData = getGlobalData(); - if (!globalData.globalScope) { - globalData.globalScope = new Scope(); + if (!globalScope) { + globalScope = new Scope(); } - return globalData.globalScope; + return globalScope; +} + +/** + * This is mainly needed for tests. + * @hidden + */ +export function setGlobalScope(scope: ScopeInterface | undefined): void { + globalScope = scope; } function generatePropagationContext(): PropagationContext { diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 9ba1d60efeab..0b39216e19a5 100644 --- a/packages/core/test/lib/base.test.ts +++ b/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, clearGlobalData, makeSession } from '../../src'; +import { Hub, Scope, makeSession, setGlobalScope } from '../../src'; import * as integrationModule from '../../src/integration'; import { TestClient, getDefaultTestClientOptions } from '../mocks/client'; import { AdHocIntegration, TestIntegration } from '../mocks/integration'; @@ -54,7 +54,7 @@ describe('BaseClient', () => { beforeEach(() => { TestClient.sendEventCalled = undefined; TestClient.instance = undefined; - clearGlobalData(); + setGlobalScope(undefined); }); afterEach(() => { diff --git a/packages/core/test/lib/prepareEvent.test.ts b/packages/core/test/lib/prepareEvent.test.ts index 9e2f69e0f654..09c48afffc35 100644 --- a/packages/core/test/lib/prepareEvent.test.ts +++ b/packages/core/test/lib/prepareEvent.test.ts @@ -9,7 +9,7 @@ import type { ScopeContext, } from '@sentry/types'; import { GLOBAL_OBJ, createStackParser } from '@sentry/utils'; -import { clearGlobalData } from '../../src'; +import { setGlobalScope } from '../../src'; import { Scope, getGlobalScope } from '../../src/scope'; import { @@ -191,7 +191,7 @@ describe('parseEventHintOrCaptureContext', () => { describe('prepareEvent', () => { beforeEach(() => { - clearGlobalData(); + setGlobalScope(undefined); }); it('works without any scope data', async () => { diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts index 3bcdbf4022ce..e04f5638c5d0 100644 --- a/packages/core/test/lib/scope.test.ts +++ b/packages/core/test/lib/scope.test.ts @@ -1,11 +1,10 @@ -import { applyScopeDataToEvent } from '@sentry/core'; -import type { Attachment, Breadcrumb, EventProcessor } from '@sentry/types'; -import { clearGlobalData } from '../../src/globals'; -import { Scope, getGlobalScope } from '../../src/scope'; +import type { Attachment, Breadcrumb } from '@sentry/types'; +import { applyScopeDataToEvent } from '../../src'; +import { Scope, getGlobalScope, setGlobalScope } from '../../src/scope'; describe('Unit | Scope', () => { beforeEach(() => { - clearGlobalData(); + setGlobalScope(undefined); }); it('allows to create & update a scope', () => { @@ -91,7 +90,7 @@ describe('Unit | Scope', () => { describe('global scope', () => { beforeEach(() => { - clearGlobalData(); + setGlobalScope(undefined); }); it('works', () => { diff --git a/packages/node-experimental/src/otel/asyncContextStrategy.ts b/packages/node-experimental/src/otel/asyncContextStrategy.ts index e0d976c71ff1..10b4ea6d7b91 100644 --- a/packages/node-experimental/src/otel/asyncContextStrategy.ts +++ b/packages/node-experimental/src/otel/asyncContextStrategy.ts @@ -1,6 +1,6 @@ import * as api from '@opentelemetry/api'; -import { setAsyncContextStrategy } from './../sdk/globals'; +import { setAsyncContextStrategy } from '../sdk/globals'; import { getCurrentHub } from './../sdk/hub'; import type { CurrentScopes } from './../sdk/types'; import { getScopesFromContext } from './../utils/contextData'; diff --git a/packages/node-experimental/src/sdk/scope.ts b/packages/node-experimental/src/sdk/scope.ts index 72b558c02e1f..f169f95250e2 100644 --- a/packages/node-experimental/src/sdk/scope.ts +++ b/packages/node-experimental/src/sdk/scope.ts @@ -1,4 +1,4 @@ -import { getGlobalData, mergeScopeData } from '@sentry/core'; +import { getGlobalScope as _getGlobalScope, mergeScopeData, setGlobalScope } from '@sentry/core'; import { OpenTelemetryScope } from '@sentry/opentelemetry'; import type { Breadcrumb, Client, Event, EventHint, Severity, SeverityLevel } from '@sentry/types'; import { uuid4 } from '@sentry/utils'; @@ -24,20 +24,17 @@ export function setCurrentScope(scope: Scope): void { * We overwrite this from the core implementation to make sure we get the correct Scope class. */ export function getGlobalScope(): Scope { - const globalData = getGlobalData(); - - if (!globalData.globalScope) { - globalData.globalScope = new Scope(); - } + const globalScope = _getGlobalScope(); // If we have a default Scope here by chance, make sure to "upgrade" it to our custom Scope - if (!(globalData.globalScope instanceof Scope)) { - const oldScope = globalData.globalScope; - globalData.globalScope = new Scope(); - globalData.globalScope.update(oldScope); + if (!(globalScope instanceof Scope)) { + const newScope = new Scope(); + newScope.update(globalScope); + setGlobalScope(newScope); + return newScope; } - return globalData.globalScope as Scope; + return globalScope; } /** Get the currently active isolation scope. */ @@ -97,6 +94,7 @@ export class Scope extends OpenTelemetryScope implements ScopeInterface { newScope._attachments = [...this['_attachments']]; newScope._sdkProcessingMetadata = { ...this['_sdkProcessingMetadata'] }; newScope._propagationContext = { ...this['_propagationContext'] }; + newScope._client = this._client; return newScope; } diff --git a/packages/node-experimental/test/integration/scope.test.ts b/packages/node-experimental/test/integration/scope.test.ts index 9604aa3c1ac3..3efb3d09506d 100644 --- a/packages/node-experimental/test/integration/scope.test.ts +++ b/packages/node-experimental/test/integration/scope.test.ts @@ -1,4 +1,4 @@ -import { clearGlobalData } from '@sentry/core'; +import { setGlobalScope } from '@sentry/core'; import { getCurrentHub, getSpanScope } from '@sentry/opentelemetry'; import * as Sentry from '../../src/'; @@ -231,7 +231,7 @@ describe('Integration | Scope', () => { describe('global scope', () => { beforeEach(() => { - clearGlobalData(); + setGlobalScope(undefined); }); it('works before calling init', () => { From ab09e3722dfb9b1aa018eab3e5dd7c50fd1ab1c8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 21 Dec 2023 10:55:03 +0100 Subject: [PATCH 4/4] explicit experimental --- packages/core/src/scope.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 08b77c4c4108..3fc78081f188 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -593,6 +593,7 @@ export function getGlobalScope(): ScopeInterface { /** * This is mainly needed for tests. + * DO NOT USE this, as this is an internal API and subject to change. * @hidden */ export function setGlobalScope(scope: ScopeInterface | undefined): void {