sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

Strapi performance monitoring broken in strapi v4?

Open DawnMD opened this issue 3 years ago • 6 comments

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 package are you using?

@sentry/node

SDK Version

7.1.2

Framework Version

Strapi 4.3.4

Link to Sentry event

No response

Steps to Reproduce

  1. Self host strapi@latest
  2. Add official @strapi/sentry plugin
  3. Add @sentry/node and @sentry/tracing latest
  4. Add a sentry middleware at src/middlewares/sentry.js
const Sentry = require("@sentry/node");
const {
  extractTraceparentData,
  stripUrlQueryAndFragment,
} = require("@sentry/tracing");

//https://docs.sentry.io/platforms/node/guides/koa/
Sentry.init({
  dsn: process.env.DSN,
  environment: strapi.config.environment,
  integrations: [
    // enable HTTP calls tracing
    new Sentry.Integrations.Http({ tracing: true }),
  ],
  tracesSampleRate: 1,
});

const tracingMiddleWare = async (ctx) => {
  const reqMethod = (ctx.method || "").toUpperCase();
  const reqUrl = ctx.url && stripUrlQueryAndFragment(ctx.url);

  // connect to trace of upstream app
  let traceparentData;
  if (ctx.request.get("sentry-trace")) {
    traceparentData = extractTraceparentData(ctx.request.get("sentry-trace"));
  }

  const transaction = Sentry.startTransaction({
    name: `${reqMethod} ${reqUrl}`,
    op: "http.server",
    ...traceparentData,
  });

  ctx.__sentry_transaction = transaction;

  Sentry.getCurrentHub().configureScope((scope) => {
    scope.setSpan(transaction);
  });

  ctx.res.on("finish", () => {
    setImmediate(() => {
      if (ctx._matchedRoute) {
        const mountPath = ctx.mountPath || "";
        transaction.setName(`${reqMethod} ${mountPath}${ctx._matchedRoute}`);
      }
      transaction.setHttpStatus(ctx.status);
      transaction.finish();
    });
  });
};

module.exports = (config, { strapi }) => {
  strapi.server.use(async (context, next) => {
    try {
      await tracingMiddleWare(context, next);
    } catch (error) {
      Sentry.captureException(error);
      throw error;
    }
  });
};
  1. Initialise @strapi/sentry at config/middlewares.js as global::sentry
  2. Add sentry plugin to config/plugins.js as
 sentry: {
    enabled: true,
    config: {
      dsn: env("DSN"),
      sendMetadata: true,
    },
  },

Expected Result

Sentry should trace the transactions made by the API calls during navigation strapi or doing graphql queries. The exact thing worked for strapi v3 but after upgrading to v4 it broke and in the context request there are no headers for sentry-tracing

Actual Result

This was from strapi v3 when it was working fine image

DawnMD avatar Sep 09 '22 18:09 DawnMD

Hi, @DawnMD.

Thanks for reporting! To clarify:

after upgrading to v4 it broke and in the context request there are no headers for sentry-tracing

Is the lack of headers the way in which it broke, or do you mean that it broke in some other way and also headers are missing?

In any case, we appreciate you letting us know, and as you suggest, it's quite possible that a change in strapi between versions 3 and 4 is causing this. I've put it on our backlog to investigate. In the meantime, community investigation or PRs welcome!

lobsterkatie avatar Sep 13 '22 03:09 lobsterkatie

Hi @lobsterkatie The way this function is executed by taking the sentry-trace header and using it to transact, quite possibly the strapi structure also changed and it ultimately broke it. I'll further investigate and compare V3 and v4 context and will update here. Thank you

DawnMD avatar Sep 13 '22 04:09 DawnMD

Greetings @DawnMD, Is there any news on this issue?

Boghdady avatar Sep 20 '22 20:09 Boghdady

Hey @Boghdady Still waiting for the sentry team to get back. I checked the strapi V3 and it was working as expected. It seems the issue with how the context object is handled for strapi V4 only

DawnMD avatar Sep 21 '22 03:09 DawnMD

Still waiting for the sentry team to get back.

FYI, this is on our backlog but hasn't yet been roadmapped, so I can't say for sure how soon we're going to pick it up. (Just don't want to set unrealistic expectations.)

That said, can you say a little more about "the issue with how the context object is handled for strapi V4 only?" What specifically are you seeing?

lobsterkatie avatar Sep 21 '22 05:09 lobsterkatie

Hi @lobsterkatie So I checked every possible configuration for tracing the transactions and it seems like when @strapi/plugin-sentry is enabled, the trace just doesn't work. When strapi plugin is disabled it is working as expected i.e it's tracing all the transactions from middlewares.

PS: I downgraded the @sentry/node and @sentry/tracing to 6.19.0. With both to the latest i.e 7.13.0 and with/without @strapi/plugin-sentry I was getting image

DawnMD avatar Sep 21 '22 09:09 DawnMD

Any chance there has been some movement on this issue?

garrettfritz avatar Nov 08 '22 15:11 garrettfritz

It works for me on latest Strapi 4, "@sentry/node": "^6.19.0", "@sentry/tracing": "^6.19.0" and without @strapi/plugin-sentry installed.

I've created global middleware:

import * as Sentry from "@sentry/node";
import {
  extractTraceparentData,
  stripUrlQueryAndFragment,
} from "@sentry/tracing";

Sentry.init({
  dsn: process.env.SENTRY_DSN,
  environment: strapi.config.environment,
  integrations: [
    // enable HTTP calls tracing
    new Sentry.Integrations.Http({ tracing: true }),
  ],
  tracesSampleRate: +process.env.SENTRY_SAMPLE_RATE,
});

const tracingMiddleWare = async (ctx, next) => {
  const reqMethod = (ctx.method || "").toUpperCase();
  const reqUrl = ctx.url && stripUrlQueryAndFragment(ctx.url);

  // connect to trace of upstream app
  let traceparentData;
  if (ctx.request.get("sentry-trace")) {
    traceparentData = extractTraceparentData(ctx.request.get("sentry-trace"));
  }

  const transaction = Sentry.startTransaction({
    name: `${reqMethod} ${reqUrl}`,
    op: "http.server",
    ...traceparentData,
  });

  ctx.__sentry_transaction = transaction;

  Sentry.getCurrentHub().configureScope((scope) => {
    scope.setSpan(transaction);
  });

  ctx.res.on("finish", () => {
    setImmediate(() => {
      if (ctx._matchedRoute) {
        const mountPath = ctx.mountPath || "";
        transaction.setName(`${reqMethod} ${mountPath}${ctx._matchedRoute}`);
      }
      transaction.setHttpStatus(ctx.status);
      transaction.finish();
    });
  });

  await next();
};

module.exports = (config, { strapi }) => {
  return async (context, next) => {
    try {
      await tracingMiddleWare(context, next);
    } catch (error) {
      Sentry.captureException(error);
      throw error;
    }
  };
};

Both tracing and error reporting are working fine.

akasummer avatar Nov 09 '22 14:11 akasummer

@akasummer yes without strapi plugin it works flawlessly. The issue is something to do with the strapi plugin I'm guessing and also there is no workaround as of now.

DawnMD avatar Nov 09 '22 15:11 DawnMD

@DawnMD I think @strapi/plugin-sentry doesn't even have any implementation for tracing (performance monitoring), only error reporting. I believe this issue should be raised with Strapi team, not Sentry.

akasummer avatar Nov 09 '22 15:11 akasummer