oppia-android icon indicating copy to clipboard operation
oppia-android copied to clipboard

Fixes #4340: Introduces Performance Metrics logging

Open Sarthak2601 opened this issue 2 years ago • 1 comments

Explanation

Fixes #4340 Introduces performance metrics logging at application startup and via background workers.

Things to note:

  1. Removal of CurrentScreen enum: I've replaced the current screen enum from the proto definition to a simple string value. The string value helps us propagate the actual activity class name to Firebase (Firebase does the same in its automatic logging). I think it's a better and more maintainable approach regarding UI monitoring as we'd have to regularly update the enum, bundleCreator + tests with new screens. Another reason to go forward with this was non availability of app-level activity class dependencies in the new domain-level observer. To accommodate this, I initially thought of moving the observer to the app module but later decided against it as we wouldn't have been able to keep track of the current screen from the worker in that case.

  2. Application startup logging at one place: I've moved apk size logging and storage usage logging from PerformanceMetricsLogger to the new ActivityLifecycleObserver. This means that the new observer is logging all app startup metrics from one place.

Essential Checklist

  • [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • [x] Any changes to scripts/assets files have their rationale included in the PR explanation.
  • [x] The PR follows the style guide.
  • [x] The PR does not contain any unnecessary code changes from Android Studio (reference).
  • [x] The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • [x] The PR is assigned to the appropriate reviewers (reference).

Sarthak2601 avatar Jul 23 '22 20:07 Sarthak2601

Hi @Sarthak2601, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

oppiabot[bot] avatar Aug 03 '22 03:08 oppiabot[bot]

Note: I'm waiting on one of the comments regarding intent key to be resolved, post which I'll implement this approach for the rest of the implementation.

cc: @BenHenning

Sarthak2601 avatar Aug 18 '22 21:08 Sarthak2601

Unassigning @rt4914 since they have already approved the PR.

oppiabot[bot] avatar Aug 24 '22 17:08 oppiabot[bot]

Hi @Sarthak2601, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

oppiabot[bot] avatar Sep 19 '22 21:09 oppiabot[bot]