Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strapi performance monitoring broken in strapi v4? #5716

Closed
3 tasks done
DawnMD opened this issue Sep 9, 2022 · 10 comments
Closed
3 tasks done

Strapi performance monitoring broken in strapi v4? #5716

DawnMD opened this issue Sep 9, 2022 · 10 comments

Comments

@DawnMD
Copy link

DawnMD commented Sep 9, 2022

Is there an existing issue for this?

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

@lobsterkatie
Copy link
Member

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 lobsterkatie changed the title Strapi performance monitoring Strapi performance monitoring broken in strapi v4? Sep 13, 2022
@DawnMD
Copy link
Author

DawnMD commented Sep 13, 2022

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

@Boghdady
Copy link

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

@DawnMD
Copy link
Author

DawnMD commented Sep 21, 2022

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

@lobsterkatie
Copy link
Member

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?

@DawnMD
Copy link
Author

DawnMD commented Sep 21, 2022

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

@garrettfritz
Copy link

Any chance there has been some movement on this issue?

@akasummer
Copy link

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.

@DawnMD
Copy link
Author

DawnMD commented Nov 9, 2022

@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.

@akasummer
Copy link

@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.

@HazAT HazAT closed this as completed Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants