Skip to content

Improve Nextjs tracing #5505

Description

@smeubank

Goal: Improve tracing in nextjs SDK - make parameterization more reliable, trace data fetchers, trace page requests for folks on Vercel

TL;DR Plan:

  • get page path at build time
  • use loader to inject into page as global variable
  • use loader to wrap canonical functions
    • getStaticPaths - start transaction, add span
    • getStaticProps - start or continue transaction, add span
    • getServerSideProps - start transaction, add span
    • getInitialProps - needs investigation, can run on client
  • use loader to wrap _app or _document
    • start or continue transaction, add span, finish transaction

Effects:

  • Parameterized name is known when transaction is started
  • Wrapping is at page level, not server level, so works on and off of Vercel (currently tracing for page requests only works off of Vercel)
  • Spans for data-fetching functions (none now)
  • Hopefully lets us eventually retire instrumentServer (brittle monkeypatching, only works off of vercel)

Open Questions:

  • How to deal with domains when action happens in multiple functions at multiple times?
  • What about background/pre-load/data-only requests?
  • How to communicate why transactions may get marginally shorter?
  • How to grab request data if no GSSP?

Tasks

⚠️ ... Required for making changes non-experimental

Prework

General prework necessary to improve the Next.js performance experience as a whole.

Transaction Name Parameterization

Connected Traces

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.

Folow-up/Cleanup/Polishing

  • ⚠️ Remove MVP loader code and remove its dependencies from package.json (chore(nextjs): Remove obsolete dataFetchers loader #5713)
  • Use RequestData integration everywhere
  • Check if new model works with CJS, if no, support people that use CJS
  • ⚠️ Check Webpack 4 support of the loaders we created (our integration tests run under webpack 4 just fine, so I think we're good here)
  • Bring withSentry into the fold (consolidate helpers)
  • Nudge users to stop wrapping API routes if using auto-wrapping (once auto-wrapping includes API routes)
  • Factor more common parts out of wrappers if possible

Documentation

  • Document new RequestData integration and options
  • Document auto-wrapping

Misc (stretch goals)

  • Have spans for getInitialProps when run client-side
  • Add instrumentation for how long server-side rendering takes
  • Investigate how it would be possible to support custom servers
  • Can common route-handling work be consolidated into helper functions across frameworks, so we're not always reinventing the wheel?

Steps for Beta

Steps for GA (Planned date: October 5, 2022)


Follow ups

Undone tasks from above have been sorted into:

Activity

  1. changed the title [-]Improve Nextjs parameterization[/-] [+]Improve Nextjs tracing[/+] on Aug 12, 2022
  2. lforst commented on Sep 20, 2022

    @lforst
    Contributor

    As for

    ⚠️ What happens for pages with no data fetchers? Should _app have 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

  3. lobsterkatie commented on Sep 26, 2022

    @lobsterkatie
    Member

    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.

  4. kachkaev commented on Oct 19, 2022

    @kachkaev

    This feature might have caused a regression: #5998

  5. lforst commented on Nov 30, 2022

    @lforst
    Contributor

    Let's consider this resolved for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions