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

NextJS Sentry is using deprecated beforeNavigate instead of beforeStartSpan #11627

Closed
3 tasks done
mpranjic24 opened this issue Apr 16, 2024 · 7 comments
Closed
3 tasks done

Comments

@mpranjic24
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/nextjs

SDK Version

7.110.0

Framework Version

13.2.4

Link to Sentry event

No response

SDK Setup

Sentry.init({
  dsn: __YOUR_DSN__,
  tracesSampleRate: 0.1,
  environment: __YOUR_ENV__,
  integrations: [
    Sentry.browserTracingIntegration({
      beforeStartSpan: (context) => {
        console.log(context);
        return {
          ...context,
          name: getTransactionName(context),
        };
      },
    }),
  ],
});

Steps to Reproduce

Used configuration from above in sentry.client.config.ts by following the documentation.

Expected Result

Function beforeStartSpan should be called and a log should appear in console when it's called.

Actual Result

Function beforeStartSpan is never called. I suspect it's caused by the weird fixBrowserTracingIntegration method that converts the most recent browser tracing integration to the deprecated one.

What's more, using beforeNavigate actually works, but produces TypeScript errors since it's not defined in BrowserTracingOptions.

This is contrary to what the documentation says.

@mydea
Copy link
Member

mydea commented Apr 17, 2024

Hey, thanks for writing in! Hmm, this seems like a bug to me - we'll look into it!

@AbhiPrasad
Copy link
Member

Fix released with https://github.com/getsentry/sentry-javascript/releases/tag/7.111.0 - please upgrade and give it a try!

@mpranjic24
Copy link
Author

Unfortunately it doesn't seem to work, @AbhiPrasad. Nothing gets logged. On a side note, before the fix I could make it work with:

// @ts-expect-error: ignore TypeScript
      beforeNavigate(options: { name: string }) {
        ...
     }

That actually produced some console logs and in Chrome network view I could see that the transaction name got updated.

Now, it seems that it doesn't work at all.

@mydea mydea reopened this Apr 19, 2024
@mydea
Copy link
Member

mydea commented Apr 24, 2024

I will look into it, it seems that something is really off there, sorry about that!

mydea added a commit that referenced this issue Apr 24, 2024
…on` (#11765)

Turns out the logic to get the options was not correct, because we used
the `options` that the integration exposes, but this has
`instrumentPageload: false` & `instrumentNavigation: false` because we
did not "fix" the `options` we expose on the integration. This was not
caught by the tests, because it only happens if passing the
`browserTracingIntegration` from `@sentry/nextjs` or
`@sentry/sveltekit`, not when passing the "original" one (which we had
tests for).

Really fixes #11627
@mydea
Copy link
Member

mydea commented Apr 24, 2024

So we've just released v7.112.2, where we hopefully actually have a full fix for your problem - sorry about the confusion!
Gonna leave this open this time until I've got confirmation that the issue is actually solved 😅

@mpranjic24
Copy link
Author

So we've just released v7.112.2, where we hopefully actually have a full fix for your problem - sorry about the confusion! Gonna leave this open this time until I've got confirmation that the issue is actually solved 😅

From what I can see, beforeStartSpan indeed gets called now 👍

Also, in the big "envelope/..." request I can see that the transaction property got changed in 2 places 👍

@s1gr1d
Copy link
Member

s1gr1d commented Apr 29, 2024

This seems to be fixed, so I am closing this 👍

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

No branches or pull requests

4 participants