Skip to content
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

feat(react): Add useProfiler hook #2659

Merged
merged 2 commits into from Jun 9, 2020
Merged

Conversation

AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Jun 8, 2020

Following up #2647

Motivation

It's important that we support all available React paradigms for @sentry/react, and as such, support adding Sentry AM through React hooks

In this PR, we add a new hook useProfiler that operates exactly like how the withProfiler HOC works earlier.

How does it work?

const Link = () => {
  Sentry.useProfiler("Link")
  return (
    <a href="noop-site">A Link</a>
  )
}

function App() {
  Sentry.useProfiler("App")
  return(
    <Component >
      <Link />
    </Component>
  )
}

image

Implementation

The profiler hook leverages the useEffect hook to push a new activity on component mount, and then pop the activity when the component has fully rendered. See the React Hooks API docs for more info how useEffect works.

@@ -36,9 +37,11 @@
"prettier-check": "^2.0.0",
"react": "^16.0.0",
"react-dom": "^16.0.0",
"react-test-renderer": "^16.13.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

Peer dependency for @testing-library/react-hooks

@getsentry-bot
Copy link
Contributor

getsentry-bot commented Jun 8, 2020

Messages
📖

@sentry/browser bundle gzip'ed minified size: (ES5: 17.0742 kB) (ES6: 16.1377 kB)

📖 ✅ TSLint passed

Generated by 🚫 dangerJS against b08049d

@AbhiPrasad AbhiPrasad requested a review from HazAT June 9, 2020 00:27
Copy link
Member

@HazAT HazAT left a comment

Choose a reason for hiding this comment

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

✌️ Changelog and a small nit

Otherwise 🥇

packages/react/src/profiler.tsx Outdated Show resolved Hide resolved
@AbhiPrasad AbhiPrasad requested a review from HazAT June 9, 2020 11:30
Copy link
Member

@HazAT HazAT left a comment

Choose a reason for hiding this comment

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

download

@AbhiPrasad AbhiPrasad merged commit 026dfe9 into master Jun 9, 2020
@AbhiPrasad AbhiPrasad deleted the abhi/feat/profiler-hook branch June 9, 2020 17:33
@mrlubos
Copy link

mrlubos commented Aug 10, 2020

Hey @AbhiPrasad @HazAT, is calling useProfiler() on the main App component enough or should I call it on every component of interest? Do you also have any suggestions for the tracesSampleRate parameter? The documentation mentions to lower it in production, but no baseline suggestions are offered. Thanks!

@AbhiPrasad
Copy link
Member Author

Hey @mrlubos.

  1. We recommend calling useProfiler on any component you are interested in profiling. If you look at Sentry's usage of the Profiler, https://github.com/getsentry/sentry/search?q=withProfiler&unscoped_q=withProfiler, we typically add it to big stateful component that are important to pageload/navigation, or to thing like loading components. This gives us a better idea of what components are mounted/unmounted while a page is loading (also gives a good idea of if you skeleton states are actually working and when components mount after data gets loaded).

  2. The reason we recommend turning down tracesSampleRate is that for really big applications, a high sample rate could cause you to heavily exceed your quota. Since quota/usage varies heavily by Sentry users, we don't really have any baseline suggestions.

For other questions, we recommend opening an issue on Github or asking on our forums about these kind of questions. That way, other people can easily search and find the answer (ask once, answer for many 🚀)

@mrlubos
Copy link

mrlubos commented Aug 10, 2020

Got it, thank you so much @AbhiPrasad!

@mrlubos
Copy link

mrlubos commented Aug 10, 2020

FYI @AbhiPrasad, the reason I ended up here is because I couldn’t find a reference for React Hooks in the Sentry documentation, so I searched this repository 😬

@AbhiPrasad
Copy link
Member Author

Yes, that was on purpose, we wanted to do some more testing with it before we made it fully public in documentation. It's still fully functional though, hence why we left it out for people to consume if they wanted it enough.

Docs coming soon though!

@skrywus
Copy link

skrywus commented Sep 23, 2020

Will that work for React Native?
I am trying to use it but nothing is happening...

@HazAT
Copy link
Member

HazAT commented Sep 24, 2020

@skrywus We are working on it now, we'll let you know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants