add instruction on node --require #9932

andreiborza · 2024-05-07T12:03:28Z

No description provided.

vercel · 2024-05-07T12:03:31Z

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

Name	Status	Preview	Comments	Updated (UTC)
sentry-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 10, 2024 1:17pm

codecov · 2024-05-07T12:09:24Z

Bundle Report

Changes will increase total bundle size by 3.01kB ⬆️

Bundle name	Size	Change
sentry-docs-server	7.43MB	3.01kB ⬆️
sentry-docs-edge-server	456.19kB	0 bytes
sentry-docs-client	6.16MB	2 bytes ⬆️

s1gr1d · 2024-05-07T12:44:40Z

docs/platforms/javascript/guides/node/index.mdx


 Once this is done, Sentry's Node SDK captures unhandled exceptions as well as tracing data for your application.

+Therefore, we recommend to create a file named `instrumentation.js` that imports and initializes Sentry.


Nitpick regarding line 36 and 46 in this file: Can you change the dash to an em dash?

lforst · 2024-05-07T12:47:22Z

platform-includes/getting-started-run-alternative/javascript.node.mdx

+```javascript {tabTitle:CommonJS}
+require('./instrumentation.js')
+


I would argue that we should not honor this escape hatch an entire code block.

Considerations:

People might do both --require AND the import

It might be confusing

We really do not want people to do that unless absolutely necessary and a dedicated code block might be promoting it.

I agree. I added these specifically to show that it really has to go to the top and any other imports come after but this is already explained in text as well. Probably better to drop it.

I have a different opinion: We definitely should to show this, because there will def. be people out there who do not have the (easy) capability to adjust the code their app is run in production. Lots of devs do not have direct access to their deployment infrastructure etc, making it much harder for them to get started with using sentry. We can call out why the other solution is preferable and lead with that, but should still show how you can get it to run otherwise. There is really nothing wrong with the instrument-on-top approach for require.js.

actually, we need to keep this anyhow and add a note that this is definitely needed for node <18 - no other way to run stuff there, I think?

actually, we need to keep this anyhow and add a note that this is definitely needed for node <18 - no other way to run stuff there, I think?

--require is supported since v1.6.0

I have a different opinion: We definitely should to show this, because there will def. be people out there who do not have the (easy) capability to adjust the code their app is run in production. Lots of devs do not have direct access to their deployment infrastructure etc, making it much harder for them to get started with using sentry. We can call out why the other solution is preferable and lead with that, but should still show how you can get it to run otherwise. There is really nothing wrong with the instrument-on-top approach for require.js.

We were just thinking that people will skip over reading the leading text and just copy paste code snippets around, ending up with both --require and a top level import.

--require is supported since v1.6.0

Ah, sweet! Good to know 🚀

We were just thinking that people will skip over reading the leading text and just copy paste code snippets around, ending up with both --require and a top level import.

You need to read this anyhow, as the text instructs you to create another file etc anyhow - this is not as easy as copy-pasting a snippet into your app.js anyhow, I'd argue. If you don't read this, it'll not work anyhow.

I'd just like to argue that while for us it is fine and easy to "just add a CLI flag when running the node app", there are loads of devs out there for whom this is not as trivial, and (as stated before) even more that simply can't do this themselves. Why should we not document how they can still use Sentry even if they can't easily adjust this themselves? I don't really see the upside of "hiding" this away? It's not even that problematic if this is double-initialized, it should work just fine even if somebody does that. And we could even guard against this, if we actually need it.

Yea you're right. I can definitely see some orgs disallowing passing flags to node, or like you said, people not even having the ability to change how things are run.

Will keep it, but maybe give it its own heading — as is, it's easy to not see that this is an alternative way of running it, wdyt?

lforst · 2024-05-07T12:47:48Z

platform-includes/getting-started-config/javascript.node.mdx

-const http = require("http");
 ```

 ```javascript {tabTitle:ESM}


Adding a filename here now would be nice!

…amples

docs/platforms/javascript/guides/node/index.mdx

Co-authored-by: vivianyentran <[email protected]>

Lms24

Can we also update the SDK initialization section in the migration guide? https://docs.sentry.io/platforms/javascript/guides/node/migration/v7-to-v8/#updated-sdk-initialization

(feel free to do this in a follow-up PR, don't want to block this any longer than necessary)

andreiborza · 2024-05-08T10:24:55Z

Can we also update the SDK initialization section in the migration guide? https://docs.sentry.io/platforms/javascript/guides/node/migration/v7-to-v8/#updated-sdk-initialization

(feel free to do this in a follow-up PR, don't want to block this any longer than necessary)

Yep can do.

s1gr1d

Thanks for adding this 🙌🏻
Do not forget to also add this to koa, aws-lambda, azure-functions, connect, express and fastify.

s1gr1d · 2024-05-08T13:36:32Z

docs/platforms/javascript/guides/node/index.mdx

 ## Configure

-Sentry should be initialized as early in your app as possible. It is essential that you call `Sentry.init` before you require any other modules in your application - otherwise, auto-instrumentation of these modules will _not_ work.
+Sentry should be initialized as early in your app as possible. It is essential that you call `Sentry.init` before you require any other modules in your application — otherwise, auto-instrumentation of these modules will _not_ work.


Suggested change

Sentry should be initialized as early in your app as possible. It is essential that you call `Sentry.init` before you require any other modules in your application — otherwise, auto-instrumentation of these modules will _not_ work.

Sentry should be initialized as early in your app as possible. It is essential that you call `Sentry.init` before you require any other modules in your application—otherwise, auto-instrumentation of these modules will _not_ work.

Thanks, fixed

s1gr1d · 2024-05-08T13:36:46Z

docs/platforms/javascript/guides/node/index.mdx

 If you set a `tracesSampleRate`, performance instrumentation will automatically be enabled for you. See <PlatformLink to="/performance/instrumentation/automatic-instrumentation">Automatic Instrumentation</PlatformLink> to learn about all the things that the SDK automatically instruments for you.

-You can also manually capture performance data - see <PlatformLink to="/performance/instrumentation/custom-instrumentation">Custom Instrumentation</PlatformLink> for details.
+You can also manually capture performance data — see <PlatformLink to="/performance/instrumentation/custom-instrumentation">Custom Instrumentation</PlatformLink> for details.


Suggested change

You can also manually capture performance data — see <PlatformLink to="/performance/instrumentation/custom-instrumentation">Custom Instrumentation</PlatformLink> for details.

You can also manually capture performance data—see <PlatformLink to="/performance/instrumentation/custom-instrumentation">Custom Instrumentation</PlatformLink> for details.

s1gr1d · 2024-05-08T13:49:04Z

docs/platforms/javascript/guides/node/run/index.mdx

@@ -0,0 +1,28 @@
+---
+title: Run Methods


We talked about that already in person: Maybe call this page "Initialization Variants" 🤔

Yep, renamed it.

s1gr1d · 2024-05-08T13:49:23Z

docs/platforms/javascript/guides/node/run/run-as-toplevel-import.mdx

@@ -0,0 +1,8 @@
+---
+title: Run as toplevel import


Suggested change

title: Run as toplevel import

title: Add as top-level import

Thank you :) fixed

andreiborza · 2024-05-09T19:47:43Z

Thanks for adding this 🙌🏻 Do not forget to also add this to koa, aws-lambda, azure-functions, connect, express and fastify.

Added the others except aws-lambda and azure-functions because I'm unsure if we should advocate an extra file there as we discussed in the office the other day.

If you could have another look please :)

Co-authored-by: vivianyentran <[email protected]> Co-authored-by: Luca Forstner <[email protected]> Co-authored-by: Stephanie Anderson <[email protected]>

add instruction on node --require

2b76c04

vercel bot deployed to Preview May 7, 2024 12:19 View deployment

s1gr1d reviewed May 7, 2024

View reviewed changes

lforst reviewed May 7, 2024

View reviewed changes

andreiborza added 2 commits May 7, 2024 14:58

Use proper emdash

5d99b50

Replace instrumentation.js with instrument.js, add filename to code s…

6e20f0f

…amples

andreiborza force-pushed the ab/node-require-instrumentationjs branch from a2b5709 to 6e20f0f Compare May 7, 2024 13:08

require/import

298ada5

vercel bot deployed to Preview May 7, 2024 13:43 View deployment

vivianyentran approved these changes May 7, 2024

View reviewed changes

docs/platforms/javascript/guides/node/index.mdx Outdated Show resolved Hide resolved

Update docs/platforms/javascript/guides/node/index.mdx

615bbce

Co-authored-by: vivianyentran <[email protected]>

vercel bot deployed to Preview May 8, 2024 07:39 View deployment

Lms24 reviewed May 8, 2024

View reviewed changes

Lms24 mentioned this pull request May 8, 2024

meta: Add CHANGELOG entry for 8.0.0 getsentry/sentry-javascript#11948

Closed

Refine run methodsb

c517790

vercel bot deployed to Preview May 8, 2024 13:24 View deployment

s1gr1d reviewed May 8, 2024

View reviewed changes

andreiborza added 2 commits May 8, 2024 16:02

Rename category to initialize sentry

96c2e28

Fix wording

d1668a8

s1gr1d approved these changes May 8, 2024

View reviewed changes

vercel bot deployed to Preview May 8, 2024 14:32 View deployment

stephanie-anderson and others added 3 commits May 8, 2024 17:15

Merge branch 'v8-docs' into ab/node-require-instrumentationjs

b71b3ec

Update node version numbers

ab6a255

Remove sign-in notice for snippet that didn't need it

efb01ec

vercel bot deployed to Preview May 8, 2024 15:32 View deployment

andreiborza added 2 commits May 9, 2024 20:30

Add express

29656ed

Add connect

c6a656d

andreiborza added 2 commits May 9, 2024 21:27

Add koa

e063d98

Add fastify

cba2c4e

vercel bot deployed to Preview May 9, 2024 20:02 View deployment

andreiborza added 5 commits May 9, 2024 23:09

Update node migration guide to use --require

49fd43d

Update express migration guide to use --require

e499779

Update connect migration guide to use --require

3b534bb

Update koa migration guide to use --require

4b6aabf

Add fastify migration guide to use --require

038de5f

vercel bot deployed to Preview May 9, 2024 21:55 View deployment

Add link to fastify migration guide

36247c6

vercel bot deployed to Preview May 10, 2024 06:59 View deployment

andreiborza added 2 commits May 10, 2024 09:14

Bump required node version for --import

8b643c9

Update node migration guide to include ESM snippets

1d9e28b

vercel bot deployed to Preview May 10, 2024 07:31 View deployment

andreiborza added 5 commits May 10, 2024 09:48

Update express migration guide to include ESM snippets

23f05ff

Update connect migration guide to include ESM snippets

0c74c0d

Update koa migration guide to include ESM snippets

2375652

Update fastify migration guide to include ESM snippets

657790d

Update ESM compat note with support from Node v18.19.0 onwards

252268f

vercel bot deployed to Preview May 10, 2024 08:43 View deployment

andreiborza marked this pull request as ready for review May 10, 2024 08:45

Restructure and reword

d6a29f7

vercel bot deployed to Preview May 10, 2024 13:17 View deployment

lforst merged commit b5b1e2c into v8-docs May 10, 2024

lforst deleted the ab/node-require-instrumentationjs branch May 10, 2024 14:07

mydea pushed a commit that referenced this pull request May 13, 2024

add instruction on node --require (#9932)

a4e6865

Co-authored-by: vivianyentran <[email protected]> Co-authored-by: Luca Forstner <[email protected]> Co-authored-by: Stephanie Anderson <[email protected]>

github-actions bot locked and limited conversation to collaborators May 29, 2024


		Once this is done, Sentry's Node SDK captures unhandled exceptions as well as tracing data for your application.

		Therefore, we recommend to create a file named `instrumentation.js` that imports and initializes Sentry.

		```javascript {tabTitle:CommonJS}
		require('./instrumentation.js')

	Sentry should be initialized as early in your app as possible. It is essential that you call `Sentry.init` before you require any other modules in your application — otherwise, auto-instrumentation of these modules will _not_ work.
	Sentry should be initialized as early in your app as possible. It is essential that you call `Sentry.init` before you require any other modules in your application—otherwise, auto-instrumentation of these modules will _not_ work.

	You can also manually capture performance data — see <PlatformLink to="/performance/instrumentation/custom-instrumentation">Custom Instrumentation</PlatformLink> for details.
	You can also manually capture performance data—see <PlatformLink to="/performance/instrumentation/custom-instrumentation">Custom Instrumentation</PlatformLink> for details.

Uh oh!

add instruction on node --require #9932

add instruction on node --require #9932

Conversation

andreiborza commented May 7, 2024

Uh oh!

vercel bot commented May 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Lms24 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreiborza commented May 8, 2024

Uh oh!

s1gr1d left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreiborza commented May 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

vercel bot commented May 7, 2024 •

edited

Loading

codecov bot commented May 7, 2024 •

edited

Loading

Lms24 left a comment •

edited

Loading