Skip to content

Commit

Permalink
fix(tracing): Instrument Prisma client in constructor of integration (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
lforst committed Jun 23, 2023
1 parent 9fd5016 commit fc7ef13
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 51 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ jobs:
- 'scripts/**'
- 'packages/core/**'
- 'packages/tracing/**'
- 'packages/tracing-internal/**'
- 'packages/utils/**'
- 'packages/types/**'
- 'packages/integrations/**'
Expand Down
56 changes: 26 additions & 30 deletions packages/tracing-internal/src/node/integrations/prisma.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -36,6 +35,7 @@ type PrismaMiddleware<T = unknown> = (
) => Promise<T>;

interface PrismaClient {
_sentryInstrumented?: boolean;
$use: (cb: PrismaMiddleware) => void;
}

Expand All @@ -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(
Expand All @@ -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
}
}
35 changes: 14 additions & 21 deletions packages/tracing/test/integrations/node/prisma.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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' } },
Expand All @@ -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();
});
});
});

0 comments on commit fc7ef13

Please sign in to comment.