search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.33k stars 2.76k forks source link

[Analytics] Rename `SWITCH_REPORT` to `OPEN_REPORT` #47142

Open kacper-mikolajczak opened 1 month ago

kacper-mikolajczak commented 1 month ago

Intro

We've been fiddling around with making SWITCH_REPORT more fine-grained in order to get clearer perspective on the timing.

When investigating the timing use cases, most of the times we use it to measure general time taken to open report, not just switching the report.

For better connection between actual use case and definition, we could rename SWITCH_REPORT to OPEN_REPORT.

The changes would look like so: SWITCH_REPORT -> OPEN_REPORT SWITCH_REPORT_THREAD -> OPEN_REPORT_THREAD SWITCH_REPORT_FROM_PREVIEW -> OPEN_REPORT_FROM_PREVIEW

kacper-mikolajczak commented 1 month ago

CC @mountiny @adhorodyski

mountiny commented 1 month ago

Alright I think this is ok but we might have to use both for migration step if we want to change this for the performance too / not only timings

kacper-mikolajczak commented 1 month ago

Right, wondering if we could detach perf timings, constants and analytics from each other, as they are now all under TIMING umbrella:

https://github.com/Expensify/App/blob/a70e9bec6ad64d8d9333d42138d2bb0df09d489c/src/CONST.ts#L965-L989

It might be cumbersome at the beginning but should be worth to do, considering we would like to invest more into analytics in the future.

Also, as the monitoring grows in complexity, do we have a documentation of what each metric represents?

P.S.

There's already a OPEN_REPORT const which I missed in original message. In that case, do you know what is stopping us from migrating SWITCH_REPORT to OPEN_REPORT entirely? (e.g. analytics setup or some external systems)

mountiny commented 1 month ago

I think we might have to make some changes in graphana to rename that

Otherwise all for making it clearer

And I dont think there is much documentation about the metrics yet so it might be worth creating some markdown as you work on it

adhorodyski commented 3 weeks ago

+1 on the docs for this! It'd be only more and more valuable for us to get a common understanding of what exactly each metric represents.

melvin-bot[bot] commented 3 days ago

This issue has not been updated in over 15 days. @mountiny, @kacper-mikolajczak 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!