feat: Integrate Grafana Faro #3139
Conversation
✅ Deploy Preview for design-insights ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for oss-insights ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Adds the react integrations and Faro error boundary around the app and initializes the default tracing/web integrations Signed-off-by: John McBride <[email protected]>
pages/_app.tsx
Outdated
| useEffect(() => { | ||
| if (process.env.NEXT_PUBLIC_FARO_COLLECTOR_URL && process.env.NEXT_PUBLIC_FARO_APP_ENVIRONMENT != "local") { | ||
| initGrafanaFaro(); | ||
| } | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
I'm unsure this is doing what I expect it to do: Faro needs to initialize on the client (not the server) so I figured that a userEffect would get the app to
But it only seems to capture traces/profiles when the App first loads on the /feed and I see this log go through:
Faro
Faro is already registered. Either add instrumentations, transports etc. to the global faro instance or use the "isolate" property
which makes me think that maybe there's a different way to instantiate it - Investigating.
There was a problem hiding this comment.
Ended up going with a useRef which now doesn't seem to cause Faro to be re-initilized
There was a problem hiding this comment.
What do the Faro docs recommend for a React app?
Signed-off-by: John McBride <[email protected]>
pages/_app.tsx
Outdated
| @@ -151,13 +165,15 @@ function MyApp({ Component, pageProps }: ComponentWithPageLayout) { | |||
| <SessionContextProvider supabaseClient={supabaseClient} initialSession={pageProps.initialSession}> | |||
| <PostHogProvider client={posthog}> | |||
| <TipProvider> | |||
| {Component.PageLayout ? ( | |||
| <Component.PageLayout> | |||
| <FaroErrorBoundary> | |||
There was a problem hiding this comment.
I'm not sure this is the right place for the error boundary ... their React docs make it seem like this should wrap a rendered <App /> and I'm not seeing errors flow through when testing.
lib/utils/grafana.ts
Outdated
| // In the future, we may choose to integrate with the router instrumentation to | ||
| // get deeper metrics on matched routes, navigation types, etc. | ||
| // Next/router doesn't seem to be supported which won't give us route metrics. | ||
| // | ||
| // Reference: https://github.com/grafana/faro-web-sdk/tree/main/packages/react | ||
| // | ||
| // router: {} |
There was a problem hiding this comment.
I'm not getting any additional routes besides feed/ in the traces and I think it's because it's due to there note being a router config here (and not really seeing a way for it to support next/router, only React router)
|
After experimenting abit with this, we can definitely get this in and start getting some good latency data. But there are some missing integrations in the library with NextJS that i think make it abit half baked. We can draft this for now and I can watch to see when they land some Next integrations. |
…gated up Signed-off-by: John McBride <[email protected]>
|
Pushing logs and errors through is working 👍🏼 tested it out with a dummy
I say let's get this in and we can keep an eye on their NextJS instrumentation coming through |
| const faroRef = useRef<null | Faro>(null); | ||
|
|
||
| useEffect(() => { | ||
| if ( | ||
| process.env.NEXT_PUBLIC_FARO_COLLECTOR_URL && | ||
| process.env.NEXT_PUBLIC_FARO_APP_ENVIRONMENT != "local" && | ||
| !faroRef.current | ||
| ) { | ||
| faroRef.current = initGrafanaFaro(); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
The useRef seems unnecessary. If all you're trying to do is initialize this for the client-side only, this should suffice.
| const faroRef = useRef<null | Faro>(null); | |
| useEffect(() => { | |
| if ( | |
| process.env.NEXT_PUBLIC_FARO_COLLECTOR_URL && | |
| process.env.NEXT_PUBLIC_FARO_APP_ENVIRONMENT != "local" && | |
| !faroRef.current | |
| ) { | |
| faroRef.current = initGrafanaFaro(); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| if ( | |
| process.env.NEXT_PUBLIC_FARO_COLLECTOR_URL && | |
| process.env.NEXT_PUBLIC_FARO_APP_ENVIRONMENT != "local" | |
| ) { | |
| initGrafanaFaro(); | |
| } | |
| }, []); |
There was a problem hiding this comment.
Yeah, that's what I tried initially but would see this pop up in:
Faro
Faro is already registered. Either add instrumentations, transports etc. to the global faro instance or use the "isolate" property
But that must be due to a re-render that happens? Either way, seems to be fine since the points still get sent to the collector endpoint ok:
|
I'm curious - is the entirety of your app client-side only, or are you doing server-side rendering of components where you're using faro? |
We do have server side rendering. Unfortunately, Faro is not well supported for Next JS yet so we're uncertain the future of this integration. |
Description
Adds the react integrations and Faro error boundary around the app.
Initializes the default tracing/web integrations for Faro to get us some internal metrics and tracing.
Related Tickets & Documents
N/a - related to ongoing observability and performance work
Mobile & Desktop Screenshots/Recordings
Here's a snapshot after running this locally and getting some of those metrics into our Grafana:
Steps to QA
Tier (staff will fill in)
[optional] Are there any post-deployment tasks we need to perform?
Need to update Netlify env vars to use these
[optional] What gif best describes this PR or how it makes you feel?