fix(tracing): Instrument Prisma client in constructor of integration (#8383)

Author: Luca Forstner

.github/workflows/build.yml

@@ -89,6 +89,7 @@ jobs:
               - 'scripts/**'
               - 'packages/core/**'
               - 'packages/tracing/**'
+              - 'packages/tracing-internal/**'
               - 'packages/utils/**'
               - 'packages/types/**'
               - 'packages/integrations/**'

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<T = unknown> = (
 ) => Promise<T>;
 
 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<unknown>) => {
+        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<unknown>) => {
-      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
   }
 }

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();
+    });
   });
 });