-
Notifications
You must be signed in to change notification settings - Fork 211
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
Add Sentry Nuxt module #5279
Add Sentry Nuxt module #5279
Conversation
44cc88f
to
3ef8856
Compare
Latest k6 run output1
Footnotes
|
4401e95
to
536f23a
Compare
Full-stack documentation: https://docs.openverse.org/_preview/5279 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
536f23a
to
56bbb8f
Compare
56bbb8f
to
be2f45f
Compare
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.
I have a couple of questions, for my own understanding. Code-wise, this PR looks good to me, and it's nice that we can make better use of Sentry with the plugin. #4907 is another good candidate for improving Sentry functionality.
frontend/package.json
Outdated
"@tailwindcss/typography": "^0.5.13", | ||
"@vueuse/core": "^12.0.0", | ||
"@wordpress/is-shallow-equal": "^5.3.0", | ||
"async-mutex": "^0.5.0", | ||
"axios": "^1.7.2", | ||
"axios-mock-adapter": "^1.22.0", | ||
"clipboard": "^2.0.11", | ||
"dotenv": "^16.4.7", |
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.
This surprised me because I had read that Nuxt has dotenv
built-in.
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, we already had it in the lock file due to Nuxt. I added it because it was mentioned in the Nuxt Sentry module setup docs, but removed it after your comment.
# Sentry Config File | ||
.env.sentry-build-plugin |
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.
What is the reason for this being named .env.sentry-build-plugin
? Could it be similar to the env setup for other other packages so that we can reuse the existing workflow (.env
file created from env.template
using the env
recipe 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.
This was added by the Sentry module wizard. I added the env variable to the GitHub repository, so it's not really necessary anymore.
Fixes
Fixes #5270 by @obulat
Description
This PR replaces our bespoke implementation of the Sentry integration with the official Sentry Nuxt module.
Testing Instructions
Run
ov env NUXT_PUBLIC_SENTRY_DSN=somevalue
, adddebug=true
to the Sentry config files (server and client) and verify that Sentry is being initialized.Checklist
Update index.md
).main
) or a parent feature branch.ov just catalog/generate-docs
for catalogPRs) or the media properties generator (
ov just catalog/generate-docs media-props
for the catalog or
ov just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin