From d92a7fc4bc9a4fb1b2d9f2908243da6b1fca9a85 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 7 Nov 2022 11:40:42 +0100 Subject: [PATCH 1/3] chore(test): Add tests for skipping of integrations for otel --- packages/node/src/integrations/http.ts | 7 +++++++ packages/node/test/integrations/http.test.ts | 20 +++++++++++++++++++ .../tracing/src/integrations/node/apollo.ts | 7 +++++++ .../tracing/src/integrations/node/express.ts | 7 +++++++ .../tracing/src/integrations/node/graphql.ts | 7 +++++++ .../tracing/src/integrations/node/mongo.ts | 7 +++++++ .../tracing/src/integrations/node/mysql.ts | 7 +++++++ .../tracing/src/integrations/node/postgres.ts | 7 +++++++ .../tracing/src/integrations/node/prisma.ts | 7 +++++++ .../tracing/test/integrations/apollo.test.ts | 14 +++++++++++++ .../tracing/test/integrations/graphql.test.ts | 14 +++++++++++++ .../test/integrations/node/mongo.test.ts | 14 +++++++++++++ .../test/integrations/node/postgres.test.ts | 14 +++++++++++++ .../test/integrations/node/prisma.test.ts | 14 +++++++++++++ packages/tracing/test/testutils.ts | 18 ++++++++++++++++- 15 files changed, 163 insertions(+), 1 deletion(-) diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index b95faf58d848..451f6d532d5b 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -38,6 +38,11 @@ export class Http implements Integration { */ public name: string = Http.id; + /** + * If the integration was skipped due to internal checks. + */ + public _wasSkipped: boolean = false; + /** * @inheritDoc */ @@ -65,6 +70,7 @@ export class Http implements Integration { ): void { // No need to instrument if we don't want to track anything if (!this._breadcrumbs && !this._tracing) { + this._wasSkipped = true; return; } @@ -72,6 +78,7 @@ export class Http implements Integration { // Do not auto-instrument for other instrumenter if (clientOptions && clientOptions.instrumenter !== 'sentry') { + this._wasSkipped = true; return; } diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 4f5205f0b596..520649631904 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -188,6 +188,26 @@ describe('tracing', () => { expect(transaction.metadata.propagations).toBe(2); }); + it("doesn't attach when using otel instrumenter", () => { + const options = getDefaultNodeClientOptions({ + dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', + tracesSampleRate: 1.0, + integrations: [new HttpIntegration({ tracing: true })], + release: '1.0.0', + environment: 'production', + instrumenter: 'otel', + }); + const hub = new Hub(new NodeClient(options)); + + const integration = new HttpIntegration() as HttpIntegration & { _wasSkipped: boolean }; + integration.setupOnce( + () => {}, + () => hub, + ); + + expect(integration._wasSkipped).toBe(true); + }); + describe('tracePropagationTargets option', () => { beforeEach(() => { // hacky way of restoring monkey patched functions diff --git a/packages/tracing/src/integrations/node/apollo.ts b/packages/tracing/src/integrations/node/apollo.ts index e505899df9f2..8334f68ec73e 100644 --- a/packages/tracing/src/integrations/node/apollo.ts +++ b/packages/tracing/src/integrations/node/apollo.ts @@ -24,6 +24,11 @@ export class Apollo implements Integration { */ public name: string = Apollo.id; + /** + * If the integration was skipped due to internal checks. + */ + public _wasSkipped: boolean = false; + /** * @inheritDoc */ @@ -38,11 +43,13 @@ export class Apollo implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error('Apollo Integration was unable to require apollo-server-core package.'); + this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Apollo Integration is skipped because of instrumenter configuration.'); + this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/express.ts b/packages/tracing/src/integrations/node/express.ts index 24260f627c0f..31a440d2139c 100644 --- a/packages/tracing/src/integrations/node/express.ts +++ b/packages/tracing/src/integrations/node/express.ts @@ -90,6 +90,11 @@ export class Express implements Integration { */ public name: string = Express.id; + /** + * If the integration was skipped due to internal checks. + */ + public _wasSkipped: boolean = false; + /** * Express App instance */ @@ -110,11 +115,13 @@ export class Express implements Integration { public setupOnce(_: unknown, getCurrentHub: () => Hub): void { if (!this._router) { __DEBUG_BUILD__ && logger.error('ExpressIntegration is missing an Express instance'); + this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Express Integration is skipped because of instrumenter configuration.'); + this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/graphql.ts b/packages/tracing/src/integrations/node/graphql.ts index 4e777459cd80..b70f11e990fb 100644 --- a/packages/tracing/src/integrations/node/graphql.ts +++ b/packages/tracing/src/integrations/node/graphql.ts @@ -16,6 +16,11 @@ export class GraphQL implements Integration { */ public name: string = GraphQL.id; + /** + * If the integration was skipped due to internal checks. + */ + public _wasSkipped: boolean = false; + /** * @inheritDoc */ @@ -26,11 +31,13 @@ export class GraphQL implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error('GraphQL Integration was unable to require graphql/execution package.'); + this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('GraphQL Integration is skipped because of instrumenter configuration.'); + this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/mongo.ts b/packages/tracing/src/integrations/node/mongo.ts index e19711d12460..47c8ca14ff36 100644 --- a/packages/tracing/src/integrations/node/mongo.ts +++ b/packages/tracing/src/integrations/node/mongo.ts @@ -102,6 +102,11 @@ export class Mongo implements Integration { */ public name: string = Mongo.id; + /** + * If the integration was skipped due to internal checks. + */ + public _wasSkipped: boolean = false; + private _operations: Operation[]; private _describeOperations?: boolean | Operation[]; private _useMongoose: boolean; @@ -124,11 +129,13 @@ export class Mongo implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error(`Mongo Integration was unable to require \`${moduleName}\` package.`); + this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Mongo Integration is skipped because of instrumenter configuration.'); + this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/mysql.ts b/packages/tracing/src/integrations/node/mysql.ts index a9e38cd1c0e7..d847ca69205b 100644 --- a/packages/tracing/src/integrations/node/mysql.ts +++ b/packages/tracing/src/integrations/node/mysql.ts @@ -20,6 +20,11 @@ export class Mysql implements Integration { */ public name: string = Mysql.id; + /** + * If the integration was skipped due to internal checks. + */ + public _wasSkipped: boolean = false; + /** * @inheritDoc */ @@ -28,11 +33,13 @@ export class Mysql implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error('Mysql Integration was unable to require `mysql` package.'); + this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Mysql Integration is skipped because of instrumenter configuration.'); + this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/postgres.ts b/packages/tracing/src/integrations/node/postgres.ts index 6ed0f46c5dd7..812d365be653 100644 --- a/packages/tracing/src/integrations/node/postgres.ts +++ b/packages/tracing/src/integrations/node/postgres.ts @@ -26,6 +26,11 @@ export class Postgres implements Integration { */ public name: string = Postgres.id; + /** + * If the integration was skipped due to internal checks. + */ + public _wasSkipped: boolean = false; + private _usePgNative: boolean; public constructor(options: PgOptions = {}) { @@ -40,11 +45,13 @@ export class Postgres implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error('Postgres Integration was unable to require `pg` package.'); + this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Postgres Integration is skipped because of instrumenter configuration.'); + this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/prisma.ts b/packages/tracing/src/integrations/node/prisma.ts index 371052217911..c40758234423 100644 --- a/packages/tracing/src/integrations/node/prisma.ts +++ b/packages/tracing/src/integrations/node/prisma.ts @@ -54,6 +54,11 @@ export class Prisma implements Integration { */ public name: string = Prisma.id; + /** + * If the integration was skipped due to internal checks. + */ + public _wasSkipped: boolean = false; + /** * Prisma ORM Client Instance */ @@ -79,11 +84,13 @@ export class Prisma implements Integration { public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { if (!this._client) { __DEBUG_BUILD__ && logger.error('PrismaIntegration is missing a Prisma Client Instance'); + this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Prisma Integration is skipped because of instrumenter configuration.'); + this._wasSkipped = true; return; } diff --git a/packages/tracing/test/integrations/apollo.test.ts b/packages/tracing/test/integrations/apollo.test.ts index 2f8e5c843d07..446248156766 100644 --- a/packages/tracing/test/integrations/apollo.test.ts +++ b/packages/tracing/test/integrations/apollo.test.ts @@ -3,6 +3,7 @@ import { Hub, Scope } from '@sentry/core'; import { Apollo } from '../../src/integrations/node/apollo'; import { Span } from '../../src/span'; +import { getTestClient } from '../testutils'; type ApolloResolverGroup = { [key: string]: () => any; @@ -100,4 +101,17 @@ describe('setupOnce', () => { }); expect(childSpan.finish).toBeCalled(); }); + + it("doesn't attach when using otel instrumenter", () => { + const client = getTestClient({ instrumenter: 'otel' }); + const hub = new Hub(client); + + const integration = new Apollo() as Apollo & { _wasSkipped: boolean }; + integration.setupOnce( + () => {}, + () => hub, + ); + + expect(integration._wasSkipped).toBe(true); + }); }); diff --git a/packages/tracing/test/integrations/graphql.test.ts b/packages/tracing/test/integrations/graphql.test.ts index e074dfc602f8..357eea327071 100644 --- a/packages/tracing/test/integrations/graphql.test.ts +++ b/packages/tracing/test/integrations/graphql.test.ts @@ -3,6 +3,7 @@ import { Hub, Scope } from '@sentry/core'; import { GraphQL } from '../../src/integrations/node/graphql'; import { Span } from '../../src/span'; +import { getTestClient } from '../testutils'; const GQLExecute = { execute() { @@ -53,4 +54,17 @@ describe('setupOnce', () => { expect(childSpan.finish).toBeCalled(); expect(scope.setSpan).toHaveBeenCalledTimes(2); }); + + it("doesn't attach when using otel instrumenter", () => { + const client = getTestClient({ instrumenter: 'otel' }); + const hub = new Hub(client); + + const integration = new GraphQL() as GraphQL & { _wasSkipped: boolean }; + integration.setupOnce( + () => {}, + () => hub, + ); + + expect(integration._wasSkipped).toBe(true); + }); }); diff --git a/packages/tracing/test/integrations/node/mongo.test.ts b/packages/tracing/test/integrations/node/mongo.test.ts index b7417a6ae9b2..fa5cdb41790e 100644 --- a/packages/tracing/test/integrations/node/mongo.test.ts +++ b/packages/tracing/test/integrations/node/mongo.test.ts @@ -3,6 +3,7 @@ import { Hub, Scope } from '@sentry/core'; import { Mongo } from '../../../src/integrations/node/mongo'; import { Span } from '../../../src/span'; +import { getTestClient } from '../../testutils'; class Collection { public collectionName: string = 'mockedCollectionName'; @@ -111,4 +112,17 @@ describe('patchOperation()', () => { }); expect(childSpan.finish).toBeCalled(); }); + + it("doesn't attach when using otel instrumenter", () => { + const client = getTestClient({ instrumenter: 'otel' }); + const hub = new Hub(client); + + const integration = new Mongo() as Mongo & { _wasSkipped: boolean }; + integration.setupOnce( + () => {}, + () => hub, + ); + + expect(integration._wasSkipped).toBe(true); + }); }); diff --git a/packages/tracing/test/integrations/node/postgres.test.ts b/packages/tracing/test/integrations/node/postgres.test.ts index 4b6c23db41e6..6b7c9ad232b8 100644 --- a/packages/tracing/test/integrations/node/postgres.test.ts +++ b/packages/tracing/test/integrations/node/postgres.test.ts @@ -3,6 +3,7 @@ import { Hub, Scope } from '@sentry/core'; import { Postgres } from '../../../src/integrations/node/postgres'; import { Span } from '../../../src/span'; +import { getTestClient } from '../../testutils'; class PgClient { // https://node-postgres.com/api/client#clientquery @@ -94,4 +95,17 @@ describe('setupOnce', () => { expect(childSpan.finish).toBeCalled(); }); }); + + it("doesn't attach when using otel instrumenter", () => { + const client = getTestClient({ instrumenter: 'otel' }); + const hub = new Hub(client); + + const integration = new Postgres() as Postgres & { _wasSkipped: boolean }; + integration.setupOnce( + () => {}, + () => hub, + ); + + expect(integration._wasSkipped).toBe(true); + }); }); diff --git a/packages/tracing/test/integrations/node/prisma.test.ts b/packages/tracing/test/integrations/node/prisma.test.ts index 3c088ac25834..67260703552b 100644 --- a/packages/tracing/test/integrations/node/prisma.test.ts +++ b/packages/tracing/test/integrations/node/prisma.test.ts @@ -3,6 +3,7 @@ import { Hub, Scope } from '@sentry/core'; import { Prisma } from '../../../src/integrations/node/prisma'; import { Span } from '../../../src/span'; +import { getTestClient } from '../../testutils'; type PrismaMiddleware = (params: unknown, next: (params?: unknown) => Promise) => Promise; @@ -58,4 +59,17 @@ describe('setupOnce', function () { done(); }); }); + + it("doesn't attach when using otel instrumenter", () => { + const client = getTestClient({ instrumenter: 'otel' }); + const hub = new Hub(client); + + const integration = new Prisma() as Prisma & { _wasSkipped: boolean }; + integration.setupOnce( + () => {}, + () => hub, + ); + + expect(integration._wasSkipped).toBe(true); + }); }); diff --git a/packages/tracing/test/testutils.ts b/packages/tracing/test/testutils.ts index 474eb57846f4..4ddd042a1240 100644 --- a/packages/tracing/test/testutils.ts +++ b/packages/tracing/test/testutils.ts @@ -1,5 +1,5 @@ import { createTransport } from '@sentry/browser'; -import { ClientOptions } from '@sentry/types'; +import { Client, ClientOptions } from '@sentry/types'; import { GLOBAL_OBJ, resolvedSyncPromise } from '@sentry/utils'; import { JSDOM } from 'jsdom'; @@ -66,3 +66,19 @@ export function getDefaultBrowserClientOptions(options: Partial = ...options, }; } + +export function getTestClient(options: Partial): Client { + class TestClient { + _options: Partial; + + constructor(options: Partial) { + this._options = options; + } + + getOptions(): Partial { + return this._options; + } + } + + return new TestClient(options) as unknown as Client; +} From 46a49177f11ab1cae5f31467971682a0f580e129 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 8 Nov 2022 14:16:12 +0100 Subject: [PATCH 2/3] chore: Make `_wasSkipped` field on integrations private --- packages/node/src/integrations/http.ts | 3 ++- packages/node/test/integrations/http.test.ts | 4 ++-- packages/tracing/src/integrations/node/apollo.ts | 3 ++- packages/tracing/src/integrations/node/express.ts | 3 ++- packages/tracing/src/integrations/node/graphql.ts | 3 ++- packages/tracing/src/integrations/node/mongo.ts | 3 ++- packages/tracing/src/integrations/node/mysql.ts | 3 ++- packages/tracing/src/integrations/node/postgres.ts | 3 ++- packages/tracing/src/integrations/node/prisma.ts | 3 ++- packages/tracing/test/integrations/apollo.test.ts | 3 ++- packages/tracing/test/integrations/graphql.test.ts | 3 ++- packages/tracing/test/integrations/node/mongo.test.ts | 3 ++- packages/tracing/test/integrations/node/postgres.test.ts | 3 ++- packages/tracing/test/integrations/node/prisma.test.ts | 3 ++- 14 files changed, 28 insertions(+), 15 deletions(-) diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 451f6d532d5b..a6354417b7a9 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -41,7 +41,8 @@ export class Http implements Integration { /** * If the integration was skipped due to internal checks. */ - public _wasSkipped: boolean = false; + // @ts-ignore This is only used for tests + private _wasSkipped: boolean = false; /** * @inheritDoc diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 520649631904..7e39fba3f51a 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -1,7 +1,7 @@ import * as sentryCore from '@sentry/core'; import { Hub } from '@sentry/core'; import { addExtensionMethods, Span, TRACEPARENT_REGEXP, Transaction } from '@sentry/tracing'; -import { TransactionContext } from '@sentry/types'; +import { Integration, TransactionContext } from '@sentry/types'; import { parseSemver } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; @@ -199,7 +199,7 @@ describe('tracing', () => { }); const hub = new Hub(new NodeClient(options)); - const integration = new HttpIntegration() as HttpIntegration & { _wasSkipped: boolean }; + const integration = new HttpIntegration() as unknown as Integration & { _wasSkipped: boolean }; integration.setupOnce( () => {}, () => hub, diff --git a/packages/tracing/src/integrations/node/apollo.ts b/packages/tracing/src/integrations/node/apollo.ts index 8334f68ec73e..abc762ced92a 100644 --- a/packages/tracing/src/integrations/node/apollo.ts +++ b/packages/tracing/src/integrations/node/apollo.ts @@ -27,7 +27,8 @@ export class Apollo implements Integration { /** * If the integration was skipped due to internal checks. */ - public _wasSkipped: boolean = false; + // @ts-ignore This is only used for tests + private _wasSkipped: boolean = false; /** * @inheritDoc diff --git a/packages/tracing/src/integrations/node/express.ts b/packages/tracing/src/integrations/node/express.ts index 31a440d2139c..408f5abcc2ed 100644 --- a/packages/tracing/src/integrations/node/express.ts +++ b/packages/tracing/src/integrations/node/express.ts @@ -93,7 +93,8 @@ export class Express implements Integration { /** * If the integration was skipped due to internal checks. */ - public _wasSkipped: boolean = false; + // @ts-ignore This is only used for tests + private _wasSkipped: boolean = false; /** * Express App instance diff --git a/packages/tracing/src/integrations/node/graphql.ts b/packages/tracing/src/integrations/node/graphql.ts index b70f11e990fb..e17d5443fe9f 100644 --- a/packages/tracing/src/integrations/node/graphql.ts +++ b/packages/tracing/src/integrations/node/graphql.ts @@ -19,7 +19,8 @@ export class GraphQL implements Integration { /** * If the integration was skipped due to internal checks. */ - public _wasSkipped: boolean = false; + // @ts-ignore This is only used for tests + private _wasSkipped: boolean = false; /** * @inheritDoc diff --git a/packages/tracing/src/integrations/node/mongo.ts b/packages/tracing/src/integrations/node/mongo.ts index 47c8ca14ff36..611c765bd19a 100644 --- a/packages/tracing/src/integrations/node/mongo.ts +++ b/packages/tracing/src/integrations/node/mongo.ts @@ -105,7 +105,8 @@ export class Mongo implements Integration { /** * If the integration was skipped due to internal checks. */ - public _wasSkipped: boolean = false; + // @ts-ignore This is only used for tests + private _wasSkipped: boolean = false; private _operations: Operation[]; private _describeOperations?: boolean | Operation[]; diff --git a/packages/tracing/src/integrations/node/mysql.ts b/packages/tracing/src/integrations/node/mysql.ts index d847ca69205b..4d41b94ac7c3 100644 --- a/packages/tracing/src/integrations/node/mysql.ts +++ b/packages/tracing/src/integrations/node/mysql.ts @@ -23,7 +23,8 @@ export class Mysql implements Integration { /** * If the integration was skipped due to internal checks. */ - public _wasSkipped: boolean = false; + // @ts-ignore This is only used for tests + private _wasSkipped: boolean = false; /** * @inheritDoc diff --git a/packages/tracing/src/integrations/node/postgres.ts b/packages/tracing/src/integrations/node/postgres.ts index 812d365be653..8e82f4e0f94e 100644 --- a/packages/tracing/src/integrations/node/postgres.ts +++ b/packages/tracing/src/integrations/node/postgres.ts @@ -29,7 +29,8 @@ export class Postgres implements Integration { /** * If the integration was skipped due to internal checks. */ - public _wasSkipped: boolean = false; + // @ts-ignore This is only used for tests + private _wasSkipped: boolean = false; private _usePgNative: boolean; diff --git a/packages/tracing/src/integrations/node/prisma.ts b/packages/tracing/src/integrations/node/prisma.ts index c40758234423..1848a8343e35 100644 --- a/packages/tracing/src/integrations/node/prisma.ts +++ b/packages/tracing/src/integrations/node/prisma.ts @@ -57,7 +57,8 @@ export class Prisma implements Integration { /** * If the integration was skipped due to internal checks. */ - public _wasSkipped: boolean = false; + // @ts-ignore This is only used for tests + private _wasSkipped: boolean = false; /** * Prisma ORM Client Instance diff --git a/packages/tracing/test/integrations/apollo.test.ts b/packages/tracing/test/integrations/apollo.test.ts index 446248156766..3bf0fb093dfb 100644 --- a/packages/tracing/test/integrations/apollo.test.ts +++ b/packages/tracing/test/integrations/apollo.test.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; +import { Integration } from '@sentry/types'; import { Apollo } from '../../src/integrations/node/apollo'; import { Span } from '../../src/span'; @@ -106,7 +107,7 @@ describe('setupOnce', () => { const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new Apollo() as Apollo & { _wasSkipped: boolean }; + const integration = new Apollo() as unknown as Integration & { _wasSkipped: boolean }; integration.setupOnce( () => {}, () => hub, diff --git a/packages/tracing/test/integrations/graphql.test.ts b/packages/tracing/test/integrations/graphql.test.ts index 357eea327071..cbda491477a3 100644 --- a/packages/tracing/test/integrations/graphql.test.ts +++ b/packages/tracing/test/integrations/graphql.test.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; +import { Integration } from '@sentry/types'; import { GraphQL } from '../../src/integrations/node/graphql'; import { Span } from '../../src/span'; @@ -59,7 +60,7 @@ describe('setupOnce', () => { const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new GraphQL() as GraphQL & { _wasSkipped: boolean }; + const integration = new GraphQL() as unknown as Integration & { _wasSkipped: boolean }; integration.setupOnce( () => {}, () => hub, diff --git a/packages/tracing/test/integrations/node/mongo.test.ts b/packages/tracing/test/integrations/node/mongo.test.ts index fa5cdb41790e..7b07ac570152 100644 --- a/packages/tracing/test/integrations/node/mongo.test.ts +++ b/packages/tracing/test/integrations/node/mongo.test.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; +import { Integration } from '@sentry/types'; import { Mongo } from '../../../src/integrations/node/mongo'; import { Span } from '../../../src/span'; @@ -117,7 +118,7 @@ describe('patchOperation()', () => { const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new Mongo() as Mongo & { _wasSkipped: boolean }; + const integration = new Mongo() as unknown as Integration & { _wasSkipped: boolean }; integration.setupOnce( () => {}, () => hub, diff --git a/packages/tracing/test/integrations/node/postgres.test.ts b/packages/tracing/test/integrations/node/postgres.test.ts index 6b7c9ad232b8..7b51aa155211 100644 --- a/packages/tracing/test/integrations/node/postgres.test.ts +++ b/packages/tracing/test/integrations/node/postgres.test.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; +import { Integration } from '@sentry/types'; import { Postgres } from '../../../src/integrations/node/postgres'; import { Span } from '../../../src/span'; @@ -100,7 +101,7 @@ describe('setupOnce', () => { const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new Postgres() as Postgres & { _wasSkipped: boolean }; + const integration = new Postgres() as unknown as Integration & { _wasSkipped: boolean }; integration.setupOnce( () => {}, () => hub, diff --git a/packages/tracing/test/integrations/node/prisma.test.ts b/packages/tracing/test/integrations/node/prisma.test.ts index 67260703552b..ff31331de8d8 100644 --- a/packages/tracing/test/integrations/node/prisma.test.ts +++ b/packages/tracing/test/integrations/node/prisma.test.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; +import { Integration } from '@sentry/types'; import { Prisma } from '../../../src/integrations/node/prisma'; import { Span } from '../../../src/span'; @@ -64,7 +65,7 @@ describe('setupOnce', function () { const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new Prisma() as Prisma & { _wasSkipped: boolean }; + const integration = new Prisma() as unknown as Integration & { _wasSkipped: boolean }; integration.setupOnce( () => {}, () => hub, From d50e97d52062c93eb57340f122f51d47ee667d3b Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 8 Nov 2022 14:56:57 +0100 Subject: [PATCH 3/3] chore: Test integration skipping by inspecting logger --- packages/node/src/integrations/http.ts | 9 +-------- packages/node/test/integrations/http.test.ts | 10 ++++++---- packages/tracing/src/integrations/node/apollo.ts | 8 -------- packages/tracing/src/integrations/node/express.ts | 8 -------- packages/tracing/src/integrations/node/graphql.ts | 8 -------- packages/tracing/src/integrations/node/mongo.ts | 8 -------- packages/tracing/src/integrations/node/mysql.ts | 8 -------- packages/tracing/src/integrations/node/postgres.ts | 8 -------- packages/tracing/src/integrations/node/prisma.ts | 8 -------- packages/tracing/test/integrations/apollo.test.ts | 8 +++++--- packages/tracing/test/integrations/graphql.test.ts | 8 +++++--- packages/tracing/test/integrations/node/mongo.test.ts | 8 +++++--- .../tracing/test/integrations/node/postgres.test.ts | 8 +++++--- packages/tracing/test/integrations/node/prisma.test.ts | 9 +++++---- 14 files changed, 32 insertions(+), 84 deletions(-) diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index a6354417b7a9..4cdd58f7ee25 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -38,12 +38,6 @@ export class Http implements Integration { */ public name: string = Http.id; - /** - * If the integration was skipped due to internal checks. - */ - // @ts-ignore This is only used for tests - private _wasSkipped: boolean = false; - /** * @inheritDoc */ @@ -71,7 +65,6 @@ export class Http implements Integration { ): void { // No need to instrument if we don't want to track anything if (!this._breadcrumbs && !this._tracing) { - this._wasSkipped = true; return; } @@ -79,7 +72,7 @@ export class Http implements Integration { // Do not auto-instrument for other instrumenter if (clientOptions && clientOptions.instrumenter !== 'sentry') { - this._wasSkipped = true; + __DEBUG_BUILD__ && logger.log('HTTP Integration is skipped because of instrumenter configuration.'); return; } diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 7e39fba3f51a..6bf8a7117327 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -1,8 +1,8 @@ import * as sentryCore from '@sentry/core'; import { Hub } from '@sentry/core'; import { addExtensionMethods, Span, TRACEPARENT_REGEXP, Transaction } from '@sentry/tracing'; -import { Integration, TransactionContext } from '@sentry/types'; -import { parseSemver } from '@sentry/utils'; +import { TransactionContext } from '@sentry/types'; +import { logger, parseSemver } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; import * as HttpsProxyAgent from 'https-proxy-agent'; @@ -189,6 +189,8 @@ describe('tracing', () => { }); it("doesn't attach when using otel instrumenter", () => { + const loggerLogSpy = jest.spyOn(logger, 'log'); + const options = getDefaultNodeClientOptions({ dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', tracesSampleRate: 1.0, @@ -199,13 +201,13 @@ describe('tracing', () => { }); const hub = new Hub(new NodeClient(options)); - const integration = new HttpIntegration() as unknown as Integration & { _wasSkipped: boolean }; + const integration = new HttpIntegration(); integration.setupOnce( () => {}, () => hub, ); - expect(integration._wasSkipped).toBe(true); + expect(loggerLogSpy).toBeCalledWith('HTTP Integration is skipped because of instrumenter configuration.'); }); describe('tracePropagationTargets option', () => { diff --git a/packages/tracing/src/integrations/node/apollo.ts b/packages/tracing/src/integrations/node/apollo.ts index abc762ced92a..e505899df9f2 100644 --- a/packages/tracing/src/integrations/node/apollo.ts +++ b/packages/tracing/src/integrations/node/apollo.ts @@ -24,12 +24,6 @@ export class Apollo implements Integration { */ public name: string = Apollo.id; - /** - * If the integration was skipped due to internal checks. - */ - // @ts-ignore This is only used for tests - private _wasSkipped: boolean = false; - /** * @inheritDoc */ @@ -44,13 +38,11 @@ export class Apollo implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error('Apollo Integration was unable to require apollo-server-core package.'); - this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Apollo Integration is skipped because of instrumenter configuration.'); - this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/express.ts b/packages/tracing/src/integrations/node/express.ts index 408f5abcc2ed..24260f627c0f 100644 --- a/packages/tracing/src/integrations/node/express.ts +++ b/packages/tracing/src/integrations/node/express.ts @@ -90,12 +90,6 @@ export class Express implements Integration { */ public name: string = Express.id; - /** - * If the integration was skipped due to internal checks. - */ - // @ts-ignore This is only used for tests - private _wasSkipped: boolean = false; - /** * Express App instance */ @@ -116,13 +110,11 @@ export class Express implements Integration { public setupOnce(_: unknown, getCurrentHub: () => Hub): void { if (!this._router) { __DEBUG_BUILD__ && logger.error('ExpressIntegration is missing an Express instance'); - this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Express Integration is skipped because of instrumenter configuration.'); - this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/graphql.ts b/packages/tracing/src/integrations/node/graphql.ts index e17d5443fe9f..4e777459cd80 100644 --- a/packages/tracing/src/integrations/node/graphql.ts +++ b/packages/tracing/src/integrations/node/graphql.ts @@ -16,12 +16,6 @@ export class GraphQL implements Integration { */ public name: string = GraphQL.id; - /** - * If the integration was skipped due to internal checks. - */ - // @ts-ignore This is only used for tests - private _wasSkipped: boolean = false; - /** * @inheritDoc */ @@ -32,13 +26,11 @@ export class GraphQL implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error('GraphQL Integration was unable to require graphql/execution package.'); - this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('GraphQL Integration is skipped because of instrumenter configuration.'); - this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/mongo.ts b/packages/tracing/src/integrations/node/mongo.ts index 611c765bd19a..e19711d12460 100644 --- a/packages/tracing/src/integrations/node/mongo.ts +++ b/packages/tracing/src/integrations/node/mongo.ts @@ -102,12 +102,6 @@ export class Mongo implements Integration { */ public name: string = Mongo.id; - /** - * If the integration was skipped due to internal checks. - */ - // @ts-ignore This is only used for tests - private _wasSkipped: boolean = false; - private _operations: Operation[]; private _describeOperations?: boolean | Operation[]; private _useMongoose: boolean; @@ -130,13 +124,11 @@ export class Mongo implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error(`Mongo Integration was unable to require \`${moduleName}\` package.`); - this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Mongo Integration is skipped because of instrumenter configuration.'); - this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/mysql.ts b/packages/tracing/src/integrations/node/mysql.ts index 4d41b94ac7c3..a9e38cd1c0e7 100644 --- a/packages/tracing/src/integrations/node/mysql.ts +++ b/packages/tracing/src/integrations/node/mysql.ts @@ -20,12 +20,6 @@ export class Mysql implements Integration { */ public name: string = Mysql.id; - /** - * If the integration was skipped due to internal checks. - */ - // @ts-ignore This is only used for tests - private _wasSkipped: boolean = false; - /** * @inheritDoc */ @@ -34,13 +28,11 @@ export class Mysql implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error('Mysql Integration was unable to require `mysql` package.'); - this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Mysql Integration is skipped because of instrumenter configuration.'); - this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/postgres.ts b/packages/tracing/src/integrations/node/postgres.ts index 8e82f4e0f94e..6ed0f46c5dd7 100644 --- a/packages/tracing/src/integrations/node/postgres.ts +++ b/packages/tracing/src/integrations/node/postgres.ts @@ -26,12 +26,6 @@ export class Postgres implements Integration { */ public name: string = Postgres.id; - /** - * If the integration was skipped due to internal checks. - */ - // @ts-ignore This is only used for tests - private _wasSkipped: boolean = false; - private _usePgNative: boolean; public constructor(options: PgOptions = {}) { @@ -46,13 +40,11 @@ export class Postgres implements Integration { if (!pkg) { __DEBUG_BUILD__ && logger.error('Postgres Integration was unable to require `pg` package.'); - this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Postgres Integration is skipped because of instrumenter configuration.'); - this._wasSkipped = true; return; } diff --git a/packages/tracing/src/integrations/node/prisma.ts b/packages/tracing/src/integrations/node/prisma.ts index 1848a8343e35..371052217911 100644 --- a/packages/tracing/src/integrations/node/prisma.ts +++ b/packages/tracing/src/integrations/node/prisma.ts @@ -54,12 +54,6 @@ export class Prisma implements Integration { */ public name: string = Prisma.id; - /** - * If the integration was skipped due to internal checks. - */ - // @ts-ignore This is only used for tests - private _wasSkipped: boolean = false; - /** * Prisma ORM Client Instance */ @@ -85,13 +79,11 @@ export class Prisma implements Integration { public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { if (!this._client) { __DEBUG_BUILD__ && logger.error('PrismaIntegration is missing a Prisma Client Instance'); - this._wasSkipped = true; return; } if (shouldDisableAutoInstrumentation(getCurrentHub)) { __DEBUG_BUILD__ && logger.log('Prisma Integration is skipped because of instrumenter configuration.'); - this._wasSkipped = true; return; } diff --git a/packages/tracing/test/integrations/apollo.test.ts b/packages/tracing/test/integrations/apollo.test.ts index 3bf0fb093dfb..8b9946ba4a70 100644 --- a/packages/tracing/test/integrations/apollo.test.ts +++ b/packages/tracing/test/integrations/apollo.test.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; -import { Integration } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { Apollo } from '../../src/integrations/node/apollo'; import { Span } from '../../src/span'; @@ -104,15 +104,17 @@ describe('setupOnce', () => { }); it("doesn't attach when using otel instrumenter", () => { + const loggerLogSpy = jest.spyOn(logger, 'log'); + const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new Apollo() as unknown as Integration & { _wasSkipped: boolean }; + const integration = new Apollo(); integration.setupOnce( () => {}, () => hub, ); - expect(integration._wasSkipped).toBe(true); + expect(loggerLogSpy).toBeCalledWith('Apollo Integration is skipped because of instrumenter configuration.'); }); }); diff --git a/packages/tracing/test/integrations/graphql.test.ts b/packages/tracing/test/integrations/graphql.test.ts index cbda491477a3..265bbe0fb0d8 100644 --- a/packages/tracing/test/integrations/graphql.test.ts +++ b/packages/tracing/test/integrations/graphql.test.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; -import { Integration } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { GraphQL } from '../../src/integrations/node/graphql'; import { Span } from '../../src/span'; @@ -57,15 +57,17 @@ describe('setupOnce', () => { }); it("doesn't attach when using otel instrumenter", () => { + const loggerLogSpy = jest.spyOn(logger, 'log'); + const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new GraphQL() as unknown as Integration & { _wasSkipped: boolean }; + const integration = new GraphQL(); integration.setupOnce( () => {}, () => hub, ); - expect(integration._wasSkipped).toBe(true); + expect(loggerLogSpy).toBeCalledWith('GraphQL Integration is skipped because of instrumenter configuration.'); }); }); diff --git a/packages/tracing/test/integrations/node/mongo.test.ts b/packages/tracing/test/integrations/node/mongo.test.ts index 7b07ac570152..e362db6f0c47 100644 --- a/packages/tracing/test/integrations/node/mongo.test.ts +++ b/packages/tracing/test/integrations/node/mongo.test.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; -import { Integration } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { Mongo } from '../../../src/integrations/node/mongo'; import { Span } from '../../../src/span'; @@ -115,15 +115,17 @@ describe('patchOperation()', () => { }); it("doesn't attach when using otel instrumenter", () => { + const loggerLogSpy = jest.spyOn(logger, 'log'); + const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new Mongo() as unknown as Integration & { _wasSkipped: boolean }; + const integration = new Mongo(); integration.setupOnce( () => {}, () => hub, ); - expect(integration._wasSkipped).toBe(true); + expect(loggerLogSpy).toBeCalledWith('Mongo Integration is skipped because of instrumenter configuration.'); }); }); diff --git a/packages/tracing/test/integrations/node/postgres.test.ts b/packages/tracing/test/integrations/node/postgres.test.ts index 7b51aa155211..2ef3754b28ef 100644 --- a/packages/tracing/test/integrations/node/postgres.test.ts +++ b/packages/tracing/test/integrations/node/postgres.test.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; -import { Integration } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { Postgres } from '../../../src/integrations/node/postgres'; import { Span } from '../../../src/span'; @@ -98,15 +98,17 @@ describe('setupOnce', () => { }); it("doesn't attach when using otel instrumenter", () => { + const loggerLogSpy = jest.spyOn(logger, 'log'); + const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new Postgres() as unknown as Integration & { _wasSkipped: boolean }; + const integration = new Postgres(); integration.setupOnce( () => {}, () => hub, ); - expect(integration._wasSkipped).toBe(true); + expect(loggerLogSpy).toBeCalledWith('Postgres Integration is skipped because of instrumenter configuration.'); }); }); diff --git a/packages/tracing/test/integrations/node/prisma.test.ts b/packages/tracing/test/integrations/node/prisma.test.ts index ff31331de8d8..d2adb685fc9d 100644 --- a/packages/tracing/test/integrations/node/prisma.test.ts +++ b/packages/tracing/test/integrations/node/prisma.test.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { Hub, Scope } from '@sentry/core'; -import { Integration } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { Prisma } from '../../../src/integrations/node/prisma'; import { Span } from '../../../src/span'; @@ -32,7 +32,6 @@ describe('setupOnce', function () { let childSpan: Span; beforeAll(() => { - // @ts-ignore, not to export PrismaClient types from integration source new Prisma({ client: Client }).setupOnce( () => undefined, () => new Hub(undefined, scope), @@ -62,15 +61,17 @@ describe('setupOnce', function () { }); it("doesn't attach when using otel instrumenter", () => { + const loggerLogSpy = jest.spyOn(logger, 'log'); + const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new Prisma() as unknown as Integration & { _wasSkipped: boolean }; + const integration = new Prisma({ client: Client }); integration.setupOnce( () => {}, () => hub, ); - expect(integration._wasSkipped).toBe(true); + expect(loggerLogSpy).toBeCalledWith('Prisma Integration is skipped because of instrumenter configuration.'); }); });