[7.108.0] Sentry.Handlers.tracingHandler uses incorret route name in Express server

[lhermann]

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?

@sentry/node

SDK Version

7.108.0

Framework Version

Node 18, Express 4.19.2

Link to Sentry event

• Transaction when it was still working: https://lhdev.sentry.io/performance/summary/?environment=PROD&project=5664623&query=http.method%3AGET&referrer=performance-transaction-summary&statsPeriod=90d&transaction=GET+%2Frooms%2F%3AroomPid%28%5BA-Z0-9%5D%7B8%7D%29&unselectedSeries=p100%28%29&unselectedSeries=avg%28%29
• Transaction for the same route after it broke: https://lhdev.sentry.io/performance/summary/?environment=PROD&project=5664623&query=http.method%3AGET&referrer=performance-transaction-summary&statsPeriod=90d&transaction=GET+%2Frooms%2FA6SW5R48&unselectedSeries=p100%28%29&unselectedSeries=avg%28%29

SDK Setup

import * as Sentry from '@sentry/node'
import { nodeProfilingIntegration } from '@sentry/profiling-node'
import express from 'express'

const app = express()

Sentry.init({
  dsn: process.env.SENTRY_DSN,
  enabled: Boolean(process.env.SENTRY_DSN),
  release: process.env.npm_package_version,
  environment: process.env.SENTRY_ENV,
  serverName: process.env.SERVER_ID,
  integrations: [
    new Sentry.Integrations.Express({ app }),
    nodeProfilingIntegration(),
  ],
  sampleRate: 1,
  tracesSampler: (samplingContext) => {
    console.log('[Sentry]', { samplingContext: samplingContext.transactionContext.name })
    return 0.05
  },
  profilesSampleRate: 0.2,
})

const router = express.Router()

router.use(Sentry.Handlers.requestHandler())
router.use(Sentry.Handlers.tracingHandler())

router.get('/test/:first/:second', (req, res) => {
  console.log('[Test Route]', { 'req.route.path': req.route.path })
  console.log('[Test Route]', { 'req.originalUrl': req.originalUrl })
  res.sendStatus(200)
})

router.use(Sentry.Handlers.errorHandler())

app.use(router)

app.listen(process.env.PORT, () => {
  console.log(`Example app listening on port ${process.env.PORT}`)
})

Steps to Reproduce

1. Start the express test server from the code above node -r dotenv/config index.js.
2. Trigger a GET request to the example route GET http://127.0.0.1:3000/test/hello/world.
3. Observe the console output.

Expected Result

[Sentry] { samplingContext: 'GET /test/:first/:second' }
[Test Route] { 'req.route.path': '/test/:first/:second' }
[Test Route] { 'req.originalUrl': '/test/hello/world' }

I expect the Sentry Trace to use the route name /test/:first/:second.

Actual Result

[Sentry] { samplingContext: 'GET /test/hello/world' }
[Test Route] { 'req.route.path': '/test/:first/:second' }
[Test Route] { 'req.originalUrl': '/test/hello/world' }

The Sentry Trace uses the originalUrl /test/hello/world instead of the router path name. As an effect, the performance dashboard is nearly useless, because all requests are filed under different names.

Observations:

• extractPathForTransaction is called twice (https://github.com/getsentry/sentry-javascript/blob/7.108.0/packages/tracing-internal/src/node/integrations/express.ts), first for the sentryTracingMiddleware without the req.route being undefined, second for the instrumentRouter function with the req.route being correct.

• extractPathForTransaction -> https://github.com/getsentry/sentry-javascript/blob/7.108.0/packages/utils/src/requestdata.ts#L107

• sentryTracingMiddleware -> https://github.com/getsentry/sentry-javascript/blob/7.108.0/packages/node/src/handlers.ts#L45

• instrumentRouter -> https://github.com/getsentry/sentry-javascript/blob/7.108.0/packages/tracing-internal/src/node/integrations/express.ts#L278

[mydea]

Hello, in the upcoming v8 version we use a new instrumentation base, where this may (or may not, hard to say) be fixed already. Could you give it a try with 8.0.0-alpha.7, just to check if this is also the case there? If you don't want to/cannot try the alpha, we can wait until a more stable release to test this then!

[lhermann]

I'm using "type": "module" with import * as Sentry from '@sentry/node' which seems not supported in v8.

[souterjk]

Hey @mydea we've got the same problem on basically the same setup (express, node, version 7 of Sentry, etc.). I tried to move over to version 8 alpha so I could tell you if its working or not, but I can't even figure out how to set v8 up for node. Can you help me with the v8 equivalent of this setup block? Almost all of the integration paths are no longer importable, and I was able to figure most of that part out by moving the paths around, but the middlewares for app.use seem completely gone in v8?

Also does the express integration no longer require the app (the express router) parameter in the new version?

Sentry.init({
    //a bunch of other irrelevant stuff...
    integrations: app
      ? [
          new Sentry.Integrations.Http({ tracing: true }),
          new Sentry.Integrations.Express({ app }),
          new Sentry.Integrations.Postgres(),
          nodeProfilingIntegration(),
          ...(anrThreshold
            ? [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold })]
            : []),
        ]
      : [
          new Sentry.Integrations.Http({ tracing: true }),
          new Sentry.Integrations.Postgres(),
          nodeProfilingIntegration(),
        ],
  });
  if (app) {
    // The request handler must be the first middleware on the app
    app.use(Sentry.Handlers.requestHandler());
    app.use(Sentry.Handlers.tracingHandler());
  }

Also the link here for how to set this up is completely dead 😢

[souterjk]

Circling back a few hours later, the doc I needed was here . This is indeed fixed in the v8 alpha. Is there any chance that whatever fixed the transaction name grouping can get ported back into v7? Unless v8 is close to a full release, this is a pretty big breakage in the most recent v7.

[mydea]

Hey, sadly the code for this is completely different in v8 than it was in v7. So it is hard to backport this, I fear. However, a full release of v8 shouldn't be too far off - we are planning to release a beta in the next week or so!

[lhermann]

FYI: Issue is not fixed with @sentry/node v7.110.1.

[mydea]

We have just released v8.0.0-beta.1 - could you give that a try and let us now if it works for you as expected on that version?

[lhermann]

@mydea I can confirm that route names are correctly identified in @sentry/node@8.

I'm using "type": "module" and am starting the test application with node -r dotenv/config --import ./instrument.js index.js.

instrument.js

import * as Sentry from '@sentry/node'

Sentry.init({
  dsn: process.env.SENTRY_DSN,
  enabled: Boolean(process.env.SENTRY_DSN),
  release: process.env.npm_package_version,
  environment: process.env.SENTRY_ENV,
  serverName: process.env.SERVER_ID,
  sampleRate: 1,
  tracesSampler: (samplingContext) => {
    console.log('[Sentry] transactionContext', samplingContext.transactionContext)
    return 1
  },
})

The log messages, while showing much more activity than v7, seems to identify the route name correctly:

Example app listening on port 3000
[Sentry] transactionContext { name: 'GET', parentSampled: undefined }
[Sentry] transactionContext { name: 'middleware - query', parentSampled: true }
[Sentry] transactionContext { name: 'middleware - expressInit', parentSampled: true }
[Sentry] transactionContext { name: 'router - /', parentSampled: true }
[Sentry] transactionContext { name: 'request handler - /test/:first/:second', parentSampled: true }
[Test Route] { 'req.route.path': '/test/:first/:second' }
[Test Route] { 'req.originalUrl': '/test/foo/bar/' }

However, on my production app I run into the "SDK fails in ESM mode in combination with date-fns" error and had to disable the instrumentation 😢: https://github.com/getsentry/sentry-javascript/issues/12154

[mydea]

Thank you for the feedback! It's good to hear that this is generally resolved. We are working on resolving the date-fns issue as soon as possible 😬

[AbhiPrasad]

We've fixed the date-fns issues with https://github.com/getsentry/sentry-javascript/releases/tag/8.8.0 - please give it a try!