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

Sentry Express instrumenting whole app instead of only router

Open woss opened this issue 3 years ago • 2 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.6.0

Framework Version

7.6.0

Link to Sentry event

No response

Steps to Reproduce

I followed the Express integration and i have this code:

/* eslint-disable @rushstack/typedef-var */

import * as Sentry from '@sentry/node';
import * as Tracing from '@sentry/tracing';
import express, { Request, Response } from 'express'; //  "express": "4.18.1",
import { type Express } from 'express';

import { port } from './config';

/**
 * Express app
 * @internal
 */
export const app: Express = express();

const router = express.Router({
  caseSensitive: true
});
/// init the Sentry
Sentry.init({
  dsn: 'obv-dsn',
  enabled: true,
  environment: 'envvvvv',
  integrations: [
    new Sentry.Integrations.Http({ tracing: true }),
    // enable Express.js middleware tracing
    new Tracing.Integrations.Express({ app, methods: ['get'], router })
  ],
  tracesSampleRate: 1.0
});
app.use(Sentry.Handlers.requestHandler());
app.use(Sentry.Handlers.tracingHandler());

router.get('/my-router-route/*', async (req: Request, res: Response) => {
  res.send('ok');
});

app.use(router);

/**
 * Home route
 */
app.get('/', (req: Request, res: Response) => {
  res.send('home');
});

/**
 * Health-check route
 */
app.get('/healthcheck', (req: Request, res: Response) => {
  res.send('OK');
});

app.post('/po', (req: Request, res: Response) => {
  res.send('OK');
});


// eslint-disable-next-line @typescript-eslint/naming-convention
app.get('*', function (req: Request, res: Response) {
  res.status(404).end();
});

// The error handler must be before any other error middleware and after all controllers
app.use(Sentry.Handlers.errorHandler());

app.listen(port);

Expected Result

only router is instrumented, and get requests in the router only. Not the whole app

Actual Result

You can see here that all routes are instrumented. I have tried to set the

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

from the app but it doesn't work.

image

woss avatar Jul 18 '22 18:07 woss

Hi @woss, thanks for writing in.

To me, it seems like app.use(Sentry.Handlers.tracingHandler()) creates transactions for all incoming requests on app. The Express integration only adds additional spans to the requests. You wrote, you already tried router.use(Sentry.Handlers.tracingHandler()) but it didn't work. Can you tell me what exactly didn't work? Did you not get transactions at all or still all transactions (i.e. also the ones you don't want)?

Coincidentally we're currently looking at improving some aspects of Express transactions. Might be interesting to find out what's going on here.

cc-ing @lobsterkatie for additional input

Lms24 avatar Jul 20 '22 09:07 Lms24

Hi @Lms24, thanks for the response.

Yes, by it didn't work i mean, there was no difference, i got the same instrumentation as i would use app.

If i can be of any help let me know. :)

woss avatar Jul 20 '22 10:07 woss

Closing this issue for cleanup - please re-open if it still applies. Thanks!

AbhiPrasad avatar Jul 27 '23 16:07 AbhiPrasad