From 21b3dc2177d8cc63e6f211a7d118928b2826fa78 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 11 Jun 2025 17:53:14 +0000 Subject: [PATCH 1/2] Add `ignoreLayers` and `ignoreLayersType` to Express integration --- PR_DESCRIPTION.md | 75 +++++++ .../node/src/integrations/tracing/express.ts | 57 ++++- .../test/integrations/tracing/express.test.ts | 197 ++++++++++++++++++ 3 files changed, 324 insertions(+), 5 deletions(-) create mode 100644 PR_DESCRIPTION.md create mode 100644 packages/node/test/integrations/tracing/express.test.ts diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 000000000000..5b8547539146 --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,75 @@ +# Add `ignoreLayers` and `ignoreLayersType` options to Express integration + +This PR adds two new options to the Express integration: `ignoreLayers` and `ignoreLayersType`, bringing it to feature parity with the underlying OpenTelemetry instrumentation. + +## Changes + +- Added `ignoreLayers` option to ignore specific Express layers based on their path +- Added `ignoreLayersType` option to ignore specific Express layers based on their type +- Added comprehensive tests for both options +- Updated JSDoc documentation with usage examples + +## Usage + +### Ignore layers by path + +The `ignoreLayers` option accepts an array of elements that can be: +- `string` for full match of the path +- `RegExp` for partial match of the path +- `function` in the form of `(path) => boolean` for custom logic + +```javascript +const Sentry = require('@sentry/node'); + +Sentry.init({ + integrations: [ + Sentry.expressIntegration({ + ignoreLayers: [ + '/health', // Ignore exact path + /^\/internal/, // Ignore paths starting with /internal + (path) => path.includes('admin') // Custom logic + ] + }) + ], +}); +``` + +### Ignore layers by type + +The `ignoreLayersType` option accepts an array of the following strings: +- `router` - for `express.Router()` +- `middleware` - for middleware functions +- `request_handler` - for request handlers (anything that's not a router or middleware) + +```javascript +const Sentry = require('@sentry/node'); + +Sentry.init({ + integrations: [ + Sentry.expressIntegration({ + ignoreLayersType: ['middleware'] // Ignore all middleware spans + }) + ], +}); +``` + +### Combining both options + +Both options can be used together: + +```javascript +const Sentry = require('@sentry/node'); + +Sentry.init({ + integrations: [ + Sentry.expressIntegration({ + ignoreLayers: ['/health', '/metrics', /^\/internal/], + ignoreLayersType: ['middleware'] + }) + ], +}); +``` + +## Note + +While this allows ignoring specific Express layers, to ignore entire Express routes from creating traces, you should use the `ignoreIncomingRequestHook` option from the HTTP instrumentation as documented in the [Express documentation](https://docs.sentry.io/platforms/javascript/guides/express/). diff --git a/packages/node/src/integrations/tracing/express.ts b/packages/node/src/integrations/tracing/express.ts index e13ce6212b79..457fc58e0df3 100644 --- a/packages/node/src/integrations/tracing/express.ts +++ b/packages/node/src/integrations/tracing/express.ts @@ -22,6 +22,19 @@ import { ExpressInstrumentationV5 } from './express-v5/instrumentation'; const INTEGRATION_NAME = 'Express'; const INTEGRATION_NAME_V5 = 'Express-V5'; +type IgnoreMatcher = string | RegExp | ((path: string) => boolean); + +interface ExpressOptions { + /** + * Ignore specific layers based on their path + */ + ignoreLayers?: IgnoreMatcher[]; + /** + * Ignore specific layers based on their type + */ + ignoreLayersType?: ('router' | 'middleware' | 'request_handler')[]; +} + function requestHook(span: Span): void { addOriginToSpan(span, 'auto.http.otel.express'); @@ -56,28 +69,32 @@ function spanNameHook(info: ExpressRequestInfo, defaultName: string): s export const instrumentExpress = generateInstrumentOnce( INTEGRATION_NAME, - () => + (options: ExpressOptions = {}) => new ExpressInstrumentation({ requestHook: span => requestHook(span), spanNameHook: (info, defaultName) => spanNameHook(info, defaultName), + ignoreLayers: options.ignoreLayers, + ignoreLayersType: options.ignoreLayersType as any, }), ); export const instrumentExpressV5 = generateInstrumentOnce( INTEGRATION_NAME_V5, - () => + (options: ExpressOptions = {}) => new ExpressInstrumentationV5({ requestHook: span => requestHook(span), spanNameHook: (info, defaultName) => spanNameHook(info, defaultName), + ignoreLayers: options.ignoreLayers, + ignoreLayersType: options.ignoreLayersType as any, }), ); -const _expressIntegration = (() => { +const _expressIntegration = ((options: ExpressOptions = {}) => { return { name: INTEGRATION_NAME, setupOnce() { - instrumentExpress(); - instrumentExpressV5(); + instrumentExpress(options); + instrumentExpressV5(options); }, }; }) satisfies IntegrationFn; @@ -89,6 +106,8 @@ const _expressIntegration = (() => { * * For more information, see the [express documentation](https://docs.sentry.io/platforms/javascript/guides/express/). * + * @param {ExpressOptions} options Configuration options for the Express integration. + * * @example * ```javascript * const Sentry = require('@sentry/node'); @@ -97,6 +116,34 @@ const _expressIntegration = (() => { * integrations: [Sentry.expressIntegration()], * }) * ``` + * + * @example + * ```javascript + * // To ignore specific middleware layers by path + * const Sentry = require('@sentry/node'); + * + * Sentry.init({ + * integrations: [ + * Sentry.expressIntegration({ + * ignoreLayers: ['/health', /^\/internal/] + * }) + * ], + * }) + * ``` + * + * @example + * ```javascript + * // To ignore specific middleware layers by type + * const Sentry = require('@sentry/node'); + * + * Sentry.init({ + * integrations: [ + * Sentry.expressIntegration({ + * ignoreLayersType: ['middleware'] + * }) + * ], + * }) + * ``` */ export const expressIntegration = defineIntegration(_expressIntegration); diff --git a/packages/node/test/integrations/tracing/express.test.ts b/packages/node/test/integrations/tracing/express.test.ts new file mode 100644 index 000000000000..4d63e54b03ed --- /dev/null +++ b/packages/node/test/integrations/tracing/express.test.ts @@ -0,0 +1,197 @@ +import { ExpressInstrumentation } from '@opentelemetry/instrumentation-express'; +import { type MockInstance, beforeEach, describe, expect, it, vi } from 'vitest'; +import { expressIntegration, instrumentExpress, instrumentExpressV5 } from '../../../src/integrations/tracing/express'; +import { ExpressInstrumentationV5 } from '../../../src/integrations/tracing/express-v5/instrumentation'; +import { INSTRUMENTED } from '../../../src/otel/instrument'; + +vi.mock('@opentelemetry/instrumentation-express'); +vi.mock('../../../src/integrations/tracing/express-v5/instrumentation'); + +describe('Express', () => { + beforeEach(() => { + vi.clearAllMocks(); + delete INSTRUMENTED.Express; + delete INSTRUMENTED['Express-V5']; + + (ExpressInstrumentation as unknown as MockInstance).mockImplementation(() => { + return { + setTracerProvider: () => undefined, + setMeterProvider: () => undefined, + getConfig: () => ({}), + setConfig: () => ({}), + enable: () => undefined, + }; + }); + + (ExpressInstrumentationV5 as unknown as MockInstance).mockImplementation(() => { + return { + setTracerProvider: () => undefined, + setMeterProvider: () => undefined, + getConfig: () => ({}), + setConfig: () => ({}), + enable: () => undefined, + }; + }); + }); + + describe('instrumentExpress', () => { + it('defaults are correct for instrumentExpress', () => { + instrumentExpress({}); + + expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentation).toHaveBeenCalledWith({ + ignoreLayers: undefined, + ignoreLayersType: undefined, + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + + it('passes ignoreLayers option to instrumentation', () => { + instrumentExpress({ ignoreLayers: ['/health', /^\/internal/] }); + + expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentation).toHaveBeenCalledWith({ + ignoreLayers: ['/health', /^\/internal/], + ignoreLayersType: undefined, + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + + it('passes ignoreLayersType option to instrumentation', () => { + instrumentExpress({ ignoreLayersType: ['middleware'] }); + + expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentation).toHaveBeenCalledWith({ + ignoreLayers: undefined, + ignoreLayersType: ['middleware'], + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + + it('passes multiple ignoreLayersType values to instrumentation', () => { + instrumentExpress({ ignoreLayersType: ['middleware', 'router'] }); + + expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentation).toHaveBeenCalledWith({ + ignoreLayers: undefined, + ignoreLayersType: ['middleware', 'router'], + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + + it('passes both options to instrumentation', () => { + instrumentExpress({ + ignoreLayers: ['/health'], + ignoreLayersType: ['request_handler'] + }); + + expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentation).toHaveBeenCalledWith({ + ignoreLayers: ['/health'], + ignoreLayersType: ['request_handler'], + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + }); + + describe('instrumentExpressV5', () => { + it('defaults are correct for instrumentExpressV5', () => { + instrumentExpressV5({}); + + expect(ExpressInstrumentationV5).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentationV5).toHaveBeenCalledWith({ + ignoreLayers: undefined, + ignoreLayersType: undefined, + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + + it('passes options to instrumentExpressV5', () => { + instrumentExpressV5({ + ignoreLayers: [(path: string) => path.startsWith('/admin')], + ignoreLayersType: ['middleware', 'router'] + }); + + expect(ExpressInstrumentationV5).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentationV5).toHaveBeenCalledWith({ + ignoreLayers: [expect.any(Function)], + ignoreLayersType: ['middleware', 'router'], + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + }); + + describe('expressIntegration', () => { + it('defaults are correct for expressIntegration', () => { + expressIntegration().setupOnce!(); + + expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentation).toHaveBeenCalledWith({ + ignoreLayers: undefined, + ignoreLayersType: undefined, + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + + expect(ExpressInstrumentationV5).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentationV5).toHaveBeenCalledWith({ + ignoreLayers: undefined, + ignoreLayersType: undefined, + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + + it('passes options from expressIntegration to both instrumentations', () => { + expressIntegration({ + ignoreLayers: [/^\/api\/v1/], + ignoreLayersType: ['middleware'] + }).setupOnce!(); + + expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentation).toHaveBeenCalledWith({ + ignoreLayers: [/^\/api\/v1/], + ignoreLayersType: ['middleware'], + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + + expect(ExpressInstrumentationV5).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentationV5).toHaveBeenCalledWith({ + ignoreLayers: [/^\/api\/v1/], + ignoreLayersType: ['middleware'], + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + + it('passes all layer types from expressIntegration to instrumentation', () => { + expressIntegration({ + ignoreLayersType: ['router', 'middleware', 'request_handler'] + }).setupOnce!(); + + expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentation).toHaveBeenCalledWith({ + ignoreLayers: undefined, + ignoreLayersType: ['router', 'middleware', 'request_handler'], + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + + expect(ExpressInstrumentationV5).toHaveBeenCalledTimes(1); + expect(ExpressInstrumentationV5).toHaveBeenCalledWith({ + ignoreLayers: undefined, + ignoreLayersType: ['router', 'middleware', 'request_handler'], + requestHook: expect.any(Function), + spanNameHook: expect.any(Function), + }); + }); + }); +}); From 679ff95ad8d9996d5c4f26367d069eb1a2f08dbd Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 11 Jun 2025 14:03:47 -0400 Subject: [PATCH 2/2] cursor cleanup --- PR_DESCRIPTION.md | 75 ------------------- .../node/src/integrations/tracing/express.ts | 32 ++++++-- .../test/integrations/tracing/express.test.ts | 8 +- 3 files changed, 31 insertions(+), 84 deletions(-) delete mode 100644 PR_DESCRIPTION.md diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md deleted file mode 100644 index 5b8547539146..000000000000 --- a/PR_DESCRIPTION.md +++ /dev/null @@ -1,75 +0,0 @@ -# Add `ignoreLayers` and `ignoreLayersType` options to Express integration - -This PR adds two new options to the Express integration: `ignoreLayers` and `ignoreLayersType`, bringing it to feature parity with the underlying OpenTelemetry instrumentation. - -## Changes - -- Added `ignoreLayers` option to ignore specific Express layers based on their path -- Added `ignoreLayersType` option to ignore specific Express layers based on their type -- Added comprehensive tests for both options -- Updated JSDoc documentation with usage examples - -## Usage - -### Ignore layers by path - -The `ignoreLayers` option accepts an array of elements that can be: -- `string` for full match of the path -- `RegExp` for partial match of the path -- `function` in the form of `(path) => boolean` for custom logic - -```javascript -const Sentry = require('@sentry/node'); - -Sentry.init({ - integrations: [ - Sentry.expressIntegration({ - ignoreLayers: [ - '/health', // Ignore exact path - /^\/internal/, // Ignore paths starting with /internal - (path) => path.includes('admin') // Custom logic - ] - }) - ], -}); -``` - -### Ignore layers by type - -The `ignoreLayersType` option accepts an array of the following strings: -- `router` - for `express.Router()` -- `middleware` - for middleware functions -- `request_handler` - for request handlers (anything that's not a router or middleware) - -```javascript -const Sentry = require('@sentry/node'); - -Sentry.init({ - integrations: [ - Sentry.expressIntegration({ - ignoreLayersType: ['middleware'] // Ignore all middleware spans - }) - ], -}); -``` - -### Combining both options - -Both options can be used together: - -```javascript -const Sentry = require('@sentry/node'); - -Sentry.init({ - integrations: [ - Sentry.expressIntegration({ - ignoreLayers: ['/health', '/metrics', /^\/internal/], - ignoreLayersType: ['middleware'] - }) - ], -}); -``` - -## Note - -While this allows ignoring specific Express layers, to ignore entire Express routes from creating traces, you should use the `ignoreIncomingRequestHook` option from the HTTP instrumentation as documented in the [Express documentation](https://docs.sentry.io/platforms/javascript/guides/express/). diff --git a/packages/node/src/integrations/tracing/express.ts b/packages/node/src/integrations/tracing/express.ts index 457fc58e0df3..f2f03cd53bfc 100644 --- a/packages/node/src/integrations/tracing/express.ts +++ b/packages/node/src/integrations/tracing/express.ts @@ -1,6 +1,6 @@ import type * as http from 'node:http'; import type { Span } from '@opentelemetry/api'; -import type { ExpressRequestInfo } from '@opentelemetry/instrumentation-express'; +import type { ExpressLayerType, ExpressRequestInfo } from '@opentelemetry/instrumentation-express'; import { ExpressInstrumentation } from '@opentelemetry/instrumentation-express'; import type { IntegrationFn } from '@sentry/core'; import { @@ -26,11 +26,33 @@ type IgnoreMatcher = string | RegExp | ((path: string) => boolean); interface ExpressOptions { /** - * Ignore specific layers based on their path + * Ignore specific layers based on their path. + * + * Accepts an array of matchers that can be: + * - String: exact path match + * - RegExp: pattern matching + * - Function: custom logic that receives the path and returns boolean */ ignoreLayers?: IgnoreMatcher[]; /** - * Ignore specific layers based on their type + * Ignore specific layers based on their type. + * + * Available layer types: + * - 'router': Express router layers + * - 'middleware': Express middleware layers + * - 'request_handler': Express request handler layers + * + * @example + * ```javascript + * // Ignore only middleware layers + * ignoreLayersType: ['middleware'] + * + * // Ignore multiple layer types + * ignoreLayersType: ['middleware', 'router'] + * + * // Ignore all layer types (effectively disables tracing) + * ignoreLayersType: ['middleware', 'router', 'request_handler'] + * ``` */ ignoreLayersType?: ('router' | 'middleware' | 'request_handler')[]; } @@ -74,7 +96,7 @@ export const instrumentExpress = generateInstrumentOnce( requestHook: span => requestHook(span), spanNameHook: (info, defaultName) => spanNameHook(info, defaultName), ignoreLayers: options.ignoreLayers, - ignoreLayersType: options.ignoreLayersType as any, + ignoreLayersType: options.ignoreLayersType as ExpressLayerType[], }), ); @@ -85,7 +107,7 @@ export const instrumentExpressV5 = generateInstrumentOnce( requestHook: span => requestHook(span), spanNameHook: (info, defaultName) => spanNameHook(info, defaultName), ignoreLayers: options.ignoreLayers, - ignoreLayersType: options.ignoreLayersType as any, + ignoreLayersType: options.ignoreLayersType as ExpressLayerType[], }), ); diff --git a/packages/node/test/integrations/tracing/express.test.ts b/packages/node/test/integrations/tracing/express.test.ts index 4d63e54b03ed..6db44c58e365 100644 --- a/packages/node/test/integrations/tracing/express.test.ts +++ b/packages/node/test/integrations/tracing/express.test.ts @@ -86,7 +86,7 @@ describe('Express', () => { it('passes both options to instrumentation', () => { instrumentExpress({ ignoreLayers: ['/health'], - ignoreLayersType: ['request_handler'] + ignoreLayersType: ['request_handler'], }); expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); @@ -115,7 +115,7 @@ describe('Express', () => { it('passes options to instrumentExpressV5', () => { instrumentExpressV5({ ignoreLayers: [(path: string) => path.startsWith('/admin')], - ignoreLayersType: ['middleware', 'router'] + ignoreLayersType: ['middleware', 'router'], }); expect(ExpressInstrumentationV5).toHaveBeenCalledTimes(1); @@ -152,7 +152,7 @@ describe('Express', () => { it('passes options from expressIntegration to both instrumentations', () => { expressIntegration({ ignoreLayers: [/^\/api\/v1/], - ignoreLayersType: ['middleware'] + ignoreLayersType: ['middleware'], }).setupOnce!(); expect(ExpressInstrumentation).toHaveBeenCalledTimes(1); @@ -174,7 +174,7 @@ describe('Express', () => { it('passes all layer types from expressIntegration to instrumentation', () => { expressIntegration({ - ignoreLayersType: ['router', 'middleware', 'request_handler'] + ignoreLayersType: ['router', 'middleware', 'request_handler'], }).setupOnce!(); expect(ExpressInstrumentation).toHaveBeenCalledTimes(1);