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

Add better performance metrics to Grafana #11997

Closed
roryabraham opened this issue Oct 19, 2022 · 14 comments
Closed

Add better performance metrics to Grafana #11997

roryabraham opened this issue Oct 19, 2022 · 14 comments
Assignees
Labels
Engineering Improvement Item broken or needs improvement. Monthly KSv2 NewFeature Something to build that is a new item. Not a priority Reviewing Has a PR in review

Comments

@roryabraham
Copy link
Contributor

roryabraham commented Oct 19, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


See this predesign for some additional context.

Problem

We've got a sort of wonky set of performance metrics in Grafana right now. For example, we measure "Initial Homepage Render" as the time between AuthScreens.constructor being called and AuthScreens.componentDidMount being called. It's not clear what this metric really means or if it's indicative of anything real to the user.

As a result, the performance metrics we can measure

Solution

Build out a better, more accurate set of Performance metrics and start tracking them with Grafana. This could include:

Others? Of course, all these metrics should be tracked on a per-platform basis.

View all open jobs on GitHub

@roryabraham roryabraham added Engineering Weekly KSv2 Improvement Item broken or needs improvement. Planning Changes still in the thought process labels Oct 19, 2022
@roryabraham roryabraham self-assigned this Oct 19, 2022
@puneetlath puneetlath added the Bug Something is broken. Auto assigns a BugZero manager. label Oct 19, 2022
@roryabraham roryabraham added NewFeature Something to build that is a new item. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Oct 19, 2022
@roryabraham
Copy link
Contributor Author

@hannojg Seems like maybe a good fit if you're interested?

@roryabraham
Copy link
Contributor Author

I also think we agreed to nix Frustration Index because it's more complex and less actionable than other metrics here

@roryabraham
Copy link
Contributor Author

Beyond just sending these metrics to a remote server, I think it would also be helpful to log these metrics locally to make it easier to benchmark individual PRs.

@hannojg
Copy link
Contributor

hannojg commented Oct 20, 2022

Oh yeah, very interested, will add this to my tasks 💪

@roryabraham
Copy link
Contributor Author

Taking this out of planning. It would be a somewhat big lift for us to make graphs.expensify.com public, so we should maybe just publish these metrics separately than our other Grafana graphs

@melvin-bot melvin-bot bot removed the Overdue label Oct 28, 2022
@melvin-bot melvin-bot bot added the Overdue label Nov 7, 2022
@roryabraham
Copy link
Contributor Author

Carved this out here as what I think should be one of @hannojg's top priorities. @hannojg if you're blocked on #11711, I'd recommend spending some time on this in the meantime.

@melvin-bot melvin-bot bot removed the Overdue label Nov 9, 2022
@melvin-bot melvin-bot bot added the Overdue label Nov 17, 2022
@roryabraham
Copy link
Contributor Author

@hannojg should we think about handing this off to someone else?

@melvin-bot melvin-bot bot removed the Overdue label Nov 19, 2022
@hannojg
Copy link
Contributor

hannojg commented Nov 21, 2022

We could, what is the priority on this one? From what I understand you are trying to heavily focus on bugs and performance issues right now, so I was prioritizing that so far (but it's still on my todo list)

@JmillsExpensify
Copy link

I posted a discussion in Slack on the priorities, though didn't add this one. I kind of think that helping any useful tests for our CI initiative is higher value – mainly because we are getting close to launching it but we only have one test at the moment.

@roryabraham
Copy link
Contributor Author

I would say this is currently lower priority than bugs, but still quite valuable in the long-term because it helps give us a better objective framework to talk about performance, particularly for those of us that aren't as well-versed in the various performance testing and profiling tools there are out there for testing React/React Native/web/mobile apps.

@roryabraham
Copy link
Contributor Author

Unassigning myself from this issue, because we completed the predesign and I'm not going to implement it myself.

@roryabraham roryabraham removed their assignment Nov 28, 2022
@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Dec 13, 2022
@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Jan 6, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 6, 2023

This issue has not been updated in over 15 days. @hannojg eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@hannojg
Copy link
Contributor

hannojg commented Jan 6, 2023

I just created these two PRs:

With these changes we will send events with environment information to Grafana, which is super important to separate.
In addition to that it makes the TTI calculation more accurate.

In general we found that TTI is the main measurement that we want to take. Any other metrics aren't really appealing for mobile platforms.
There might be special performance for the web platforms.
However, the measurements we send to Grafana (TTIs for various flows), are mainly there to help us catch performance issues within the staging/production app (e.g. after new slow code got merged)

@melvin-bot melvin-bot bot closed this as completed Mar 6, 2023
@MelvinBot
Copy link

@hannojg, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Improvement Item broken or needs improvement. Monthly KSv2 NewFeature Something to build that is a new item. Not a priority Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

5 participants