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.

Sign up for GitHub

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

Jump to bottom

Fixes #4335: Addition of support for uploading performance metric logs #4399

Merged
merged 187 commits into from
Sep 5, 2022
Merged

Fixes #4335: Addition of support for uploading performance metric logs #4399

merged 187 commits into from
Sep 5, 2022

Conversation

Sarthak2601
Copy link
Contributor

@Sarthak2601 Sarthak2601 commented Jun 21, 2022 •
edited
Loading

Explanation

Fixes #4335

  • Introduces support for uploading performance metric logs to the remote service.
  • The PR modifies files related to work manager to add and queue work request for uploading metric logs. It also modifies files related to unpacking and repacking of event logs into bundles for logging to firebase.
  • Added tests to verify these functionalities as well.

Essential Checklist

  • 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: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

@Sarthak2601
qualifiers and constants for metric record and upload times
ac1cb65
@Sarthak2601
comments
ee7df23
@Sarthak2601
dagger provides for flags
0adaa4b
@Sarthak2601
rename to enablePerformanceMetricCollection
fee9bd6
@Sarthak2601
initial proto
d3f746f
@Sarthak2601
nit fixes
37a22c5
@Sarthak2601
Merge branch 'performance-metrics-platform-params' into performance-m…
c33d75f 
…etrics-proto
@Sarthak2601
nit fixes
2b3cc6f
@Sarthak2601
comments.
8d7ace6
@Sarthak2601
nits
49e2a9a
@Sarthak2601
nit
c9a1840
@Sarthak2601
Merge branch 'performance-metrics-platform-params' into performance-m…
5a28ad7 
…etrics-proto
@Sarthak2601
updates.
7dc2fa4
@Sarthak2601
updates.
c515e7d
@Sarthak2601
nits.
9e98a40
@Sarthak2601
Merge branch 'develop' into performance-metrics-platform-params
c7d75e2
@Sarthak2601
Merge branch 'performance-metrics-platform-params' into performance-m…
3a8929d 
…etrics-proto
@Sarthak2601
metric log inclusion.
1ae4e17
@Sarthak2601
base setup.
16603dd
@Sarthak2601
Merge branch 'develop' into performance-metrics-platform-params
e06979e
@Sarthak2601
name correction.
0d2a668
@Sarthak2601
Merge branch 'performance-metrics-platform-params' into performance-m…
598a6ea 
…etrics-proto
@Sarthak2601
nits.
52e12eb
@Sarthak2601
Merge branch 'performance-metrics-proto' into performance-metrics-log…
d992388 
…-creation
@Sarthak2601
nits.
dae5990
@Sarthak2601
storage comment
f2112a2
@Sarthak2601
pss comment
0b1181f
@Sarthak2601
Merge branch 'develop' into performance-metrics-proto
694a987
@Sarthak2601
network usage comment.
dbe1740
@Sarthak2601
network usage comment - part 2.
c41c2e5
BenHenning
BenHenning reviewed Aug 28, 2022
View reviewed changes
Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Sarthak2601! The PR generally LGTM. Will wait to approve after the base PR is merged and this one is updated to be based on develop.

@BenHenning BenHenning assigned Sarthak2601 and unassigned BenHenning Aug 28, 2022
Base automatically changed from performance-metrics-log-creation to develop September 2, 2022 06:32
@Sarthak2601 Sarthak2601 requested a review from rt4914 as a code owner September 2, 2022 06:32
@Sarthak2601
Merge branch 'develop' into performance-metric-log-upload
28ce0b5 
# Conflicts:
#	domain/src/main/java/org/oppia/android/domain/oppialogger/loguploader/LogReportWorkManagerInitializer.kt
#	domain/src/test/java/org/oppia/android/domain/oppialogger/loguploader/LogReportWorkManagerInitializerTest.kt
#	utility/src/main/java/org/oppia/android/util/logging/firebase/FirebaseEventLogger.kt
@Sarthak2601 Sarthak2601 assigned rt4914 and BenHenning and unassigned Sarthak2601 Sep 2, 2022
BenHenning
BenHenning approved these changes Sep 5, 2022
View reviewed changes
Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Sarthak2601! LGTM. Feel free to merge whenever you need to.

@BenHenning BenHenning assigned Sarthak2601 and unassigned rt4914 and BenHenning Sep 5, 2022
@BenHenning
Copy link
Member

(Unassigning Rajat since this PR doesn't need his review).

@BenHenning BenHenning removed the request for review from rt4914 September 5, 2022 08:14
@oppiabot oppiabot bot added the PR: LGTM label Sep 5, 2022
@oppiabot
Copy link

oppiabot bot commented Sep 5, 2022

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!

@Sarthak2601 Sarthak2601 merged commit 3839846 into develop Sep 5, 2022
@Sarthak2601 Sarthak2601 deleted the performance-metric-log-upload branch September 5, 2022 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenHenning BenHenning BenHenning approved these changes

Assignees

@Sarthak2601 Sarthak2601

Labels
PR: LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add log upload support for performance metrics collection
3 participants
@Sarthak2601 @BenHenning @rt4914