Sentry no longer uses route parameter names for transactions

[apmorton]

Package + Version

• @sentry/browser
• @sentry/node
• raven-js
• raven-node (raven for node)
• other:

Version:

5.25.0

Description

Since #2714 sentry no longer uses req.route.path for transaction names, which causes route paramter values to be logged rather than route parameter names

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

const app = express();
Sentry.init({ dsn: 'secret' });

const subRouter = express.Router();

subRouter.get('/throw-new-error/:example', (req, res) => {
  throw new Error('An ' + req.params.example + ' error');
});

app.use(Sentry.Handlers.requestHandler());
app.use('/sub', subRouter);
app.use(Sentry.Handlers.errorHandler());

app.listen(3000, function () {
  console.log('Example app listening on port 3000!');
});

wget http://localhost:3000/sub/throw-new-error/my-error-type

Transaction as reported using @sentry/node@5.18.1: GET|/throw-new-error/:example
Transaction as reported using @sentry/node@5.19.0 (or higher): GET /sub/throw-new-error/my-error-type

[apmorton]

Not entirely related in the end, but since this is how we came to notice the issue - I will drop this here as well.

sentry-javascript/packages/node/src/handlers.ts

Lines 38 to 39 in 7c14283

	// TODO: At this point `req.route.path` (which we use in `extractTransaction`) is not available
	// but `req.path` or `req.url` should do the job as well. We could unify this here.

It seems the performance tracing has never used req.route.path. I think this would be a good feature to have.

As it stands, the performance tracing treats every url parameter value as a unique transaction:

(this list continues on for quite a while).
It would be nice for us to have a single POST /api/compiler/:compiler/compile transaction.

Perhaps the solution to "unify" things as described in the comment in the source above is to call extractTransaction here:

sentry-javascript/packages/node/src/handlers.ts

Lines 65 to 66 in 7c14283

	res.once('finish', () => {
	transaction.setHttpStatus(res.statusCode);

Like:

transaction.setName(extractTransaction(req));

(and the needed changes to pass an options object to the tracingHandler function)

[rodolfoBee]

As a workaround, you can change the transaction name before sending it to Sentry by changing event.transaction in a GlobalEventProcessor.

Sentry.addGlobalEventProcessor( (event) => {
  if(event.type === "transaction"){
    event.transaction = "my trasaction name"
  }
  return event;
}

This is also mentioned under Grouping Transactions

[lobsterkatie]

@apmorton - this should solve your issue: #3048.

We will do a release either later this week or early next.

I'm going to close this, but if that doesn't solve your problem once it's released, please feel free to bump this thread.

[BerndSchrooten]

I don't think this was solved in the last version after #3048 got merged. For us it still logs the full URL in version 5.27.4, putting a log in addExpressReqToTransaction which was introduced in PR #3048 shows req.route as undefined.
I believe this is because it is only set once the route is handled and this function is called before a Router handles the request. As suggested by @apmorton it might be better to unify it in handler.ts

On errors, the transaction is logged with the route path as the transaction name because that middleware comes after the router.

[kamilogorek]

@BerndSchrooten will be fixed in #3064
Thanks for pointing that out.