Repository navigation
Improve Nextjs tracing #5505
Description
Activity
- linked a pull request that will close this issueref(nextjs): Wrap server-side data-fetching methods during build #5503
on Aug 1, 2022 - removed a link to a pull requestref(nextjs): Wrap server-side data-fetching methods during build #5503
on Aug 2, 2022 - linked a pull request that will close this issueref(nextjs): Use loader to set `RewriteFrames` helper value #5445
on Aug 12, 2022 - removed a link to a pull requestref(nextjs): Use loader to set `RewriteFrames` helper value #5445
on Aug 12, 2022 - changed the title
[-]Improve Nextjs parameterization[/-][+]Improve Nextjs tracing[/+]on Aug 12, 2022 - added a commit that references this issue
on Aug 19, 2022 - added a commit that references this issue
on Aug 22, 2022 - added a commit that references this issue
on Sep 19, 2022 As for
⚠️ What happens for pages with no data fetchers? Should_apphave a different helper? (no name for transaction)Thinking about what was said in the last all-hands the performance product is something that should hint towards issues that are fixable via changing code. Since pages with no data fetchers are static, the only way to make serving them faster is via infrastructure (faster machines/network/CDN). Most of the time infrastructure isn't really a dev concern (I know that sometimes it is but I believe it's not the main persona we're targeting with the product).
Additionally, I believe the frontend-span we have for the request should be enough to give users a sense of what's going on. Since there isn't any fancy computation or fetching going on in the backend when a static page is requested, a backend transaction isn't going to be very useful here, since it's always going to consist of only a singular span.
Considering the above, I suggest we mark this as non-critical for now. Do you have a different opinion on this? @lobsterkatie
- added a commit that references this issue
on Sep 26, 2022 I'm fine to leave it for now. I figured out part of what I was worried about (yes, pages with dynamic paths but without data fetchers turn into JS rather than just static HTML, but it's JS which runs on the front end, not the back end, so don't have to be dealt with here. The rest of what made me add that TODO (I was seeing transactions show up with undefined names in my testing) isn't something I can now easily replicate. If it's an issue it'll come up again.
Reacted by Luca ForstnerThis feature might have caused a regression: #5998
Let's consider this resolved for now.
Goal: Improve tracing in nextjs SDK - make parameterization more reliable, trace data fetchers, trace page requests for folks on Vercel
TL;DR Plan:
Effects:
instrumentServer(brittle monkeypatching, only works off of vercel)Open Questions:
Tasks
Prework
General prework necessary to improve the Next.js performance experience as a whole.
RewriteFrames(ref(nextjs): Use loader to setRewriteFrameshelper value #5445)getServerSideProps,getStaticPropsandgetStaticPathsduring build-time (ref(nextjs): Wrap server-side data-fetching methods during build #5503)getStaticPaths(fix(nextjs): Remove experimental wrapping ofgetStaticPaths#5561)Transaction Name Parameterization
getServerSidePropsandgetStaticProps(feat(nextjs): Add spans and route parameterization in data fetching wrappers #5564)getInitialPropsfor normal pages (not_app.js,_error.js,_document.js) (feat(nextjs): Create spans and route parameterization in server-sidegetInitialProps#5587)getInitialPropsin_app.js,_error.jsand_document.js(feat(nextjs): Instrument server-sidegetInitialPropsof_app,_documentand_error#5604)Connected Traces
redirectandnotFoundresponses fromgetServerSideProps(alsogetStaticProps?)) (feat(nextjs): Connect trace between data-fetching methods and pageload #5655)isPrefetchRequest: booleantag to errors and do the one or more of the following to transactions 1. different name 2. data field 3. different op) (there is apurpose: prefetchheader in those requests)getServerSidePropspage that throws: Transaction name is/_errorbut DSC contains route ofgetServerSideProps. (SeeTODOcomment in src/performance/client.ts) ([nextjs] Fix transaction name getting lost when hitting_errorpage #5826)Serverside Transaction improvements
Currently, for mysterious reasons, in some circumstances no server-side non-API-route transactions are started for Next.js apps. (This is over and above the known limitation of non-API-route tracing not working on Vercel.) The following changes will enable serverside transactions in both those mysterious situations and on Vercel.
getInitialPropsandgetServerSideProps(feat(nextjs): Create transactions ingetInitialPropsandgetServerSideProps#5593)RequestDataintegration to work for error events (ref(nextjs): UseRequestDataintegration for errors #5729)getStaticProps(Wrapper exists but we don't have access to the request. Is this solvable? Do we even care, given that this generally runs in the background?)withSentryinto a wrapper/helper)res.sendsimilar to how we do it inwithSentry(ref(nextjs): Use flush code fromwithSentryin all backend wrappers #5814)Figure out how to propagate scope between data-fetching functions in the same transaction (ordocument scope-propagation limitations)instrumentServer.ts?statusto transactions and spans and update to "internal_error" on error (feat(nextjs): Add status to data-fetcher spans #5777)What happens for pages with no data fetchers? Should_apphave a different helper? (no name for transaction)Folow-up/Cleanup/Polishing
package.json(chore(nextjs): Remove obsoletedataFetchersloader #5713)RequestDataintegration everywhereCheck if new model works with CJS, if no, support people that use CJSwithSentryinto the fold (consolidate helpers)Documentation
RequestDataintegration and optionsMisc (stretch goals)
getInitialPropswhen run client-sideSteps for Beta
autoInstrumentServerFunctionssentry-docs#5542)withSentryfunction inwith-sentryexample vercel/next.js#41326)Steps for GA (Planned date: October 5, 2022)
autoInstrumentServerFunctionsper default #5919)withSentry(Remove references to manually usewithSentryfrom Next.js docs sentry-docs#5543)Follow ups
Undone tasks from above have been sorted into:
withSentryand possiblyinstrumentServer(JS SDK v8 Deprecations List #5194)_errorbetter ([nextjs] Fix transaction name getting lost when hitting_errorpage #5826)RequestDataintegration (RequestData integration prework, work, and postwork #5756)