-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(metrics): Add comments for v7 configuration #10154
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(metrics): Add comments for v7 configuration #10154
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| ``` | ||
|
|
||
| ```JavaScript | ||
| // instrumentation.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had not filename for the server side configuration snippet of NextJS.
From what I gathered it should be instrumentation.js?
Bundle ReportChanges will decrease total bundle size by 13 bytes ⬇️
|
| dsn: '___PUBLIC_DSN___', | ||
| // Only needed for SDK versions < 8.0.0 | ||
| // integrations: [ | ||
| // Sentry.metrics.metricsAggregatorIntegration(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, where does this come from? We don't really do this for any other thing, we always show up-to-date docs as far as I can tell? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of metrics GA happening so close to v8, we had some confusion about this and people had troubles intrumenting custom metrics. Daniel gave the green light to have a short-term callout for this in docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users that still use v7 were confused during the onboarding process.
We discussed it with @HazAT as a temporary measure for metrics GA in the planning today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright, all good then!
| @@ -1,10 +1,11 @@ | |||
| To configure metrics, add the `metricsAggregator` experimental option to your `Sentry.init` call. | |||
| Metrics work out of the box by calling `Sentry.init` no further setup is required. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Metrics work out of the box by calling `Sentry.init` no further setup is required. | |
| Metrics work out of the box by calling `Sentry.init`, no further setup is required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, thx! will also fix the source of the copy 🍝
Display v7 configuration instructions for metrics as comment.
Relates to getsentry/sentry#71510