From fc7ef13f359a0e4bb0b96a1762d68ff4ea30c3d5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 23 Jun 2023 10:06:31 +0200 Subject: [PATCH] fix(tracing): Instrument Prisma client in constructor of integration (#8383) --- .github/workflows/build.yml | 1 + .../src/node/integrations/prisma.ts | 56 +++++++++---------- .../test/integrations/node/prisma.test.ts | 35 +++++------- 3 files changed, 41 insertions(+), 51 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3bc95b171d94..b8e3fcafcb34 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -89,6 +89,7 @@ jobs: - 'scripts/**' - 'packages/core/**' - 'packages/tracing/**' + - 'packages/tracing-internal/**' - 'packages/utils/**' - 'packages/types/**' - 'packages/integrations/**' diff --git a/packages/tracing-internal/src/node/integrations/prisma.ts b/packages/tracing-internal/src/node/integrations/prisma.ts index 20a84aa36aaf..ab6fd1f1ae5b 100644 --- a/packages/tracing-internal/src/node/integrations/prisma.ts +++ b/packages/tracing-internal/src/node/integrations/prisma.ts @@ -1,7 +1,6 @@ -import type { Hub } from '@sentry/core'; -import { trace } from '@sentry/core'; -import type { EventProcessor, Integration } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { getCurrentHub, trace } from '@sentry/core'; +import type { Integration } from '@sentry/types'; +import { addNonEnumerableProperty, logger } from '@sentry/utils'; import { shouldDisableAutoInstrumentation } from './utils/node-utils'; @@ -36,6 +35,7 @@ type PrismaMiddleware = ( ) => Promise; interface PrismaClient { + _sentryInstrumented?: boolean; $use: (cb: PrismaMiddleware) => void; } @@ -55,17 +55,30 @@ export class Prisma implements Integration { */ public name: string = Prisma.id; - /** - * Prisma ORM Client Instance - */ - private readonly _client?: PrismaClient; - /** * @inheritDoc */ public constructor(options: { client?: unknown } = {}) { - if (isValidPrismaClient(options.client)) { - this._client = options.client; + // We instrument the PrismaClient inside the constructor and not inside `setupOnce` because in some cases of server-side + // bundling (Next.js) multiple Prisma clients can be instantiated, even though users don't intend to. When instrumenting + // in setupOnce we can only ever instrument one client. + // https://github.com/getsentry/sentry-javascript/issues/7216#issuecomment-1602375012 + // In the future we might explore providing a dedicated PrismaClient middleware instead of this hack. + if (isValidPrismaClient(options.client) && !options.client._sentryInstrumented) { + addNonEnumerableProperty(options.client as any, '_sentryInstrumented', true); + + options.client.$use((params, next: (params: PrismaMiddlewareParams) => Promise) => { + if (shouldDisableAutoInstrumentation(getCurrentHub)) { + return next(params); + } + + const action = params.action; + const model = params.model; + return trace( + { name: model ? `${model} ${action}` : action, op: 'db.sql.prisma', data: { 'db.system': 'prisma' } }, + () => next(params), + ); + }); } else { __DEBUG_BUILD__ && logger.warn( @@ -77,24 +90,7 @@ export class Prisma implements Integration { /** * @inheritDoc */ - public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - if (!this._client) { - __DEBUG_BUILD__ && logger.error('PrismaIntegration is missing a Prisma Client Instance'); - return; - } - - if (shouldDisableAutoInstrumentation(getCurrentHub)) { - __DEBUG_BUILD__ && logger.log('Prisma Integration is skipped because of instrumenter configuration.'); - return; - } - - this._client.$use((params, next: (params: PrismaMiddlewareParams) => Promise) => { - const action = params.action; - const model = params.model; - return trace( - { name: model ? `${model} ${action}` : action, op: 'db.sql.prisma', data: { 'db.system': 'prisma' } }, - () => next(params), - ); - }); + public setupOnce(): void { + // Noop - here for backwards compatibility } } diff --git a/packages/tracing/test/integrations/node/prisma.test.ts b/packages/tracing/test/integrations/node/prisma.test.ts index 3096401ec43a..61c0e5fb07f6 100644 --- a/packages/tracing/test/integrations/node/prisma.test.ts +++ b/packages/tracing/test/integrations/node/prisma.test.ts @@ -1,7 +1,6 @@ /* eslint-disable deprecation/deprecation */ -/* eslint-disable @typescript-eslint/unbound-method */ -import { Hub, Scope } from '@sentry/core'; -import { logger } from '@sentry/utils'; +import * as sentryCore from '@sentry/core'; +import { Hub } from '@sentry/core'; import { Integrations } from '../../../src'; import { getTestClient } from '../../testutils'; @@ -38,21 +37,15 @@ class PrismaClient { } describe('setupOnce', function () { - const Client: PrismaClient = new PrismaClient(); - - beforeAll(() => { - new Integrations.Prisma({ client: Client }).setupOnce( - () => undefined, - () => new Hub(undefined, new Scope()), - ); - }); - beforeEach(() => { mockTrace.mockClear(); + mockTrace.mockReset(); }); it('should add middleware with $use method correctly', done => { - void Client.user.create()?.then(() => { + const prismaClient = new PrismaClient(); + new Integrations.Prisma({ client: prismaClient }); + void prismaClient.user.create()?.then(() => { expect(mockTrace).toHaveBeenCalledTimes(1); expect(mockTrace).toHaveBeenLastCalledWith( { name: 'user create', op: 'db.sql.prisma', data: { 'db.system': 'prisma' } }, @@ -62,18 +55,18 @@ describe('setupOnce', function () { }); }); - it("doesn't attach when using otel instrumenter", () => { - const loggerLogSpy = jest.spyOn(logger, 'log'); + it("doesn't trace when using otel instrumenter", done => { + const prismaClient = new PrismaClient(); + new Integrations.Prisma({ client: prismaClient }); const client = getTestClient({ instrumenter: 'otel' }); const hub = new Hub(client); - const integration = new Integrations.Prisma({ client: Client }); - integration.setupOnce( - () => {}, - () => hub, - ); + jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); - expect(loggerLogSpy).toBeCalledWith('Prisma Integration is skipped because of instrumenter configuration.'); + void prismaClient.user.create()?.then(() => { + expect(mockTrace).not.toHaveBeenCalled(); + done(); + }); }); });