getsentry / sentry-javascript

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

Docs for integrating with Nx monorepo #8982

Open nordhagen opened 10 months ago

nordhagen commented 10 months ago

Problem Statement

The Sentry docs need a dedicated section on how to properly configure Sentry in the context of a NextJS app in an Nx monorepo.

Solution Brainstorm

Seeing as this is a common environment, I believe the Sentry documentation needs a dedicated example for this. The basic example for NextJS serves to confuse because of how Nx, plugins, NextJS and Sentry are to interact. I see lots of questions online about this with many different attempts at answering. I think there should be one canonical example from an authoritative source on this.

lforst commented 10 months ago

@nordhagen Thanks for opening this feature request!

We haven't yet investigated what causes the interaction with Nx to be such a pain. Putting this on our backlog. If you have any ideas and pointers what Nx is doing so that we can work around it in the SDK, feel free to ping us here!

nordhagen commented 10 months ago

@lforst Thank you for the quick response. I would love to help, but unfortunately I'm really struggling myself and I have nothing to add right now.

I believe the problem is two-fold:

  1. There seems to be a convention around wrapping the nextConfig in a function call and assigning the returned value to module.exports. However both Nx and Sentry wants to do this much the same way, which makes the resulting config awkward to reason about. (module.exports = composePlugins(...plugins)(withSentryConfig(nextConfig, sentryWebpackPluginOptions))???
  2. The withNx plugin is required in Nx to make NextJS aware of the monorepo environment. Without it package paths will not be correct. I'm not sure, but I believe there might be something in the way the Sentry config reads the resulting environment or that the order of operations matter in an implicit way.

Sorry I couldn't be of more help. Ultimately the best would be if @sentry/wizard could be Nx workspace-aware.

lforst commented 10 months ago

@nordhagen Thanks! For me, right now this seems like a documentation issue but I will have to verify - anyhow, we'll see what we can do!

nordhagen commented 10 months ago

@nordhagen Thanks! For me, right now this seems like a documentation issue but I will have to verify - anyhow, we'll see what we can do!

Thanks. Documentation goes a long way. But BTW the Vercel integration for Sentry also seems to struggle with Nx monorepos. At least I couldn't get it to work properly with our setup. Then again that might be because the NextJS config isn't correct.

nordhagen commented 9 months ago

@lforst Sorry to nag, but is there any movement on this? I'm thinking you guys who know your product could probably add this in no time 😬

lforst commented 9 months ago

I don't think this is a priority for us right now. Especially since Nx seems to be the offending party here. Our SDK is generally pretty indifferent about import paths and so on as long as it conforms to standard resolution mechanisms.

It seems Nx has added instructions to the docs on how things should be set up if you use Sentry: https://nx.dev/recipes/next/next-config-setup#composing-plugins-using-utility-(nx-16-and-later):~:text=Sentry%20needs%20to%20be%20added%20last which I notice is just wrong.

I'm gonna edit their docs. However, I am not sure if I will add this to our docs since Nx usage seems to be low - relatively speaking.

nordhagen commented 9 months ago

Thank you for the quick response. Could you please explain what it is that's wrong about their code example, so that maybe I can get our setup done without waiting for Nx to approve your changes to their docs?

Also, from this repo it looks like Sentry is using Nx as well :)

lforst commented 9 months ago

@nordhagen I opened a PR to their docs that shows the right way to do it: https://github.com/nrwl/nx/pull/19338

charleskoehl commented 9 months ago

@nordhagen thanks for opening this feature request. It would be very helpful to include instructions for making sure sourcemaps for shared libs are uploaded and properly referenced by the app sourcemaps. We still have not found a solution for this, so almost none of the sentry stacktraces are unminified.

nordhagen commented 9 months ago

@lforst Thanks a lot for the link to the PR. I was able to get it working with you suggestions and we now finally have source maps and a setup we can actually reason about. Much appreciated 🙏

Issue can be closed as far as I'm concerned, but seeing as this was very valuable for me, you might consider adding your suggestion to Nx in the Sentry docs as well since you already have it figured out.

lforst commented 9 months ago

@nordhagen Thanks for circling back! <3 I will think about adding docs for Nx! Can I ask what exact suggestion of mine turned out to be helpful?

nordhagen commented 9 months ago

@nordhagen Thanks for circling back! <3 I will think about adding docs for Nx! Can I ask what exact suggestion of mine turned out to be helpful?

Not one thing specifically. Using the PR I was able to pull out a complete example which showed how the next config and the NX and Sentry parts of it relate to each other. This enabled me to understand how the config needs to be built so it works. Complete resulting config below.

/* eslint-disable @typescript-eslint/no-var-requires */
const { withNx } = require('@nx/next')
const { createSecureHeaders } = require('next-secure-headers')
const { redirects } = require('./utils/redirects')
const { withSentryConfig } = require('@sentry/nextjs')
const BundleAnalyzerPlugin =
  require('webpack-bundle-analyzer').BundleAnalyzerPlugin

const sentryWebpackPluginOptions = {
  org: 'hjemmelegene',
  project: 'app-frontend',
  authToken: process.env.SENTRY_AUTH_TOKEN,
  silent: true,
  plugins: [new BundleAnalyzerPlugin()],
}

const nextConfig = {
  sentry: {
    hideSourceMaps: true,
  },
  compiler: {
    styledComponents: true,
  },
  webpack: (config) => {
    const originalEntry = config.entry
    config.entry = async () => {
      const entries = await originalEntry()
      if (
        entries['main.js'] &&
        !entries['main.js'].includes('./client/polyfills.js')
      ) {
        entries['main.js'].unshift('./client/polyfills.js')
      }
      return entries
    }
    config.resolve.fallback = { fs: false }
    return config
  },
  exportPathMap: async function (defaultPathMap) {
    return {
      '/': { page: '/' },
    }
  },
  rewrites: async function () {
    return [
      /* Specifics excluded */
    ]
  },
  headers: async function () {
    return [
      {
        source: '/(.*)',
        headers: createSecureHeaders({
          frameGuard: 'sameorigin',
        }),
      },
    ]
  },
  redirects: async function () {
    return redirects
  },
  images: {
    domains: [
      /* Specifics excluded */
    ],
  },
  nx: {
    svgr: false,
  },
}

const plugins = [

]

module.exports = async (phase, context) => {
  let updatedConfig = plugins.reduce((acc, fn) => fn(acc), nextConfig)

  updatedConfig = await withNx(updatedConfig)(phase, context)
  updatedConfig = withSentryConfig(updatedConfig, sentryWebpackPluginOptions)

  return updatedConfig
}