ios: flush stats on background / termination#709
Closed
rebello95 wants to merge 1 commit intoexpose-flushfrom
Closed
ios: flush stats on background / termination#709rebello95 wants to merge 1 commit intoexpose-flushfrom
rebello95 wants to merge 1 commit intoexpose-flushfrom
Conversation
Builds on #707 by consuming this interface to flush stats when the app is backgrounded or terminated. This will help ensure that stats are sent to the server when possible before the app potentially loses its in-memory stats cache. Part of #573. Signed-off-by: Michael Rebello <[email protected]>
Contributor
Author
|
Going with a bit of a different approach, so ignore this PR for now 😄 |
goaway
reviewed
Mar 2, 2020
| + (void)startFlushingStatsOnLifecycleChanges { | ||
| static dispatch_once_t lifecycleMonitoringStarted; | ||
| dispatch_once(&lifecycleMonitoringStarted, ^{ | ||
| [self startObservingLifecycleNotifications]; |
Contributor
There was a problem hiding this comment.
nit: I feel sort of like startObservingLifecycleNotification should be the public method name, since the fact that we're flushing stats on them seems more like an implementation detail.
goaway
reviewed
Mar 2, 2020
| } | ||
|
|
||
| + (void)lifecycleDidChangeWithNotification:(NSNotification *)notification { | ||
| NSLog(@"[Envoy] triggering stats flush (%@)", notification.name); |
Contributor
There was a problem hiding this comment.
Is this meant to be here? (Do we generally utilize NSLog in our platform layer? Is it normal for third-part libraries to do so?)
Contributor
Author
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment.
Sounds good, and thanks for opening the issue.
Contributor
Author
|
Replacing with #717 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Builds on #707 by consuming this interface to flush stats when the app is backgrounded or terminated. This will help ensure that stats are sent to the server when possible before the app potentially loses its in-memory stats cache.
Part of #573.
Signed-off-by: Michael Rebello [email protected]