Add FXIOS-4785 [v106] Adds metrics to track number of history visits #11634
Conversation
|
Data review request in #11635 |
lmarceau
changed the title
|
Build is green here |
|
@nbhasin2 I think this is ready for review, can you think of anything going wrong if we run a query like this? it's asynchronous and runs only once on initialization - I couldn't find another easy spot to collect this telemetry 😕 |
|
I also want to cc @travis79 here - No need for you to take time to review the swift changes, but I wanted to make sure that adding an additional |
|
I am going to close and re-open the issue as BR is being weird for this PR |
| @@ -157,6 +169,24 @@ open class SQLiteHistory { | |||
| let args: Args = [] | |||
| return db.runQueryConcurrently(sql, args: args, factory: SQLiteHistory.iconHistoryColumnFactory) | |||
| } | |||
|
|
|||
| public func countVisits(callback: @escaping (Int?) -> Void) { | |||
There was a problem hiding this comment.
@tarikeshaq is this a method we plan on keeping for long or the idea is to get rid of it once you have the required data?
There was a problem hiding this comment.
The plan is to keep the data for long so we can monitor the sizes of databases and compare this implementation and the application-services one. But we will be getting rid of this method when we swap to using application services history
That said, also with the application services history we will want to keep track of the number of visits, but that will probably happen in Rust as opposed to here in the app
There was a problem hiding this comment.
@tarikeshaq good to merge, let me know if you have proper permission
There was a problem hiding this comment.
@nbhasin2 thanks a ton!!! unfortunely I don't :( Is there a way I can get the permission to land a PR once it's approved?
FXIOS-4785 #11635
Ahead of migrating the history storage to use the application-services places component, we want to track down the number of history visits in a user's database. This adds those metrics.
## Keeping as a draft until I get data reviewI'll probably want a quick glean eyes on this as well to make sure it's OK to add the new metrics.yaml file