From ed248df9efaa987aded6b32fd1498cf65169fc97 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Tue, 23 May 2023 16:43:12 +0200 Subject: [PATCH 1/7] feat: Add default ignoreTransactions --- .../core/src/integrations/inboundfilters.ts | 18 ++++++++++++++++-- .../lib/integrations/inboundfilters.test.ts | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 998c7ab8203d..176fe879dfcf 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -5,6 +5,16 @@ import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/u // this is the result of a script being pulled in from an external domain and CORS. const DEFAULT_IGNORE_ERRORS = [/^Script error\.?$/, /^Javascript error: Script error\.? on line 0$/]; +const DEFAULT_IGNORE_TRANSACTIONS = [ + /^.*healthcheck.*$/, + /^.*healthy.*$/, + /^.*live.*$/, + /^.*ready.*$/, + /^.*heartbeat.*$/, + /^.*\/health$/, + /^.*\/healthz$/ +]; + /** Options for the InboundFilters integration */ export interface InboundFiltersOptions { allowUrls: Array; @@ -26,7 +36,7 @@ export class InboundFilters implements Integration { */ public name: string = InboundFilters.id; - public constructor(private readonly _options: Partial = {}) {} + public constructor(private readonly _options: Partial = {}) { } /** * @inheritDoc @@ -64,7 +74,11 @@ export function _mergeOptions( ...(clientOptions.ignoreErrors || []), ...DEFAULT_IGNORE_ERRORS, ], - ignoreTransactions: [...(internalOptions.ignoreTransactions || []), ...(clientOptions.ignoreTransactions || [])], + ignoreTransactions: [ + ...(internalOptions.ignoreTransactions || []), + ...(clientOptions.ignoreTransactions || []), + ...DEFAULT_IGNORE_TRANSACTIONS, + ], ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true, }; } diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index a528d765bfeb..e12894bfd3d7 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -208,6 +208,21 @@ const TRANSACTION_EVENT_3: Event = { type: 'transaction', }; +const TRANSACTION_EVENT_HEALTH: Event = { + transaction: 'GET /health', + type: 'transaction', +}; + +const TRANSACTION_EVENT_HEALTH_2: Event = { + transaction: 'GET /healthy', + type: 'transaction', +}; + +const TRANSACTION_EVENT_HEALTH_3: Event = { + transaction: 'GET /live', + type: 'transaction', +}; + describe('InboundFilters', () => { describe('_isSentryError', () => { it('should work as expected', () => { @@ -373,6 +388,9 @@ describe('InboundFilters', () => { it('uses default filters', () => { const eventProcessor = createInboundFiltersEventProcessor(); expect(eventProcessor(TRANSACTION_EVENT, {})).toBe(TRANSACTION_EVENT); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(null); }); }); From 5628289862b56a7b288e9eb3be3f21e02fb534af Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 25 May 2023 15:32:03 +0200 Subject: [PATCH 2/7] ref: Add disableDefaults option to inbound integration --- packages/core/src/integrations/inboundfilters.ts | 9 +++++++-- .../core/test/lib/integrations/inboundfilters.test.ts | 8 ++++++++ .../test/integration/test/client/tracingPageLoad.test.ts | 4 ++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 176fe879dfcf..55185399457d 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -22,6 +22,7 @@ export interface InboundFiltersOptions { ignoreErrors: Array; ignoreTransactions: Array; ignoreInternal: boolean; + disableDefaults: boolean; } /** Inbound filters configurable by the user */ @@ -66,18 +67,22 @@ export function _mergeOptions( internalOptions: Partial = {}, clientOptions: Partial = {}, ): Partial { + + if (internalOptions.disableDefaults === true) { + + } return { allowUrls: [...(internalOptions.allowUrls || []), ...(clientOptions.allowUrls || [])], denyUrls: [...(internalOptions.denyUrls || []), ...(clientOptions.denyUrls || [])], ignoreErrors: [ ...(internalOptions.ignoreErrors || []), ...(clientOptions.ignoreErrors || []), - ...DEFAULT_IGNORE_ERRORS, + ...(internalOptions.disableDefaults === true ? [] : DEFAULT_IGNORE_ERRORS), ], ignoreTransactions: [ ...(internalOptions.ignoreTransactions || []), ...(clientOptions.ignoreTransactions || []), - ...DEFAULT_IGNORE_TRANSACTIONS, + ...(internalOptions.disableDefaults === true ? [] : DEFAULT_IGNORE_TRANSACTIONS), ], ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true, }; diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index e12894bfd3d7..17bc51840a19 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -392,6 +392,14 @@ describe('InboundFilters', () => { expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(null); expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(null); }); + + it('disable default filters', () => { + const eventProcessor = createInboundFiltersEventProcessor({ disableDefaults: true }); + expect(eventProcessor(TRANSACTION_EVENT, {})).toBe(TRANSACTION_EVENT); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(TRANSACTION_EVENT_HEALTH); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(TRANSACTION_EVENT_HEALTH_2); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(TRANSACTION_EVENT_HEALTH_3); + }); }); describe('denyUrls/allowUrls', () => { diff --git a/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts b/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts index 028c8b2d7998..dc0f76d67644 100644 --- a/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts +++ b/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts @@ -4,7 +4,7 @@ import { Transaction } from '@sentry/types'; test('should report a `pageload` transaction', async ({ page }) => { const transaction = await getMultipleSentryEnvelopeRequests(page, 1, { - url: '/healthy', + url: '/testy', envelopeType: 'transaction', }); @@ -16,5 +16,5 @@ test('should report a `pageload` transaction', async ({ page }) => { }, }); - expect(await countEnvelopes(page, { url: '/healthy', envelopeType: 'transaction', timeout: 4000 })).toBe(1); + expect(await countEnvelopes(page, { url: '/testy', envelopeType: 'transaction', timeout: 4000 })).toBe(1); }); From 47f88494c6516aa7c472d1b255fe7dfb36d58caf Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 25 May 2023 15:33:16 +0200 Subject: [PATCH 3/7] ref: Remove if --- packages/core/src/integrations/inboundfilters.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 55185399457d..63a454149969 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -67,10 +67,6 @@ export function _mergeOptions( internalOptions: Partial = {}, clientOptions: Partial = {}, ): Partial { - - if (internalOptions.disableDefaults === true) { - - } return { allowUrls: [...(internalOptions.allowUrls || []), ...(clientOptions.allowUrls || [])], denyUrls: [...(internalOptions.denyUrls || []), ...(clientOptions.denyUrls || [])], From 898e51440b82fd738c1ffe63917301361f30eafb Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 25 May 2023 16:01:39 +0200 Subject: [PATCH 4/7] fix: Linter --- packages/core/src/integrations/inboundfilters.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 63a454149969..2bfece3ac98b 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -12,7 +12,7 @@ const DEFAULT_IGNORE_TRANSACTIONS = [ /^.*ready.*$/, /^.*heartbeat.*$/, /^.*\/health$/, - /^.*\/healthz$/ + /^.*\/healthz$/, ]; /** Options for the InboundFilters integration */ @@ -37,7 +37,7 @@ export class InboundFilters implements Integration { */ public name: string = InboundFilters.id; - public constructor(private readonly _options: Partial = {}) { } + public constructor(private readonly _options: Partial = {}) {} /** * @inheritDoc From c063be3ad4a81ab64c4afce45c524b3ec3945a74 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 25 May 2023 17:24:53 +0200 Subject: [PATCH 5/7] Update packages/core/src/integrations/inboundfilters.ts Co-authored-by: Lukas Stracke --- packages/core/src/integrations/inboundfilters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 2bfece3ac98b..2139a8abb234 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -73,7 +73,7 @@ export function _mergeOptions( ignoreErrors: [ ...(internalOptions.ignoreErrors || []), ...(clientOptions.ignoreErrors || []), - ...(internalOptions.disableDefaults === true ? [] : DEFAULT_IGNORE_ERRORS), + ...(internalOptions.disableDefaults ? [] : DEFAULT_IGNORE_ERRORS), ], ignoreTransactions: [ ...(internalOptions.ignoreTransactions || []), From 7040d861daf1daf931dc386d442ee97a5408bdaf Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 25 May 2023 17:31:17 +0200 Subject: [PATCH 6/7] ref: Split vars --- packages/core/src/integrations/inboundfilters.ts | 7 ++++--- .../test/lib/integrations/inboundfilters.test.ts | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 2139a8abb234..fb70eba57c57 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -22,7 +22,8 @@ export interface InboundFiltersOptions { ignoreErrors: Array; ignoreTransactions: Array; ignoreInternal: boolean; - disableDefaults: boolean; + disableErrorDefaults: boolean; + disableTransactionDefaults: boolean; } /** Inbound filters configurable by the user */ @@ -73,12 +74,12 @@ export function _mergeOptions( ignoreErrors: [ ...(internalOptions.ignoreErrors || []), ...(clientOptions.ignoreErrors || []), - ...(internalOptions.disableDefaults ? [] : DEFAULT_IGNORE_ERRORS), + ...(internalOptions.disableErrorDefaults ? [] : DEFAULT_IGNORE_ERRORS), ], ignoreTransactions: [ ...(internalOptions.ignoreTransactions || []), ...(clientOptions.ignoreTransactions || []), - ...(internalOptions.disableDefaults === true ? [] : DEFAULT_IGNORE_TRANSACTIONS), + ...(internalOptions.disableTransactionDefaults === true ? [] : DEFAULT_IGNORE_TRANSACTIONS), ], ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true, }; diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index 17bc51840a19..88ab23f65909 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -387,14 +387,24 @@ describe('InboundFilters', () => { it('uses default filters', () => { const eventProcessor = createInboundFiltersEventProcessor(); + expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(null); expect(eventProcessor(TRANSACTION_EVENT, {})).toBe(TRANSACTION_EVENT); expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(null); expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(null); expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(null); }); - it('disable default filters', () => { - const eventProcessor = createInboundFiltersEventProcessor({ disableDefaults: true }); + it('disable default error filters', () => { + const eventProcessor = createInboundFiltersEventProcessor({ disableErrorDefaults: true }); + expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(SCRIPT_ERROR_EVENT); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(null); + }); + + it('disable default transaction filters', () => { + const eventProcessor = createInboundFiltersEventProcessor({ disableTransactionDefaults: true }); + expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(null); expect(eventProcessor(TRANSACTION_EVENT, {})).toBe(TRANSACTION_EVENT); expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(TRANSACTION_EVENT_HEALTH); expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(TRANSACTION_EVENT_HEALTH_2); From 979f3ceb545547911d9b24bfd32d8dac07121a03 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Wed, 31 May 2023 10:10:03 +0200 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Lukas Stracke --- packages/core/src/integrations/inboundfilters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index fb70eba57c57..3b6b2e8933de 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -79,7 +79,7 @@ export function _mergeOptions( ignoreTransactions: [ ...(internalOptions.ignoreTransactions || []), ...(clientOptions.ignoreTransactions || []), - ...(internalOptions.disableTransactionDefaults === true ? [] : DEFAULT_IGNORE_TRANSACTIONS), + ...(internalOptions.disableTransactionDefaults ? [] : DEFAULT_IGNORE_TRANSACTIONS), ], ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true, };