Skip to content
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

Transaction starts after beforeEach is resolved when vueRouterInstrumentation is present #4072

Closed
5 tasks done
laygir opened this issue Oct 17, 2021 · 9 comments
Closed
5 tasks done

Comments

@laygir
Copy link

laygir commented Oct 17, 2021

Package + Version

  • @sentry/vue: 6.13.3
  • @sentry/tracing: 6.13.3

Version:

@sentry/vue: 6.13.3
@sentry/tracing: 6.13.3

Description

I have an XHR request on the beforeEach hook of Vue Router.

When vueRouterInstrumentation is enabled pageload or navigation transaction starts after beforeEach resolved, causing the the XHR requests in beforeEach to NOT have sentry-trace header.

When vueRouterInstrumentation is disabled, then pageload transaction starts before beforeEach and the request has the sentry-trace header present as expected.

But then when navigated to a new page, navigation transaction again starts after beforeEach causing requests to not be included in the transaction..

So it seems like using vueRouterInstrumentation takes some things away while adding some other things..

Is this the intentional/expected behaviour, I'm not sure and if so, what would you suggest to solve this?
For example wouldn't it be possible to start the navigation transaction on the beforeEach hook?

Eventually my goal is to have all my api calls traced. I've to say I'm a bit surpised that automatic instrumentation does not pick up individual xhr requests unless there is an active transaction.

Here's sandbox
https://codesandbox.io/s/sentry-tracing-beforeeach-7j8g6?file=/src/main.js

Thanks!


Update:

Noticed here transaction is already being started on beforeEach hook.
https://github.com/getsentry/sentry-javascript/blob/master/packages/vue/src/router.ts#L36

Is this the result of a merge strategy behaviour?

@laygir
Copy link
Author

laygir commented Feb 22, 2022

Hello hello!
It's been 5-6 months and no progress here. Any plans on addressing this one?

@ydennisy
Copy link

one more for this one! any updates here - at least a workaround would be nice!

@ydennisy
Copy link

ydennisy commented Aug 19, 2022

It has been one nearly one year and no response here... pretty poor for a paid product.

Lets convert this into a what are alternatives to sentry thread?

@laygir
Copy link
Author

laygir commented Aug 24, 2022

@ydennisy indeed, it's a shame. I have 3 written assurances that this problem was going to be fixed in "a few weeks" but we are approaching the anniversary of it..

I'm not gonna lie, I love Sentry, have been using it for 5 years in multiple projects and it helped me a lot to discover issues.
But this behaviour of theirs is the one I hate the most.

If you are not going to fix it, don't pretend that you will and make me wait for 10 months and do multiple follow ups.

@Lms24
Copy link
Member

Lms24 commented Nov 17, 2022

Hi @ydennisy and @laygir apologies I didn't come across this issue when I was working on improving a few things around the Vue instrumentations.

When vueRouterInstrumentation is enabled pageload or navigation transaction starts after beforeEach resolved, causing the the XHR requests in beforeEach to NOT have sentry-trace header.

For pageload transactions, this should have changed, as we now start this transaction earlier (basically on initialization of the SDK rather than on the beforeEach call, which more accurately represents a pageload overall). See #5983.

We released this change in 7.16.0. Would you mind giving the SDK a try with a version >= 7.16.0?

But then when navigated to a new page, navigation transaction again starts after beforeEach causing requests to not be included in the transaction..

I'm curious if this still applies, as we're starting navigation transactions in our beforeEach handler. Are you suggesting to start this even earlier? Tbh, I'm not a Vue expert at all so happy to hear some suggestions on how we can improve here.

In other news, we're also experimenting with #5750 (first PR was opened up a few days ago). We don't have a particular timeline yet but it might be worth trying this out when it's ready.

Hope this helps a little and again, sorry for the delays.

@LeoMelody
Copy link

I change in 7.20.0 and resolved。But the LCP and CLS data is disappeared。I don't know if it is related to this issue: #4683

@AbhiPrasad
Copy link
Member

But the LCP and CLS data is disappeared

We leverage the web vitals library to report LCP and CLS info so there are a couple of things that might be happening here.

  1. LCP/CLS is happening after your pageload is finished. We recommend increasing your idleTimeout to catch this possibility.
  2. We were reporting intermediate LCP/CLS values erroneously. As per the docs, it is recommended only the final LCP value is captured.
  3. The browser itself did not report an LCP/CLS value that we can use. If the web-vitals library does not record an LCP/CLS value, we cannot report it.

Generally we can only report values that are occuring during a running transaction (pageload/navigation), so if the metric triggers after the transaction we cannot record it.

With #5750 being worked on, we'll add more auto-instrumentation, making it more likely that these values will get attached to an active transaction.

@AbhiPrasad
Copy link
Member

Given we've fixed this issue with #5983 and it was confirmed to be fixed by @LeoMelody, I'll be closing this issue. Please don't hesitate to open a new issue if anything else comes up. Thanks!

@LeoMelody
Copy link

But the LCP and CLS data is disappeared

We leverage the web vitals library to report LCP and CLS info so there are a couple of things that might be happening here.

  1. LCP/CLS is happening after your pageload is finished. We recommend increasing your idleTimeout to catch this possibility.
  2. We were reporting intermediate LCP/CLS values erroneously. As per the docs, it is recommended only the final LCP value is captured.
  3. The browser itself did not report an LCP/CLS value that we can use. If the web-vitals library does not record an LCP/CLS value, we cannot report it.

Generally we can only report values that are occuring during a running transaction (pageload/navigation), so if the metric triggers after the transaction we cannot record it.

With #5750 being worked on, we'll add more auto-instrumentation, making it more likely that these values will get attached to an active transaction.

OK. I'll try this.Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants