Skip to content

Sentry Express instrumenting whole app instead of only router #5433

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

Closed
3 tasks done
Tracked by #5510
woss opened this issue Jul 18, 2022 · 3 comments
Closed
3 tasks done
Tracked by #5510

Sentry Express instrumenting whole app instead of only router #5433

woss opened this issue Jul 18, 2022 · 3 comments

Comments

@woss
Copy link

woss commented Jul 18, 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.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

@Lms24
Copy link
Member

Lms24 commented Jul 20, 2022

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

@woss
Copy link
Author

woss commented Jul 20, 2022

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. :)

@AbhiPrasad
Copy link
Member

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

@AbhiPrasad AbhiPrasad closed this as not planned Won't fix, can't repro, duplicate, stale Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants