Prisma Tracer not working on production builds

jporsay commented 1 year ago

Is there an existing issue for this?

[X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
[X] I have reviewed the documentation https://docs.sentry.io/
[X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using? If you use the CDN bundles, please specify the exact bundle (e.g. `bundle.tracing.min.js`) in your SDK setup.

@sentry/nextjs

SDK Version

7.37.2

Framework Version

13.1.1

Link to Sentry event

https://pulposapp.sentry.io/performance/pulpos-app-dev:d8283cfa1e9f4cd893e0e8cc26e59988/?project=4504339811074048&query=http.method%3AGET&referrer=performance-transaction-summary&statsPeriod=7d&transaction=GET+%2Fapi%2Ftrpc%2F%5Btrpc%5D&unselectedSeries=p100%28%29

SDK Setup

  Sentry.init({
    environment: env.NEXT_PUBLIC_APP_ENV,
    dsn: env.NEXT_PUBLIC_SENTRY_DSN,
    tracesSampleRate: 1.0,
    profilesSampleRate: 1.0,
    integrations: [
      new CaptureConsole({
        levels: ['log', 'warn', 'error', 'debug', 'assert'],
      }),
      new Tracing.Integrations.Prisma({ client: prisma }),
      new ProfilingIntegration(),
    ],
  });

Steps to Reproduce

For context, this is being run within an NX monorepo.

Steps:

Build production build: nx build app (application is called app)
Execute production build: nx run app:serve:production
Open the application in a page that has any tRPC API call with database use.

Expected Result

Profiling contains prisma database spans.

Actual Result

No database spans are present. However the interesting thing is that if I were to execute the application in development mode, the spans are reported correctly. This issue started happening when we upgraded the following packages:

"@sentry/nextjs": "^7.28.1" -> "^7.37.2",
"@sentry/profiling-node": "^0.0.12" -> "^7.37.2",
"@sentry/replay": "^7.28.1" -> "^7.37.2",
"@sentry/tracing": "^7.28.1" -> "^7.37.2",

AbhiPrasad commented 1 year ago

Hey @jporsay, thanks for writing in! What version of prisma are you using?

AbhiPrasad commented 1 year ago

execute the application in development mode

What is the difference between running the application in production/development mode? Do you bundle differently? Do you change prisma config in any way?

jporsay commented 1 year ago

We're using 4.8.0

jporsay commented 1 year ago

What is the difference between running the application in production/development mode? Do you bundle differently? Do you change prisma config in any way?

We do not change the prisma config. As for the bundling, there are no changes that we know of. Bundling is that provided by NX (15.3.0) and the NextJS version provided above.

lforst commented 1 year ago

I recently noticed something similar going on in a test app of mine. I investigated a bit but couldn't get to the root cause yet. What I found though is that manually instrumenting prisma in sentry.server.config.js seems to work:

import * as Sentry from '@sentry/nextjs';
import { getCurrentHub } from '@sentry/nextjs';
import { prismaClient } from './prisma/client';

prismaClient.$use(async (params, next) => {
  const scope = getCurrentHub().getScope();
  const parentSpan = scope?.getSpan();

  const action = params.action;
  const model = params.model;

  const span = parentSpan?.startChild({
    description: model ? `${model} ${action}` : action,
    op: 'db.sql.prisma',
  });

  // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
  const rv: unknown = await next(params);

  span?.finish();

  // eslint-disable-next-line @typescript-eslint/no-unsafe-return
  return rv;
});

Sentry.init({
  dsn: 'dsn',
  tracesSampleRate: 1.0,
  environment: process.env.VERCEL_ENV || 'development',
  includeLocalVariables: true,
});

jporsay commented 1 year ago

We've thought about adding manual instrumentation to prisma, but given that it was working without any issues before, we'd rather help surface this than going the "manual route".

joe-giunti-kiefa commented 1 year ago

Seeing the same problem with sentry 7.54.0, prisma4.14.1, and next 13.1.1. DB tracing works fine in dev mode but doesn't not produce traces in the production build.

lforst commented 1 year ago

Ok documenting my process to figure out why it is not working:

We previously identified that manually instrumenting the Prisma client inside sentry.server.config.ts works.
I placed a property on the client before passing it to the integration. This should let us identify the client.
Inside the working $use instrumentation I logged out the property. It read undefined so we can be sure that there are at least two different Prisma client instances floating around somehow.
I placed a log statement before my new PrismaClient() call to check if it was indeed called twice somehow? -> Yup.
The client was indeed initialized multiple times. Not initially, but when navigating around inside my test app - in particular when navigating from pages on the pages router to pages to the app router.
To investigate further I placed a console.trace() statement before the new PrismaClient() call.
The client seems to be initialized once for pages/_app, once in total for all actual "pages" inside pages/ and once in total for all pages inside app/. This seems to be in line with how server code is bundled in Next.js.
This behaviour is not caused by the Sentry SDK, but we need to fix our integration.

Brainstorming a fix:

Solution 1

Instead of an integration, or rather in addition to our integration, we export a Prisma middleware that can be passed to prismaClient.$use(). This way the client can be easily instrumented whenever it is created.

Solution 2

Instead of adding the Prisma middleware inside of setupOnce we could add it inside the constructor of our integration. We just have to make sure that we don't instrument a particular client multiple times - which we could achieve by setting a property. This is a bit awkward though because it will be completely pointless that this is inside an integration. I guess the upside is that we are fixing existing setups by doing this, our docs stay intact, and we keep the canonical way of having Integrations to instrument stuff.

joe-giunti-kiefa commented 1 year ago

@lforst this still doesn't seem to work for us even on the latest version of sentry and prisma and itt may be because we are using clientExtensions. We extend the client on each request to facilitate RLS and then put that into our TRPC context for use in our routers, so the extended client is not just available to pass a normal reference to the Prisma integration constructor because it is created using a function on each HTTP request. This is the expected way to implement this according to the prisma docs:

export const prisma =
  globalForPrisma.prisma ||
  new PrismaClient({
    log:
      env.NODE_ENV === "development" ? ["query", "error", "warn"] : ["error"],
    datasources: {
      db: {
        url: process.env.DATABASE_URL_APP,
      },
    },
  });

if (process.env.NODE_ENV !== "production") globalForPrisma.prisma = prisma;

export default prisma;

const restrictClientExtension = ...

export const wrapPrisma = (orgId: string, userId: string) => {
  return prisma
    .$extends(restrictClientExtension(orgId, userId))
    .$extends(executeRawRlsExtension(orgId, userId))
    .$extends(setTenantExtension(orgId));
};

and then in our server config file

import prisma from "~/server/prisma";

Sentry.init({
  dsn: SENTRY_DSN,
  environment: env,
  profilesSampleRate: env == "development" ? 1.0 : 0,
  integrations: [
    // Add profiling integration to list of integrations
    new ProfilingIntegration(),
    // this is the prisma export from the previous snippet. When setting up our TRPC context
    // we create the extended client using the wrapPrisma function above, and it only exists in that context
    new Sentry.Integrations.Prisma({ client: prisma }),
  ],
});

I think the only way this integration can really work is if the tracing happens within a client extension.

lforst commented 1 year ago

@joe-giunti-kiefa I think you are right. I am not really happy with our Prisma integration right now and I am planning to do a better version that is able to also capture queries and such. As far as I can remember client extensions are quite new so we just didn't get around to build support for them yet.

ThallesP commented 10 months ago

@joe-giunti-kiefa I think you are right. I am not really happy with our Prisma integration right now and I am planning to do a better version that is able to also capture queries and such. As far as I can remember client extensions are quite new so we just didn't get around to build support for them yet.

@lforst any update on this?

lforst commented 10 months ago

@ThallesP Yes I looked into it and it is currently not possible to have a proper tracer that uses the client extension as it breaks out of async context. Luckily we will soon have full OTEL support so this is gonna be solved by that.

SheaBelsky commented 9 months ago

@lforst Is there another issue I should follow for when this integration works again? Should I simply remove the Sentry.Integrations.Prisma integration from my codebase for now? Happy to stay subscribed to this issue if this is where progress will be posted to, but I am unable to see Prisma query spans anywhere in my performance logging (neither dev nor production.)

lforst commented 9 months ago

@SheaBelsky The prisma integration should work, however it won't capture the content of your queries. It should create spans though.

Can you please open an issue and provide reproduction steps for your problem. Thanks!

SheaBelsky commented 9 months ago

@SheaBelsky The prisma integration should work, however it won't capture the content of your queries. It should create spans though.

Can you please open an issue and provide reproduction steps for your problem. Thanks!

Yes I will open a separate issue! Thanks!

SheaBelsky commented 9 months ago

@lforst Opened a new one here: https://github.com/getsentry/sentry-javascript/issues/10680

getsentry / sentry-javascript