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

React Router V5 Integration is not parameterizing transaction names #11815

Closed
3 tasks done
nicholas-codecov opened this issue Apr 26, 2024 · 7 comments · Fixed by #11855
Closed
3 tasks done

React Router V5 Integration is not parameterizing transaction names #11815

nicholas-codecov opened this issue Apr 26, 2024 · 7 comments · Fixed by #11855

Comments

@nicholas-codecov
Copy link

nicholas-codecov commented Apr 26, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/react

SDK Version

8.0.0-beta.3

Framework Version

React 18.2.0

Link to Sentry event

https://codecov.sentry.io/performance/?project=5514400&statsPeriod=14d

SDK Setup

export const setupSentry = ({
  history,
}: {
  history: ReturnType<typeof createBrowserHistory>
}) => {
  // no sentry dsn, don't init
  if (!config.SENTRY_DSN) {
    return
  }

  // configure sentry product integrations
  const replay = Sentry.replayIntegration()
  const tracing = Sentry.reactRouterV5BrowserTracingIntegration({
    history,
  })

  const integrations = [replay, tracing]

  // Only show feedback button in production
  // spotlight takes the place of the feedback widget in dev mode
  if (config.NODE_ENV === 'production') {
    const feedback = Sentry.feedbackIntegration({
      colorScheme: 'light',
      showBranding: false,
      formTitle: 'Give Feedback',
      buttonLabel: 'Give Feedback',
      submitButtonLabel: 'Send Feedback',
      nameLabel: 'Username',
      isEmailRequired: true,
    })
    integrations.push(feedback)
  }

  const tracePropagationTargets = ['api.codecov.io', 'stage-api.codecov.dev']
  // wrapped in a silent try/catch incase the URL is invalid
  try {
    const { hostname } = new URL(config.API_URL)
    // add the hostname to the tracePropagationTargets if it's not already there
    if (!tracePropagationTargets.includes(hostname)) {
      tracePropagationTargets.push(hostname)
    }
  } catch {}

  Sentry.init({
    dsn: config.SENTRY_DSN,
    debug: config.NODE_ENV !== 'production',
    release: config.SENTRY_RELEASE,
    environment: config.SENTRY_ENVIRONMENT,
    integrations,
    tracePropagationTargets,
    tracesSampleRate: config?.SENTRY_TRACING_SAMPLE_RATE,
    // Capture n% of all sessions
    replaysSessionSampleRate: config?.SENTRY_SESSION_SAMPLE_RATE,
    // Of the remaining x% of sessions, if an error happens start capturing
    replaysOnErrorSampleRate: config?.SENTRY_ERROR_SAMPLE_RATE,
    beforeSend: (event, _hint) => {
      if (checkForBlockedUserAgents()) {
        return null
      }

      return event
    },
    beforeSendTransaction: (event, _hint) => {
      if (checkForBlockedUserAgents()) {
        return null
      }

      return event
    },
    ...deClutterConfig,
  })

  if (config.NODE_ENV === 'development') {
    Spotlight.init({
      injectImmediately: true,
      integrations: [Spotlight.sentry()],
    })
  }
}

Steps to Reproduce

  1. Installed v8.0.0-beta.3

Just to note, we do currently have two routes that are not wrapped with the Sentry helper.

Expected Result

Screenshot 2024-04-26 at 11 24 56

Actual Result

Screenshot 2024-04-26 at 11 24 31

@nicholas-codecov
Copy link
Author

We have since bumped to beta 4 of V8.

@mydea
Copy link
Member

mydea commented Apr 29, 2024

Hey, just to clarify: Are you using withSentryRouting as described here:

https://docs.sentry.io/platforms/javascript/guides/react/features/react-router/#parameterized-transaction-names

without this, you wouldn't get parametrized routes.

@nicholas-codecov
Copy link
Author

Yes sorry, i forgot to include that portion of our Sentry file:

https://github.com/codecov/gazebo/blob/main/src/sentry.ts#L53

@mydea
Copy link
Member

mydea commented Apr 30, 2024

Thanks for bearing with us! I identified the problem and have a fix up here: #11855

@nicholas-codecov
Copy link
Author

@mydea thanks for the quick fix, wondering if this will also be included with V8?

@mydea
Copy link
Member

mydea commented Apr 30, 2024

@mydea thanks for the quick fix, wondering if this will also be included with V8?

Yes, it will be in the next release - most likely we'll cut v8.0.0-beta.6 early thursday!

@nicholas-codecov
Copy link
Author

Awesome tysm!

mydea added a commit that referenced this issue May 2, 2024
Turns out we were not correctly setting parametrized route names for
react router v4/v5 😬 We had no proper test covering this.

So I added a new E2E test `react-router-5` that actually checks that
parametrization etc. works as expected.
While at it, I also updated the react-router-6 E2E test to actually
check these things as well - previously, we mostly only checked that
_anything_ was sent to sentry, but didn't look at the content we sent.

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

Successfully merging a pull request may close this issue.

3 participants