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

[HOLD for payment 2023-02-07] Make app start TTI more accurate #13996

Closed
6 tasks done
hannojg opened this issue Jan 5, 2023 · 20 comments
Closed
6 tasks done

[HOLD for payment 2023-02-07] Make app start TTI more accurate #13996

hannojg opened this issue Jan 5, 2023 · 20 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@hannojg
Copy link
Contributor

hannojg commented Jan 5, 2023

Action Performed:

  • When the app starts we measure the time it takes until the SidebarLinks are rendered, known as "App start TTI".
    This measurement is reported to Grafana. Currently any waiting time due to waiting on network data is included in the TTI. This PR changes that, and will only report the TTI when we render the SidebarLinks with cached data on the first render.
  • Currently when reporting to grafana no information about the environment is included, meaning data from staging and production is mixed. This PR also adds env info to the grafana logging.

This is an effort towards: #11997

Expected Result:

  • Only report TTI from cached render results (we want to know if we introduced performance regression issues in the code, not whether the API is slow to respond)
  • Events reported to grafana include environment information

Actual Result:

/

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Yes they can, this is just for internal performance monitoring.

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.2.48-2
Reproducible in staging?: N/A
Reproducible in production?: N/A
Email or phone of affected tester (no customers): N/A
Logs: N/A
Notes/Photos/Videos: N/A
Expensify/Expensify Issue URL: N/A
Issue reported by: @hannojg
Slack conversation: None, but this issue #11997

View all open jobs on GitHub

@hannojg hannojg added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Jan 5, 2023
@melvin-bot melvin-bot bot closed this as completed Jan 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 5, 2023

📣 @hannojg! 📣
Please report bugs or suggest features in the #expensify-open-source Slack channel, don't directly open issues in this repo!
Instructions here to join the channel 📖

@melvin-bot
Copy link

melvin-bot bot commented Jan 5, 2023

Triggered auto assignment to @abekkala (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@AndrewGable AndrewGable reopened this Jan 5, 2023
@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Jan 5, 2023
@hannojg
Copy link
Contributor Author

hannojg commented Jan 5, 2023

I just created this PR for the issue: #13997

@abekkala
Copy link
Contributor

abekkala commented Jan 5, 2023

@hannojg did you report this bug in the #expensify-open-source Slack channel? (As you don't directly open issues in this repo)

If so, can you link to that slack conversation please?

@hannojg
Copy link
Contributor Author

hannojg commented Jan 5, 2023

Oh yeah, sorry, I didn't 😅 I am from Margelo and this issue was created as part of Margelos collaboration working week in SF with Expensify.

@AndrewGable
Copy link
Contributor

Confirmed @abekkala - Working with @hannojg on this one. They get paid hourly so no need to worry about upwork.

@melvin-bot
Copy link

melvin-bot bot commented Jan 13, 2023

@AndrewGable, @abekkala, @hannojg Whoops! This issue is 2 days overdue. Let's get this updated quick!

@hannojg
Copy link
Contributor Author

hannojg commented Jan 16, 2023

The PR is still in review @MelvinBot

@AndrewGable AndrewGable added Weekly KSv2 and removed Daily KSv2 labels Jan 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 30, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 31, 2023
@melvin-bot melvin-bot bot changed the title Make app start TTI more accurate [HOLD for payment 2023-02-07] Make app start TTI more accurate Jan 31, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 31, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 31, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot
Copy link

melvin-bot bot commented Jan 31, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.62-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-02-07. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot
Copy link

melvin-bot bot commented Jan 31, 2023

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@AndrewGable] The PR that introduced the bug has been identified. Link to the PR:
  • [@AndrewGable] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@AndrewGable] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@abekkala] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

@abekkala
Copy link
Contributor

abekkala commented Feb 1, 2023

@AndrewGable I just want to confirm there is still no action needed from me (BugZero) as payments aren't made via Upwork and I don't need to add/edit a Test Rail?

@AndrewGable
Copy link
Contributor

Confirmed!

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Feb 6, 2023
@AndrewGable AndrewGable removed the Awaiting Payment Auto-added when associated PR is deployed to production label Feb 9, 2023
@melvin-bot melvin-bot bot removed the Overdue label Feb 9, 2023
@aimane-chnaif
Copy link
Contributor

C+ Payment is pending for internal PR review

@AndrewGable AndrewGable added the Awaiting Payment Auto-added when associated PR is deployed to production label Feb 9, 2023
@AndrewGable AndrewGable reopened this Feb 9, 2023
@AndrewGable
Copy link
Contributor

@abekkala - Can you please handle? Thanks!

@abekkala
Copy link
Contributor

@AndrewGable LOL so I do need to make a payment via Upwork?
Who is the C+ on this that needs payment? I'm not sure who was considered Margelo and who needs to be paid via Upwork.

@aimane-chnaif
Copy link
Contributor

I am C+ and @hannojg is from Margelo. I am only the one to be paid via upwork

@abekkala
Copy link
Contributor

abekkala commented Feb 10, 2023

@aimane-chnaif I've sent you a contract/offer via Upwork.
Once accepted I'll send payment :)

@abekkala
Copy link
Contributor

Payment has been sent, the contract has been completed, and the Upwork job post has been closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
None yet
Development

No branches or pull requests

4 participants