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

Transactions aren't capturing async child spans

Open jpike88 opened this issue 3 years ago • 7 comments

  • [x] Review the documentation: https://docs.sentry.io/
  • [x] Search for existing issues: https://github.com/getsentry/sentry-javascript/issues
  • [x] Use the latest release: https://github.com/getsentry/sentry-javascript/releases
  • [x] Provide a link to the affected event from your Sentry account

Package + Version

  • [x] @sentry/node

Version:

6.5.1

Description

const transaction = Sentry.startTransaction({
					op: encodedApiFunctionToRealName[finalPublicMethodName],
					name: encodedApiFunctionToRealName[finalPublicMethodName],
				});

				try {
					const result = await auth(accessToken);
transaction.finish();
}
})

In the above code, many mysql functions are running, but none of the spans are showing up in the log. I breakpointed inside @sentry/tracing/dist/integrations/mysql.js, and the problem seems to be this code is failing:

const parentSpan = scope?.getSpan();

getSpan() returns undefined.

The transaction logic doesn't seem to be very 'smart', it's not propagating the scope properly via the stack.

This is a big problem for our business, we want to switch to your product from Elastic APM but this makes that untenable.

jpike88 avatar Jun 09 '21 09:06 jpike88

The elastic-apm-node library may provide some insight on how to redesign things better

jpike88 avatar Jun 09 '21 10:06 jpike88

You might need to do

        Sentry.configureScope((scope) => {

            scope.setSpan(transaction);
        });

AdriVanHoudt avatar Jun 09 '21 12:06 AdriVanHoudt

I'll try within the next hour. Why isn't that done by default?

jpike88 avatar Jun 09 '21 12:06 jpike88

No clue 🤷 , it is in the example on https://docs.sentry.io/platforms/node/performance/

AdriVanHoudt avatar Jun 09 '21 12:06 AdriVanHoudt

wtf it actually worked. but this to me still seems like bizarre/unexpected behaviour and warrants further explanation or maybe a fix

jpike88 avatar Jun 09 '21 12:06 jpike88

My guess would be that you need to do this for your 'root' transaction but startTransaction doesn't know if it is the 'root' one so it can't do it automatically. But no clue if that is actually the reason :D Glad it works now for you.

Since you're switching you might run into https://github.com/getsentry/sentry-javascript/issues/3660 as well btw.

AdriVanHoudt avatar Jun 09 '21 13:06 AdriVanHoudt

Yeah weird. With elastic-apm-node I just had an init call, and a simple one liner in the block of interest, no need to finish it or call any weird methods:

apm.setTransactionName('blah');

I hope this lib can learn from the others and implement a simpler approach.

jpike88 avatar Jun 09 '21 14:06 jpike88

it actually worked.

we are aware of this issue, and at least for this specific topic it appears a fix was possible so I will close the issue and we address overarching topic internally

smeubank avatar Jan 12 '23 12:01 smeubank