From f8f6f6ba69498f2d887ee03bfad98ae5b0c7ccb4 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sat, 8 Jul 2023 13:07:27 +0100 Subject: [PATCH 1/7] feat(core): Add `ModuleMetadata` integration --- packages/core/src/envelope.ts | 6 ++++ packages/core/src/integrations/index.ts | 1 + packages/core/src/integrations/metadata.ts | 38 ++++++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 packages/core/src/integrations/metadata.ts diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 5e79d1707d67..81bff5a740fd 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -17,6 +17,8 @@ import { getSdkMetadataForEnvelopeHeader, } from '@sentry/utils'; +import { stripMetadataFromStackFrames } from './metadata'; + /** * Apply SdkInfo (name, version, packages, integrations) to the corresponding event key. * Merge with existing data if any. @@ -83,6 +85,10 @@ export function createEventEnvelope( // of this `delete`, lest we miss putting it back in the next time the property is in use.) delete event.sdkProcessingMetadata; + // Module metadata is only added to stack frames by the `ModuleMetadata` integration` to aid in tagging and routing + // of events. Sentry will ignore these properties on the server so we should strip them to save bandwidth. + stripMetadataFromStackFrames(event); + const eventItem: EventItem = [{ type: eventType }, event]; return createEnvelope(envelopeHeaders, [eventItem]); } diff --git a/packages/core/src/integrations/index.ts b/packages/core/src/integrations/index.ts index ddb199ef4e5f..33a6961e4577 100644 --- a/packages/core/src/integrations/index.ts +++ b/packages/core/src/integrations/index.ts @@ -1,2 +1,3 @@ export { FunctionToString } from './functiontostring'; export { InboundFilters } from './inboundfilters'; +export { ModuleMetadata } from './metadata'; diff --git a/packages/core/src/integrations/metadata.ts b/packages/core/src/integrations/metadata.ts new file mode 100644 index 000000000000..cb14031ad6cf --- /dev/null +++ b/packages/core/src/integrations/metadata.ts @@ -0,0 +1,38 @@ +import type { Event, EventProcessor, Hub, Integration } from '@sentry/types'; + +import { addMetadataToStackFrames } from '../metadata'; + +/** + * Adds module metadata to stack frames. + * + * Metadata can be injected by the Sentry bundler plugins using the `_experiments.moduleMetadata` config option. + */ +export class ModuleMetadata implements Integration { + /* + * @inheritDoc + */ + public static id: string = 'ModuleMetadata'; + + /** + * @inheritDoc + */ + public name: string = ModuleMetadata.id; + + /** + * @inheritDoc + */ + public setupOnce(addGlobalEventProcessor: (processor: EventProcessor) => void, getCurrentHub: () => Hub): void { + const client = getCurrentHub().getClient(); + + if (!client) { + return; + } + + const stackParser = client.getOptions().stackParser; + + addGlobalEventProcessor((event: Event) => { + addMetadataToStackFrames(stackParser, event); + return event; + }); + } +} From d30ca1801348a5fd168fc146c282b675e32a376a Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 9 Jul 2023 14:58:05 +0100 Subject: [PATCH 2/7] Export separatly --- packages/browser/src/index.ts | 1 + packages/core/src/index.ts | 2 +- packages/core/src/integrations/index.ts | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 0a69fc44d858..9dbe7f977d7e 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -34,6 +34,7 @@ export { spanStatusfromHttpCode, trace, makeMultiplexedTransport, + ModuleMetadata, } from '@sentry/core'; export type { SpanStatusType } from '@sentry/core'; export type { Span } from '@sentry/types'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index db9d21c2f2e8..9ff61f05cdb3 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -46,7 +46,7 @@ export { prepareEvent } from './utils/prepareEvent'; export { createCheckInEnvelope } from './checkin'; export { hasTracingEnabled } from './utils/hasTracingEnabled'; export { DEFAULT_ENVIRONMENT } from './constants'; - +export { ModuleMetadata } from './integrations/metadata'; import * as Integrations from './integrations'; export { Integrations }; diff --git a/packages/core/src/integrations/index.ts b/packages/core/src/integrations/index.ts index 33a6961e4577..ddb199ef4e5f 100644 --- a/packages/core/src/integrations/index.ts +++ b/packages/core/src/integrations/index.ts @@ -1,3 +1,2 @@ export { FunctionToString } from './functiontostring'; export { InboundFilters } from './inboundfilters'; -export { ModuleMetadata } from './metadata'; From a104f32e8424f0fea620a43bd864671e35066b26 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 10 Jul 2023 14:51:16 +0100 Subject: [PATCH 3/7] Strip metadata via `beforeEnvelope` and add integration test --- packages/core/src/integrations/metadata.ts | 20 +++++- .../test/lib/integrations/metadata.test.ts | 66 +++++++++++++++++++ packages/core/test/mocks/client.ts | 11 +++- 3 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 packages/core/test/lib/integrations/metadata.test.ts diff --git a/packages/core/src/integrations/metadata.ts b/packages/core/src/integrations/metadata.ts index cb14031ad6cf..ec7c30bdac3a 100644 --- a/packages/core/src/integrations/metadata.ts +++ b/packages/core/src/integrations/metadata.ts @@ -1,6 +1,7 @@ -import type { Event, EventProcessor, Hub, Integration } from '@sentry/types'; +import type { Event, EventItem, EventProcessor, Hub, Integration } from '@sentry/types'; +import { forEachEnvelopeItem } from '@sentry/utils'; -import { addMetadataToStackFrames } from '../metadata'; +import { addMetadataToStackFrames, stripMetadataFromStackFrames } from '../metadata'; /** * Adds module metadata to stack frames. @@ -24,10 +25,23 @@ export class ModuleMetadata implements Integration { public setupOnce(addGlobalEventProcessor: (processor: EventProcessor) => void, getCurrentHub: () => Hub): void { const client = getCurrentHub().getClient(); - if (!client) { + if (!client || typeof client.on !== 'function') { return; } + client.on('beforeEnvelope', envelope => { + forEachEnvelopeItem(envelope, (item, type) => { + if (type === 'event' || type === 'transaction') { + const event = Array.isArray(item) ? (item as EventItem)[1] : undefined; + + if (event) { + stripMetadataFromStackFrames(event); + (item as EventItem)[1] = event; + } + } + }); + }); + const stackParser = client.getOptions().stackParser; addGlobalEventProcessor((event: Event) => { diff --git a/packages/core/test/lib/integrations/metadata.test.ts b/packages/core/test/lib/integrations/metadata.test.ts new file mode 100644 index 000000000000..de723fc4bd53 --- /dev/null +++ b/packages/core/test/lib/integrations/metadata.test.ts @@ -0,0 +1,66 @@ +import { Event, StackFrame } from '@sentry/types'; +import { createStackParser, GLOBAL_OBJ, nodeStackLineParser, parseEnvelope } from '@sentry/utils'; +import { TextDecoder, TextEncoder } from 'util'; + +import { createTransport, getCurrentHub, ModuleMetadata } from '../../../src'; +import { getDefaultTestClientOptions, TestClient } from '../../mocks/client'; + +const stackParser = createStackParser(nodeStackLineParser()); + +const stack = new Error().stack || ''; + +describe('ModuleMetadata integration', () => { + beforeEach(() => { + TestClient.sendEventCalled = undefined; + TestClient.instance = undefined; + + GLOBAL_OBJ._sentryModuleMetadata = GLOBAL_OBJ._sentryModuleMetadata || {}; + GLOBAL_OBJ._sentryModuleMetadata[stack] = { team: 'frontend' }; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('Adds and removes metadata', done => { + const options = getDefaultTestClientOptions({ + dsn: 'https://username@domain/123', + enableSend: true, + stackParser, + integrations: [new ModuleMetadata()], + beforeSend: (event, _hint) => { + // copy the frames since reverse in in-place + const lastFrame = [...(event.exception?.values?.[0].stacktrace?.frames || [])].reverse()[0]; + // Ensure module_metadata is populated in beforeSend callback + expect(lastFrame?.module_metadata).toEqual({ team: 'frontend' }); + return event; + }, + transport: () => + createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, async req => { + const [, items] = parseEnvelope(req.body, new TextEncoder(), new TextDecoder()); + + expect(items[0][1]).toBeDefined(); + const event = items[0][1] as Event; + const error = event.exception?.values?.[0]; + + // Ensure we're looking at the same error we threw + expect(error?.value).toEqual('Some error'); + + const lastFrame = [...(error?.stacktrace?.frames || [])].reverse()[0]; + // Ensure the last frame is in fact for this file + expect(lastFrame?.filename).toEqual(__filename); + + // Ensure module_metadata has been stripped from the event + expect(lastFrame?.module_metadata).toBeUndefined(); + + done(); + return {}; + }), + }); + + const client = new TestClient(options); + const hub = getCurrentHub(); + hub.bindClient(client); + hub.captureException(new Error('Some error')); + }); +}); diff --git a/packages/core/test/mocks/client.ts b/packages/core/test/mocks/client.ts index 7ca980189e19..f7d8830a1402 100644 --- a/packages/core/test/mocks/client.ts +++ b/packages/core/test/mocks/client.ts @@ -54,7 +54,7 @@ export class TestClient extends BaseClient { // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types public eventFromException(exception: any): PromiseLike { - return resolvedSyncPromise({ + const event: Event = { exception: { values: [ { @@ -65,7 +65,14 @@ export class TestClient extends BaseClient { }, ], }, - }); + }; + + const frames = this._options.stackParser(exception.stack || '', 1); + if (frames.length && event?.exception?.values?.[0]) { + event.exception.values[0] = { ...event.exception.values[0], stacktrace: { frames } }; + } + + return resolvedSyncPromise(event); } public eventFromMessage( From d76a059cd92943360c91e504fb9940abc6e1ef8a Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 10 Jul 2023 14:55:06 +0100 Subject: [PATCH 4/7] Fix lint --- packages/core/test/lib/integrations/metadata.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/lib/integrations/metadata.test.ts b/packages/core/test/lib/integrations/metadata.test.ts index de723fc4bd53..3529548e2cff 100644 --- a/packages/core/test/lib/integrations/metadata.test.ts +++ b/packages/core/test/lib/integrations/metadata.test.ts @@ -1,4 +1,4 @@ -import { Event, StackFrame } from '@sentry/types'; +import type { Event } from '@sentry/types'; import { createStackParser, GLOBAL_OBJ, nodeStackLineParser, parseEnvelope } from '@sentry/utils'; import { TextDecoder, TextEncoder } from 'util'; From 7e407c10ef46a22210e3b7b57eb20a249368f637 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 10 Jul 2023 14:59:03 +0100 Subject: [PATCH 5/7] Better comments --- packages/core/src/integrations/metadata.ts | 11 ++++++++--- packages/core/test/lib/integrations/metadata.test.ts | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/core/src/integrations/metadata.ts b/packages/core/src/integrations/metadata.ts index ec7c30bdac3a..78a778d91c8a 100644 --- a/packages/core/src/integrations/metadata.ts +++ b/packages/core/src/integrations/metadata.ts @@ -1,4 +1,4 @@ -import type { Event, EventItem, EventProcessor, Hub, Integration } from '@sentry/types'; +import type { EventItem, EventProcessor, Hub, Integration } from '@sentry/types'; import { forEachEnvelopeItem } from '@sentry/utils'; import { addMetadataToStackFrames, stripMetadataFromStackFrames } from '../metadata'; @@ -7,6 +7,10 @@ import { addMetadataToStackFrames, stripMetadataFromStackFrames } from '../metad * Adds module metadata to stack frames. * * Metadata can be injected by the Sentry bundler plugins using the `_experiments.moduleMetadata` config option. + * + * When this integration is added, the metadata passed to the bundler plugin is added to the stack frames of all events + * under the `module_metadata` property. This can be used to help in tagging or routing of events from different teams + * our sources */ export class ModuleMetadata implements Integration { /* @@ -29,6 +33,7 @@ export class ModuleMetadata implements Integration { return; } + // We need to strip metadata from stack frames before sending them to Sentry since these are client side only. client.on('beforeEnvelope', envelope => { forEachEnvelopeItem(envelope, (item, type) => { if (type === 'event' || type === 'transaction') { @@ -36,7 +41,7 @@ export class ModuleMetadata implements Integration { if (event) { stripMetadataFromStackFrames(event); - (item as EventItem)[1] = event; + item[1] = event; } } }); @@ -44,7 +49,7 @@ export class ModuleMetadata implements Integration { const stackParser = client.getOptions().stackParser; - addGlobalEventProcessor((event: Event) => { + addGlobalEventProcessor(event => { addMetadataToStackFrames(stackParser, event); return event; }); diff --git a/packages/core/test/lib/integrations/metadata.test.ts b/packages/core/test/lib/integrations/metadata.test.ts index 3529548e2cff..464da2d97fbb 100644 --- a/packages/core/test/lib/integrations/metadata.test.ts +++ b/packages/core/test/lib/integrations/metadata.test.ts @@ -22,7 +22,7 @@ describe('ModuleMetadata integration', () => { jest.clearAllMocks(); }); - test('Adds and removes metadata', done => { + test('Adds and removes metadata from stack frames', done => { const options = getDefaultTestClientOptions({ dsn: 'https://username@domain/123', enableSend: true, From 017904fbf294d16541a628c73a71a94889939c14 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 10 Jul 2023 14:59:53 +0100 Subject: [PATCH 6/7] Don't strip in core code --- packages/core/src/envelope.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 81bff5a740fd..5e79d1707d67 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -17,8 +17,6 @@ import { getSdkMetadataForEnvelopeHeader, } from '@sentry/utils'; -import { stripMetadataFromStackFrames } from './metadata'; - /** * Apply SdkInfo (name, version, packages, integrations) to the corresponding event key. * Merge with existing data if any. @@ -85,10 +83,6 @@ export function createEventEnvelope( // of this `delete`, lest we miss putting it back in the next time the property is in use.) delete event.sdkProcessingMetadata; - // Module metadata is only added to stack frames by the `ModuleMetadata` integration` to aid in tagging and routing - // of events. Sentry will ignore these properties on the server so we should strip them to save bandwidth. - stripMetadataFromStackFrames(event); - const eventItem: EventItem = [{ type: eventType }, event]; return createEnvelope(envelopeHeaders, [eventItem]); } From 0cf7781a4c1ad22446e6cb0b24cf621a1dce4934 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 13 Jul 2023 18:15:34 +0100 Subject: [PATCH 7/7] Don't strip from transactions --- packages/core/src/integrations/metadata.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/integrations/metadata.ts b/packages/core/src/integrations/metadata.ts index 78a778d91c8a..05af1d88ebe9 100644 --- a/packages/core/src/integrations/metadata.ts +++ b/packages/core/src/integrations/metadata.ts @@ -36,7 +36,7 @@ export class ModuleMetadata implements Integration { // We need to strip metadata from stack frames before sending them to Sentry since these are client side only. client.on('beforeEnvelope', envelope => { forEachEnvelopeItem(envelope, (item, type) => { - if (type === 'event' || type === 'transaction') { + if (type === 'event') { const event = Array.isArray(item) ? (item as EventItem)[1] : undefined; if (event) {