This repository has been archived by the owner on Apr 12, 2022. It is now read-only.
Metrics : add metrics listener and call its methods in appropriate places #316
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bmarty
suggested changes
Jul 12, 2018
| */ | ||
|
|
||
| public void setMetricsListener(MetricsListener metricsListener) { | ||
| this.mMetricsListener = metricsListener; |
| */ | ||
|
|
||
| public void setMetricsListener(MetricsListener metricsListener) { | ||
| this.mMetricsListener = metricsListener; |
| * | ||
| * @param metricsListener the metrics listener | ||
| */ | ||
|
|
| @@ -430,6 +434,16 @@ public MXDataHandler getDataHandler() { | |||
| return mDataHandler; | |||
| } | |||
|
|
|||
| /** | |||
| * Update the metrics listener mode | |||
| * | ||
| * @param metricsListener the metrics listener | ||
| */ | ||
|
|
| @@ -279,6 +282,11 @@ private void setIsKilled(boolean isKilled) { | |||
| } | |||
| } | |||
|
|
|||
| public MXFileStore setMetricsListener(MetricsListener metricsListener) { | |||
| @@ -279,6 +282,11 @@ private void setIsKilled(boolean isKilled) { | |||
| } | |||
| } | |||
|
|
|||
| public MXFileStore setMetricsListener(MetricsListener metricsListener) { | |||
| this.mMetricsListener = metricsListener; | |||
| @@ -129,6 +132,17 @@ public EventsThread(Context context, EventsRestClient apiClient, EventsThreadLis | |||
| mPowerManager = (PowerManager) context.getSystemService(Context.POWER_SERVICE); | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
same remark than for other classes
| @@ -279,6 +282,11 @@ private void setIsKilled(boolean isKilled) { | |||
| } | |||
| } | |||
|
|
|||
| public MXFileStore setMetricsListener(MetricsListener metricsListener) { | |||
| this.mMetricsListener = metricsListener; | |||
| return this; | |||
There was a problem hiding this comment.
Only this one return this. Consider also for the other similar method, or don't do that
| if (isInitialSyncDone()) { | ||
| // get the latest events asap | ||
| resumeInitialSync(); |
There was a problem hiding this comment.
Thanks for creating subs :)
bmarty
reviewed
Jul 13, 2018
| @@ -529,6 +522,9 @@ public void run() { | |||
| // extract the room states | |||
| mRoomReceiptsToLoad.addAll(listFiles(mStoreRoomsMessagesReceiptsFolderFile.list())); | |||
| mPreloadTime = System.currentTimeMillis() - fLoadTimeT0; | |||
| if (mMetricsListener != null) { | |||
| mMetricsListener.onStorePreloaded(mPreloadTime); | |||
There was a problem hiding this comment.
So this will be be called on another thread, maybe we should specify it on the Javadoc
bmarty
approved these changes
Jul 16, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The metrics are then pushed to analytics tools.
It refers to element-hq/riot-android#2391