getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.76k stars 1.52k forks source link

Page Navigations picking up incorrect transaction name #12356

Open edwardgou-sentry opened 1 month ago

edwardgou-sentry commented 1 month ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

8.0.0

Framework Version

No response

Link to Sentry event

https://sentry.sentry.io/performance/trace/269e373c084640b2ae03a40395da2096/?fov=0%2C19730.000019073486&node=txn-f55ce92527a34a56ac96d4055b0933e4&statsPeriod=1h&timestamp=1717538248

SDK Setup

Sentry saas

Steps to Reproduce

On Sentry saas, navigate to the /profiling page, then navigate to the /issues page.

Expected Result

The navigation to the /issues page should create a transaction with /issues as the name.

Actual Result

The navigation to the /issues page produces a transaction with navigation - /profiling/ as the transaction name. It seems to have picked up the route name of the previous page somehow. The url tag looks correct though.

image image

edwardgou-sentry commented 1 month ago

I was able to reproduce this consistently as well on my vite react test app.

I'm not sure if it has something to do with transactions not closing properly, because theres a pretty suspicious gap in the transaction span waterfall

lforst commented 4 weeks ago

Hi, would you mind sharing something like a minimal reproducton on how to cause this behaviour? Thank you!

edwardgou-sentry commented 4 weeks ago

Hey @lforst, I just set up a simple vite + react app via: npm create vite@latest --template react Then I installed react-router-dom and sentry/react And I just set up two routes in react-router

Here's the setup:

import React from 'react';
import ReactDOM from 'react-dom/client';
import App from './App.jsx';
import Other from './Other.jsx';
import './index.css';
import { useEffect } from "react";
import * as Sentry from "@sentry/react";
import {
  createRoutesFromChildren,
  matchRoutes,
  useLocation,
  useNavigationType,
  createBrowserRouter,
  RouterProvider,
} from "react-router-dom";

Sentry.init({
  dsn: "...",
  integrations: [
    Sentry.browserTracingIntegration({
      useEffect,
      useLocation,
      useNavigationType,
      createRoutesFromChildren,
      matchRoutes,
    }),
    Sentry.replayIntegration(),
  ],
  tracesSampleRate: 1.0,
  debug: true,
});

const router = createBrowserRouter([
  {
    path: "/",
    element: <App/>,
  },
  {
    path: "/other/",
    element: <Other />,
  },
]);

ReactDOM.createRoot(document.getElementById("root")).render(
  <React.StrictMode>
    <RouterProvider router={router} />
  </React.StrictMode>
);

https://github.com/getsentry/sentry-javascript/assets/83961295/1603859f-b2e8-478e-8e49-557983cb4b06

edwardgou-sentry commented 4 weeks ago

I also realized that we might not consider this much of an issue, because we expect users to use reactRouterV6BrowserTracingIntegration.

I was only able to reproduce this using:

lforst commented 3 weeks ago

Which version of react-router did you install?

edwardgou-sentry commented 3 weeks ago

Which version of react-router did you install?

Hey @lforst , I installed react router v6!

lforst commented 3 weeks ago

Ok so unless I am missing something I think this is fine, because reactRouterV3BrowserTracingIntegration in combination with react-router@6 is a bit of undefined behavior. 🤔 If you use react-router@6 you should be using reactRouterV6BrowserTracingIntegration. If you are using react-router@3 you should be using reactRouterV3BrowserTracingIntegration.

edwardgou-sentry commented 3 weeks ago

@lforst sorry I should have clarified better:

For my vite + react test app, I am using react-router@6

On Sentry Saas, we are using react-router@3 and reactRouterV3BrowserTracingIntegration. I was able to reproduce this transaction name issue in this scenario.

So I agree this is probably fine for react-router@6, because users should be using the reactRouterV6BrowserTracingIntegration. For react-router@3 with reactRouterV3BrowserTracingIntegration, there may be an issue here because I could reproduce it on Sentry Saas.