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

extend event logging for debugging purposes #22

Merged
merged 6 commits into from
Nov 9, 2017
Merged

extend event logging for debugging purposes #22

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3498613
extend event logging
rclee Nov 7, 2017
0e342b9
Add MMEEventLogger singleton and event types
rclee Nov 8, 2017
c180872
Add DebugTypes to MMEConstants
rclee Nov 9, 2017
f14c3bc
Remove old methods
rclee Nov 9, 2017
1f2f1f9
Fix missing key
rclee Nov 9, 2017
d359ab8
Remove writeEventToLocalDebugLog: from header
rclee Nov 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions MapboxMobileEvents/MMEEventLogger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ NS_ASSUME_NONNULL_BEGIN
+ (void)setEnabled:(BOOL)enabled;
Copy link
Contributor

Choose a reason for hiding this comment

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

We could probably make enabled a property.

Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot that this class has so far only used class methods to invoke logging (e.g. [MMEEventLogger logEvent:debugEvent]). This PR introduces the notion of using an instance of MMEEventLogger in MMEEventsManager.

I think we should change the methods to make them instance based and add a singleton interface so that we can use a [MMEEventLogger sharedLogger] instance everywhere we want log messages to go to the same file with the same date. We can still create 1-off instances for other kinds of logging we may eventually want to do in different files.

+ (void)logEvent:(MMEEvent *)event;

- (void)writeEventToLocalDebugLog:(MMEEvent *)event;
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we could remove this in the header file and make it an implementation detail.


@end

NS_ASSUME_NONNULL_END
48 changes: 48 additions & 0 deletions MapboxMobileEvents/MMEEventLogger.m
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
#import "MMEEventLogger.h"

@interface MMEEventLogger()

@property (nonatomic, copy) NSString *dateForDebugLogFile;
@property (nonatomic) NSFileManager *fileManager;
@property (nonatomic) dispatch_queue_t debugLogSerialQueue;

@end

@implementation MMEEventLogger

static BOOL _enabled;
Expand All @@ -18,4 +26,44 @@ + (void)setEnabled:(BOOL)enabled {
_enabled = enabled;
}

- (void)writeEventToLocalDebugLog:(MMEEvent *)event {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can go ahead and combine this new method and the old logEvent. I like the name logEvent because it is easier to say. We can just assume that if something gets logged it is written to the console and to disk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I definitely like this approach better. Initially I had similar thoughts with combining the two methods. My only concern was potentially having too many logs written locally and that a majority of the debug logs will have the same MMEEventKeyLocalDebug key.

It seems like we'll still need to have other keys for at least flush and push and that would likely result in any MMEEventKeyLocalDebug key/value pairs being unused; unless that information is interesting in some way...

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's go ahead and default to writing "too much" for now as a first pass. We can eventually introduce the notion of log levels to filter what actually gets printed and written.

if (!_enabled) {
return;
}

if (!self.dateForDebugLogFile) {
NSDateFormatter *dateFormatter = [[NSDateFormatter alloc] init];
[dateFormatter setDateFormat:@"yyyy'-'MM'-'dd"];
[dateFormatter setTimeZone:[NSTimeZone systemTimeZone]];
self.dateForDebugLogFile = [dateFormatter stringFromDate:[NSDate date]];
}

if (!self.debugLogSerialQueue) {
NSString *uniqueID = [[NSProcessInfo processInfo] globallyUniqueString];
NSString *appBundleID = [[NSBundle mainBundle] bundleIdentifier];
self.debugLogSerialQueue = dispatch_queue_create([[NSString stringWithFormat:@"%@.%@.events.debugLog", appBundleID, uniqueID] UTF8String], DISPATCH_QUEUE_SERIAL);
}

dispatch_async(self.debugLogSerialQueue, ^{
if ([NSJSONSerialization isValidJSONObject:event]) {
NSData *jsonData = [NSJSONSerialization dataWithJSONObject:event options:NSJSONWritingPrettyPrinted error:nil];

NSString *jsonString = [[NSString alloc] initWithData:jsonData encoding:NSUTF8StringEncoding];
jsonString = [jsonString stringByAppendingString:@",\n"];

NSString *logFilePath = [[NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES) firstObject] stringByAppendingPathComponent:[NSString stringWithFormat:@"telemetry_log-%@.json", self.dateForDebugLogFile]];

NSFileManager *fileManager = [[NSFileManager alloc] init];
if ([fileManager fileExistsAtPath:logFilePath]) {
NSFileHandle *fileHandle = [NSFileHandle fileHandleForWritingAtPath:logFilePath];
[fileHandle seekToEndOfFile];
[fileHandle writeData:[jsonString dataUsingEncoding:NSUTF8StringEncoding]];
} else {
[fileManager createFileAtPath:logFilePath contents:[jsonString dataUsingEncoding:NSUTF8StringEncoding] attributes:@{ NSFileProtectionKey: NSFileProtectionCompleteUntilFirstUserAuthentication }];
}
}
});
}


@end
4 changes: 4 additions & 0 deletions MapboxMobileEvents/MMEEventsManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ @interface MMEEventsManager () <MMELocationManagerDelegate>
@property (nonatomic) MMENSDateWrapper *dateWrapper;
@property (nonatomic, getter=isLocationMetricsEnabled) BOOL locationMetricsEnabled;
@property (nonatomic) UIBackgroundTaskIdentifier backgroundTaskIdentifier;
@property (nonatomic) MMEEventLogger *logger;

@end

Expand Down Expand Up @@ -58,6 +59,7 @@ - (instancetype)init {
_uniqueIdentifer = [[MMEUniqueIdentifier alloc] initWithTimeInterval:_configuration.instanceIdentifierRotationTimeInterval];
_application = [[MMEUIApplicationWrapper alloc] init];
_dateWrapper = [[MMENSDateWrapper alloc] init];
_logger = [[MMEEventLogger alloc] init];
}
return self;
}
Expand Down Expand Up @@ -233,6 +235,7 @@ - (void)sendTurnstileEvent {

MMEEvent *turnstileEvent = [MMEEvent turnstileEventWithAttributes:turnstileEventAttributes];
[self pushDebugEventWithAttributes:@{MMEEventKeyLocalDebugDescription: [NSString stringWithFormat:@"Sending turnstile event: %@", turnstileEvent]}];
[self.logger writeEventToLocalDebugLog:turnstileEvent];

__weak __typeof__(self) weakSelf = self;
[self.apiClient postEvent:turnstileEvent completionHandler:^(NSError * _Nullable error) {
Expand Down Expand Up @@ -360,6 +363,7 @@ - (void)pushEvent:(MMEEvent *)event {

[self.eventQueue addObject:event];
[self pushDebugEventWithAttributes:@{MMEEventKeyLocalDebugDescription: [NSString stringWithFormat:@"Added event to event queue; event queue now has %ld events", (long)self.eventQueue.count]}];
[self.logger writeEventToLocalDebugLog:event];

if (self.eventQueue.count >= self.configuration.eventFlushCountThreshold) {
[self flush];
Expand Down