Node/Nest: Can't make the startSpan to create a child span instead of a transaction

[ericjeker]

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.99

Framework Version

Nest.js 10

Link to Sentry event

No response

SDK Setup

{
  dsn: process.env.SENTRY_DNS,
  // Transaction with profiling cost 1.3 instead of 1.0,
  // you can add more profiling here for example Prisma or postgresql
  integrations: [
    // enable HTTP calls tracing
    new Sentry.Integrations.Http({ tracing: true }),
  ],
  // Performance Monitoring
  tracesSampleRate: process.env.NODE_ENV === 'production' ? 0.1 : 1.0,
  // Set sampling rate for profiling - this is relative to tracesSampleRate
  profilesSampleRate: 1.0,

  // Set the environment & release version
  environment: process.env.NODE_ENV,
  release: `${process.env.npm_package_name}@${process.env.npm_package_version}`,

  // Disable transport in development, transaction are still captured in debug mode
  enabled: process.env.NODE_ENV !== 'development',
  enableTracing: process.env.NODE_ENV !== 'development',

  // Enable debug mode to log event submission
  debug: process.env.NODE_ENV === 'development',

  // Called for message and error events
  beforeSend(event) {
    // Modify or drop the event here
    // process.env.NODE_ENV === 'development' && console.log(event);
    return event;
  },

  // Called for transaction events, you can further debug your transactions here
  beforeSendTransaction(event) {
    // Modify or drop the event here
    // process.env.NODE_ENV === 'development' && console.log(event);
    return event;
  },
}

Steps to Reproduce

1. Pull https://github.com/ericjeker/nestjs-sentry-example/tree/feature/update-sentry-799
2. Setup the right SENTRY_DNS in .env
3. Start the API with npm run start:dev
4. Use CURL to GET /
5. There will be one http.server span for the handler, started in SentryInterceptor, but no child span.
6. There will be one separate span (transaction) for AppController.getHello()

Expected Result

The child span that is started in the handler (AppController.getHello()) should be a child span, not a separate transaction.

Actual Result

The child span is a separate transaction.

[mydea]

Hey,

I've just tried to reproduce it, and it appears correct to me:

could you share a link to a trace where this is incorrect, maybe?

[ericjeker]

Are you sure you downloaded the branch (feature/update-sentry-799) I linked and not the main branch? The main still use the deprecated version of @sentry/node, while the feature branch use the startSpan.

[mydea]

Ah, you're right of course, sorry 😬

So the problem there most likely is the way that this works internally:

return Sentry.startSpanManual(
      {
        op: 'http.server',
        name: `${method} ${url}`,
      },
      (span) => {
        return next.handle().pipe(
          tap(() => {
            span.end();
          }),
        );
      },
    );

Basically, startSpan and startSpanManual work by making the active span be set for the duration of the callback. I would guess the problem is that next.handle() somehow runs the inner stuff outside of the callback (maybe async, or something like this?), leading to the active span not being available in there 🤔 I don't know enough about how this works internally in Nest.js, but I strongly suspect that's the problem.

Overall, I think it's not worth it for you to rewrite this, because in the upcoming v8 we will have auto-instrumentation for nest.js - so you will not have to do that anymore, but should get your routes automatically instrumented. We'll do this by actually wrapping nest.js itself which will allow us to make this work properly.

I've made a branch with this where this is working: https://github.com/mydea/nestjs-sentry-example/tree/fn/nest-node-experimental - in v8 this will become the standard. This branch has an output like this:

I'll keep this issue open until we have a first proper release of v8. You can use node-experimental right now if you want (it is the same as sentry node except for the swapped out performance implementation under the hood), or wait for v8 which should have a first release in the next month or so.

[ericjeker]

That's great, can't wait for V8 then!

I will test some implementations we have with that experimental branch, also with Fastify and DB integrations (MikroORM) and maybe create some more test branches to cover these different cases.

[mydea]

That's great, can't wait for V8 then!

I will test some implementations we have with that experimental branch, also with Fastify and DB integrations (MikroORM) and maybe create some more test branches to cover these different cases.

great, please come back with any feedback you have, we're excited to hear it! Fastify should also be auto-instrumented out of the box. If you find something not being auto instrumented well, let us know, too!

[HialeahLiam]

Hey @mydea! I tried out implementing Sentry using the experimental SDK just like you showed in your fn/nest-node-experimental branch here:

I've made a branch with this where this is working: https://github.com/mydea/nestjs-sentry-example/tree/fn/nest-node-experimental - in v8 this will become the standard. This branch has an output like this:

However, tracing isn't working and all the transactions are dropped from sampling:

The only difference between your fork and my code is that I'm using Prisma, and interestingly enough, only the prisma client connection gets connected as a transaction:

Also notice how it says Inheriting remote parent's sampled decision for GET: false. Could this be part of the issue? What does the log mean by "remote parent" ?

[mydea]

remote parent means that it is picking up a trace header that contains a negative sampling decision, thus also dropping transaction here. Can this be the case in your example - so something is calling your endpoint with an ongoing trace?

[getsantry[bot]]

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 🥀