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

feat(nuxt): Add "Deployment Provider Setup" #11486

Closed
wants to merge 3 commits into from

Conversation

s1gr1d
Copy link
Member

@s1gr1d s1gr1d commented Oct 4, 2024

DESCRIBE YOUR PR

Adds a new menu item to describe the deployment setup for Nuxt as this is different for every provider and still in an experimental state.

image

IS YOUR CHANGE URGENT?

Help us prioritize incoming PRs by letting us know when the change needs to go live.

  • Urgent deadline (GA date, etc.):
  • [x ] Other deadline: 4. Oct or 7. Oct would be nice
  • None: Not urgent, can wait up to 1 week+

SLA

  • Teamwork makes the dream work, so please add a reviewer to your PRs.
  • Please give the docs team up to 1 week to review your PR unless you've added an urgent due date to it.
    Thanks in advance for your help!

PRE-MERGE CHECKLIST

Make sure you've checked the following before merging your changes:

  • Checked Vercel preview for correctness, including links
  • PR was reviewed and approved by any necessary SMEs (subject matter experts)
  • PR was reviewed and approved by a member of the Sentry docs team

Copy link

vercel bot commented Oct 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
changelog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 7, 2024 11:34am
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 7, 2024 11:34am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
develop-docs ⬜️ Ignored (Inspect) Visit Preview Oct 7, 2024 11:34am

@s1gr1d s1gr1d requested review from mydea, lforst, lizokm and smeubank October 4, 2024 11:48
Copy link

codecov bot commented Oct 4, 2024

Bundle Report

Changes will increase total bundle size by 921 bytes (0.01%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
sentry-docs-server-cjs 7.45MB 930 bytes (0.01%) ⬆️
sentry-docs-edge-server-array-push 257.07kB 3 bytes (-0.0%) ⬇️
sentry-docs-client-array-push 6.42MB 6 bytes (-0.0%) ⬇️

Copy link
Member

@smeubank smeubank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work! we can add more from here in other PRs. Only wouldn't put it above source maps for now

@@ -0,0 +1,9 @@
---
title: Deployment Provider Setup
sidebar_order: 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sidebar_order: 2
sidebar_order: 4

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd suggest below source maps at least

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it 👍

Comment on lines +17 to +18
Internally, Sentry uses a specific ESM loader hook "[import-in-the-middle/hook.mjs](https://www.npmjs.com/package/import-in-the-middle)" which is
automatically added when you add the Sentry server-side config file with `--import`. This will automatically instrument all modules in your application.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: I am not sure if we need to/should include this - this is very much internal stuff that users don't really need to know/understand, and also could possibly change. IMHO it is enough to have the paragraph above, which states that sentry has to be initialized via --import ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to agree. I think wherever possible, we should only give the users the information they need most and avoid going too deep :)

@@ -0,0 +1,9 @@
---
title: Deployment Provider Setup
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L: This is very much bike shedding, but what about just calling this Deploying Nuxt with Sentry or something like this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, if we want to keep it generic, e.g. "Deploying your application" or something like this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s1gr1d @mydea could we just call this "Alternate Installation Methods"? or "Alternate Ways to Set Up"?

As described above, there are some prerequisites for adding Sentry to a Nuxt project in ESM syntax:

1. The possibility to add the node `--import` CLI flag.
2. Knowing the correct path to your Sentry server config file within your deployment environment (to add the path to the `--import` CLI flag).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
2. Knowing the correct path to your Sentry server config file within your deployment environment (to add the path to the `--import` CLI flag).
2. Knowing the correct path to your Sentry server config file within your deployment environment (as you need to pass it to the `--import` CLI flag).

1. The possibility to add the node `--import` CLI flag.
2. Knowing the correct path to your Sentry server config file within your deployment environment (to add the path to the `--import` CLI flag).
3. The `hook.mjs` file is available under `node_modules/import-in-the-middle/hook.mjs` wherever your server-side code is executed.
4. Running Node.js version 18.19.0 or higher (support for `--import`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
4. Running Node.js version 18.19.0 or higher (support for `--import`).
4. Running Node.js version 18.19.0 or higher

If you get a warning that the `hook.mjs` file is not found, please report this in [the Sentry JS GitHub repo](https://github.com/getsentry/sentry-javascript/issues) or file an issue at your deployment provider.

Nitro (which is used by Nuxt and SolidStart on the server side) uses [`@vercel/nft`](https://github.com/vercel/nft) during the build. `@vercel/nft` includes files that are added with `module.register()` from version 0.27.4 onwards.
In case you get a runtime error along the lines `Cannot find module '/node_modules/import-in-the-middle/hook.mjs'`, make sure to add a resolution (yarn) or an override (npm/pnpm) for `@vercel/nft`:
Copy link
Member

@mydea mydea Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In case you get a runtime error along the lines `Cannot find module '/node_modules/import-in-the-middle/hook.mjs'`, make sure to add a resolution (yarn) or an override (npm/pnpm) for `@vercel/nft`:
If you get a runtime error along the lines of `Cannot find module '/node_modules/import-in-the-middle/hook.mjs'`, be sure to add an override (npm/pnpm) or a resolution (yarn) for `@vercel/nft`:

we should probably mention npm first (generally)

Nitro (which is used by Nuxt and SolidStart on the server side) uses [`@vercel/nft`](https://github.com/vercel/nft) during the build. `@vercel/nft` includes files that are added with `module.register()` from version 0.27.4 onwards.
In case you get a runtime error along the lines `Cannot find module '/node_modules/import-in-the-middle/hook.mjs'`, make sure to add a resolution (yarn) or an override (npm/pnpm) for `@vercel/nft`:

```json {tabTitle:yarn} {filename: package.json}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we flip these around, npm first, then yarn?

Comment on lines +80 to +81


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

extra new lines here

---
title: Deployment Provider Setup
sidebar_order: 4
description: "Review our alternate installation methods."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably left over from copying this file ;)

The path to the function is depending on your setup. When building the preset locally, the file is available at: `.netlify/functions-internal/server/sentry.server.config.mjs`.

<Alert level="warning">
The docs for this are still vague and we will update them as soon as we know more about which exact path to use for the `--import` flag.
Copy link
Member

@mydea mydea Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess what you want to say here is that the docs are not really ready/done here yet? "vague" implies for me that everything works but is not documented well 🤔 Maybe rather:

Suggested change
The docs for this are still vague and we will update them as soon as we know more about which exact path to use for the `--import` flag.
The docs for this are still a work in progress. We'll update them as soon as we know more about which exact path to use for the `--import` flag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, does this work as of today? If it does not actually work we should not document it, and/or call out very specifically that it does not work!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to @mydea. I think it's best not to document it if we know it doesn't work.

Copy link
Contributor

@lizokm lizokm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should rethink what we name these new docs to avoid it sounding too vague. @s1gr1d let me know if you want to chat about it more and we can set up a call :)

@@ -0,0 +1,9 @@
---
title: Deployment Provider Setup
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s1gr1d @mydea could we just call this "Alternate Installation Methods"? or "Alternate Ways to Set Up"?

---
title: Deployment Provider Setup
sidebar_order: 4
description: "Review our alternate installation methods."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: "Review our alternate installation methods."
description: "Here are a few other ways to set up Sentry in your application."

@@ -0,0 +1,97 @@
---
title: General
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
title: General
title: General Deployment Setup

---
title: General
sidebar_order: 1
description: "Learn about the general deployment setup when using Sentry."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: "Learn about the general deployment setup when using Sentry."
description: "Learn how to use a general deployment setup when adding Sentry to a Nuxt project in ESM syntax."

Comment on lines +8 to +12

<PlatformSection supported={['javascript.nuxt']}>
Nuxt compiles all your code for the client and server side to ECMAScript modules (ESM) syntax ([learn more about ESM and Nuxt](https://nuxt.com/docs/guide/concepts/esm)).
</PlatformSection>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<PlatformSection supported={['javascript.nuxt']}>
Nuxt compiles all your code for the client and server side to ECMAScript modules (ESM) syntax ([learn more about ESM and Nuxt](https://nuxt.com/docs/guide/concepts/esm)).
</PlatformSection>
<PlatformSection supported={['javascript.nuxt']}>
Nuxt compiles all your code for the client and server side to ECMAScript modules (ESM) syntax ([learn more about ESM and Nuxt](https://nuxt.com/docs/guide/concepts/esm)).
</PlatformSection>

It would be great if you could add a little more context here. Like a sentence or two explaining why a user would want to use this type of setup. Maybe something as simple as: Use this setup if you want to add Sentry to a Nuxt project in ESM syntax. (As the first line on the page.)

Comment on lines +12 to +16

<Alert level="info">
Before approaching the steps below, read the general documentation around <PlatformLink to="/deployment-provider-setup/general/">setting up Sentry on a deployment provider</PlatformLink> to get quick general overview of the process.
This page only shows Netlify-specific steps.
</Alert>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Alert level="info">
Before approaching the steps below, read the general documentation around <PlatformLink to="/deployment-provider-setup/general/">setting up Sentry on a deployment provider</PlatformLink> to get quick general overview of the process.
This page only shows Netlify-specific steps.
</Alert>
This page only shows Netlify-specific steps. We recommend that you read the general documentation for <PlatformLink to="/deployment-provider-setup/general/">setting up Sentry on a deployment provider</PlatformLink> to get quick general overview of the process before getting started with the steps below.

It's a bit jarring to have 2 different-colored alerts at the very beginning of the page. I think this sentence works better as a regular sentence rather than as an alert.

This page only shows Netlify-specific steps.
</Alert>

## Approach 1: Adding the `--import` flag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## Approach 1: Adding the `--import` flag
## Approach 1: Adding the `--import` Flag

Comment on lines +19 to +23

<PlatformSection supported={['javascript.nuxt']}>
The server-side of Nuxt is deployed as a Netlify Function. So make sure to scope the environment variable `NODE_OPTIONS` to "Functions".
The path to the function is depending on your setup. When building the preset locally, the file is available at: `.netlify/functions-internal/server/sentry.server.config.mjs`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<PlatformSection supported={['javascript.nuxt']}>
The server-side of Nuxt is deployed as a Netlify Function. So make sure to scope the environment variable `NODE_OPTIONS` to "Functions".
The path to the function is depending on your setup. When building the preset locally, the file is available at: `.netlify/functions-internal/server/sentry.server.config.mjs`.
<PlatformSection supported={['javascript.nuxt']}>
The server-side of Nuxt is deployed as a Netlify Function, so make sure to scope the environment variable `NODE_OPTIONS` to "Functions".
The path to the function depends on your setup. If you're building the preset locally, you'll find the file at: `.netlify/functions-internal/server/sentry.server.config.mjs`.

The path to the function is depending on your setup. When building the preset locally, the file is available at: `.netlify/functions-internal/server/sentry.server.config.mjs`.

<Alert level="warning">
The docs for this are still vague and we will update them as soon as we know more about which exact path to use for the `--import` flag.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to @mydea. I think it's best not to document it if we know it doesn't work.

Comment on lines +34 to +39

Enabling `experimental_basicServerTracing` can be used when adding the node option `--import` does not work. This however only comes with limited tracing functionality because Sentry server config is not **pre**loaded with `--import`.

<PlatformSection supported={['javascript.nuxt']}>
Enabling this option will automatically import the Sentry server config at the top of the server entry file (server entry: `.netlify/functions-internal/server/server.mjs`).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Enabling `experimental_basicServerTracing` can be used when adding the node option `--import` does not work. This however only comes with limited tracing functionality because Sentry server config is not **pre**loaded with `--import`.
<PlatformSection supported={['javascript.nuxt']}>
Enabling this option will automatically import the Sentry server config at the top of the server entry file (server entry: `.netlify/functions-internal/server/server.mjs`).
If adding the node option `--import` doesn't work, you can enable `experimental_basicServerTracing` instead. Note, that `experimental_basicServerTracing` only comes with limited tracing functionality because Sentry server config is not **pre**loaded with `--import`.
<PlatformSection supported={['javascript.nuxt']}>
Enabling this option will automatically import the Sentry server config at the top of the server entry file (server entry: `.netlify/functions-internal/server/server.mjs`).

@lforst lforst removed their request for review October 23, 2024 07:31
@getsantry
Copy link
Contributor

getsantry bot commented Nov 13, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added Stale and removed Stale labels Nov 13, 2024
@s1gr1d
Copy link
Member Author

s1gr1d commented Nov 18, 2024

I'll close this as users don't have to include the --import flag anymore and setting Sentry up in Nuxt is much easier now. But those docs could still be added in case a deployment provider setup gets too complicated.

@s1gr1d s1gr1d closed this Nov 18, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants