getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.98k stars 1.57k forks source link

Wrong transaction name when multiple middlewares are using same variable in Express app #11018

Closed Mordred closed 4 months ago

Mordred commented 7 months ago

Is there an existing issue for this?

How do you use Sentry?

Self-hosted/on-premise

Which SDK are you using?

@sentry/node

SDK Version

7.106.0

Framework Version

express@4.18.2

Link to Sentry event

No response

SDK Setup

Sentry.init({
  dsn: __DSN__,
  tracesSampleRate: 1.0,
  integrations: [
    new Sentry.Integrations.Express({ app }),
  ]
})

Steps to Reproduce

I have two middlewares with same variable but the second is more strict, because I need to do something for some languages. (https://replit.com/@jantosko/Sentry-Express-Transaction-Name#index.js)

const express = require('express');
const Sentry = require('@sentry/node');

const app = express();

Sentry.init({
  dsn: 'https://test@sentry.com/1',
  tracesSampleRate: 1.0,
  integrations: [
    new Sentry.Integrations.Express({ app }),
  ]
})

app.use(Sentry.Handlers.requestHandler());
app.use(Sentry.Handlers.tracingHandler());

app.use('/:language/', (req, res, next) => {
  console.log(res.__sentry_transaction?.name)
  next();
});

app.use('/:language(sk|en)/', (req, res, next) => {
  console.log(res.__sentry_transaction?.name)
  next();
});

app.get('/:language/hello/', (req, res) => {
  console.log(res.__sentry_transaction?.name)
  res.send('Hello Express app!')
})

app.listen(3000, () => {
  console.log('server started');
})

When you hit /sk/hello/ the console output is:

GET /sk/hello/
GET /:language/:language

Expected Result

I expected that the transaction name will be GET /:language/hello/

Actual Result

Transaction name is GET /:language/:language :(

Lms24 commented 7 months ago

Hey @Mordred thanks for writing in!

My hunch is that this is caused by our express router instrumentation that does a bunch of shady prototype patching to correctly parameterize a route name in time. The good news is that we're getting rid of all of this in favour of OpenTelemetry instrumentation. My hope is that their parameterization approach is much better than ours and problems like this one are going to be a thing of the past. This new instrumentation will be added in version 8 of our SDKs which we're working on right now.

We already shipped an alpha version: @sentry/node@8.0.0-alpha.2. If you're brave, feel free to give it a try ;) I'm curious if this fixes your issue.

Lms24 commented 7 months ago

Btw, https://github.com/getsentry/sentry-javascript/issues/11023 sums up which frameworks and libraries will be instrumented automatically (via OpenTelemetry) in v8.

Mordred commented 7 months ago

OK, I will wait for at least beta release :)

mydea commented 5 months ago

We released v8.0.0, could you give it a try and let us know if it works as expected for you?

getsantry[bot] commented 5 months ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

indranuntd commented 2 months ago

@mydea the issue is happening with v8 also. Tried with version 8.27.0.

mydea commented 2 months ago

@mydea the issue is happening with v8 also. Tried with version 8.27.0.

Do you mind creating a new issue with details of your (v8) setup and the problem you are seeing? That will make it easier for us to debug the problem - thank you!