-
Couldn't load subscription status.
- Fork 1.2k
Adding Sentry plugin doc #965
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Nice, clean documentation! 👏
| * Include additional metadata in Sentry events to assist in debugging | ||
| * Expose a [global Sentry service](#global-sentry-service) | ||
|
|
||
| ## Installation |
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.
The titles in this documentation don't have the same format as those in the Email plugin documentation @StrapiShaun is rewriting. Since you 2 are (re)writing 3 plugin docs at the same time (Sentry, Upload, Email) it would be nice to follow similar structures if possible and make the same choices in terms of titles format etc.
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.
Also, should we use upper case or lower case for the bullet points here?
Include → include, etc.
We've been using lower-case versions in other places of the documentation, but maybe it should be uppercase in proper English? 🤔
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.
@StrapiShaun and I did discuss this, and don't believe there's enough similarity in the plugins that they can follow an identical template / format, so I don't think that's workable either here or for future plugin/provider docs.
@pwizla My vote is obviously for uppercase but happy to change it if the rest disagree?
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.
Great and clean PR, Gabriel, thanks!
On top of Mégane's comments, I have a few more questions or suggestions (see dedicated comments), plus a general one:
This is something we started discussing with the GraphQL configuration Gustav documented the other day. Do you think we should keep plugins configurations in Setup & Deployment > Configurations > Optional > Plugins (like we did for GraphQL already) or should these configurations stay in the dedicated plugins documentation sections?
Since we'll probably add more plugins configurations, and while our team is rewriting Email & Upload docs, what do you all think about this? (cc @meganelacheny @StrapiShaun)
| * Include additional metadata in Sentry events to assist in debugging | ||
| * Expose a [global Sentry service](#global-sentry-service) | ||
|
|
||
| ## Installation |
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.
Also, should we use upper case or lower case for the bullet points here?
Include → include, etc.
We've been using lower-case versions in other places of the documentation, but maybe it should be uppercase in proper English? 🤔
|
|
||
| Create or edit your `./config/plugins.js` file to configure the Sentry plugin. The following properties are available: | ||
|
|
||
| | Property | Type | Default Value | Description | |
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.
To stay consistent with other documentation sections (for instance Configurations pages, but please feel free to highlight any inconsistency we might have anywhere else), I'd suggest we have these columns, in this specific order:
Parameter, Description, Type, Default (or "Default value" if you prefer).
Also, I think we capitalized types (e.g. Boolean, Integer) in other parts of the documentations. Though maybe we could change this. MDN usually uses capitalized type names, but what typeof returns in JavaScript is not capitalized (typeof true returns boolean for instance). 🤔
|
|
||
| | Method | Description | Parameters | | ||
| | ------ | ----------- | ---------- | | ||
| | `sendError` | Manually send errors to Sentry. | <ul><li><code>error</code>: The error to be sent.</li><li><code>configureScope</code>: Optional. Enables you to customize the error event.</li></ul><br>See the official [Sentry documentation](https://docs.sentry.io/platforms/node/enriching-events/scopes/#configuring-the-scope) for more details. | |
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.
Personal preference here, but maybe it would a good idea to format the method names with parentheses just like when you call them (sendError(), getInstance()). This way, I think devs reading the table will instantly understand these are methods, not parameters or other configuration options.
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.
Just out of curiosity, do you know if errors you can send with the Sentry plugin can/should follow the format described in Error handling, or is it something completely different?
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.
From a quick look at their docs I think you can pass just about anything you'd like captured as an event, you'll just be missing the stacktrace for any non-error objects passed.
|
As discussed with you, Closed in favor of #992, thank you, guys! |
What does it do?
Documents the Sentry plugin within the Developer Documentation.
Why is it needed?
Strapi maintained plugin that is not documented.
Related issue(s)/PR(s)
#13