Skip to content
This repository has been archived by the owner on Dec 19, 2024. It is now read-only.

Sentry: Turn on auto instrument server funcs #1561

Merged
merged 4 commits into from
Apr 11, 2023

Conversation

masonmcelvain
Copy link
Contributor

@masonmcelvain masonmcelvain commented Apr 11, 2023

I'm not sure why this was turned off, and it doesn't seem to make any difference with it on, but let's turn it back on.

https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#configure-server-side-auto-instrumentation

automatically instrument API routes and server-side Next.js data fetching methods with error and performance monitoring.

qa_req 0

CC @danielbeardsley

Connects #1491

@vercel
Copy link

vercel bot commented Apr 11, 2023

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

Name Status Preview Comments Updated (UTC)
react-commerce ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2023 8:37pm
react-commerce-prod ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2023 8:37pm

@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis for @ifixit/commerce-frontend

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis for @ifixit/commerce-frontend

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@danielbeardsley
Copy link
Member

CR 👍

I think this is the right thing, but it's hard for me to believe we aren't instrumenting with sentry at all except for the places where we do it explicitly.

deploy_block 👍 on looking at those explicit places and ensuring we won't be double-reporting.

Sentry's server function auto-instrumentation should capture these errors for us.
@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis for @ifixit/commerce-frontend

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@masonmcelvain
Copy link
Contributor Author

Great suggestion! I did a quick audit of the places we explicitly use sentry, and I think these were the only potential double-captures. Our applySentryFetchMiddleware() middleware doesn't look like it will double report.

un_deploy_block ✌🏻

Copy link
Member

@danielbeardsley danielbeardsley left a comment

Choose a reason for hiding this comment

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

CR 👍

Nice work!

@deltuh-vee deltuh-vee merged commit 76203fa into main Apr 11, 2023
@deltuh-vee deltuh-vee deleted the turn-on-sentry-auto-instrument branch April 11, 2023 23:02
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.

3 participants