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 2 commits
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/MMEConstants.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ extern NSString * const MMEEventTypeMapTap;
extern NSString * const MMEEventTypeMapDragEnd;
extern NSString * const MMEEventTypeLocation;
extern NSString * const MMEEventTypeLocalDebug;
extern NSString * const MMEEventTypeFlush;
extern NSString * const MMEEventTypePush;

// Gestures
extern NSString * const MMEEventGestureSingleTap;
Expand Down
2 changes: 2 additions & 0 deletions MapboxMobileEvents/MMEConstants.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
NSString * const MMEEventTypeMapDragEnd = @"map.dragend";
NSString * const MMEEventTypeLocation = @"location";
NSString * const MMEEventTypeLocalDebug = @"debug";
NSString * const MMEEventTypeFlush = @"flush";
NSString * const MMEEventTypePush = @"push";

NSString * const MMEEventGestureSingleTap = @"SingleTap";
NSString * const MMEEventGestureDoubleTap = @"DoubleTap";
Expand Down
2 changes: 2 additions & 0 deletions MapboxMobileEvents/MMEEvent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@
+ (instancetype)mapDragEndEventWithDateString:(NSString *)dateString attributes:(NSDictionary *)attributes;
+ (instancetype)navigationEventWithName:(NSString *)name attributes:(NSDictionary *)attributes;
+ (instancetype)debugEventWithAttributes:(NSDictionary *)attributes;
+ (instancetype)pushEventWithAttributes:(NSDictionary *)attributes;
+ (instancetype)flushEventWithAttributes:(NSDictionary *)attributes;

@end
14 changes: 14 additions & 0 deletions MapboxMobileEvents/MMEEvent.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@

@implementation MMEEvent

+ (instancetype)pushEventWithAttributes:(NSDictionary *)attributes {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of making an API that would need to expand for new, arbitrary things that could all be categorized as "debug" events, let's reuse the existing debugEventWithAttributes API and plan on using the attributes for distinction.

I'm saying this also because I think of push and flush as actions that we want to track in a debug log but not as actual events that we send to the server, if that makes sense.

MMEEvent *pushEvent = [[MMEEvent alloc] init];
pushEvent.name = MMEEventTypePush;
pushEvent.attributes = attributes;
return pushEvent;
}

+ (instancetype)flushEventWithAttributes:(NSDictionary *)attributes {
MMEEvent *flushEvent = [[MMEEvent alloc] init];
flushEvent.name = MMEEventTypeFlush;
flushEvent.attributes = attributes;
return flushEvent;
}

+ (instancetype)turnstileEventWithAttributes:(NSDictionary *)attributes {
MMEEvent *turnstileEvent = [[MMEEvent alloc] init];
turnstileEvent.name = MMEEventTypeAppUserTurnstile;
Expand Down
10 changes: 7 additions & 3 deletions MapboxMobileEvents/MMEEventLogger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ NS_ASSUME_NONNULL_BEGIN

@interface MMEEventLogger : NSObject

+ (BOOL)isEnabled;
+ (void)setEnabled:(BOOL)enabled;
+ (void)logEvent:(MMEEvent *)event;
@property (nonatomic, getter=isEnabled) BOOL enabled;

+ (instancetype)sharedLogger;

- (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

Expand Down
67 changes: 59 additions & 8 deletions MapboxMobileEvents/MMEEventLogger.m
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,72 @@
#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;
+ (instancetype)sharedLogger {
static MMEEventLogger *_sharedLogger;
static dispatch_once_t onceToken;

dispatch_once(&onceToken, ^{
_sharedLogger = [[MMEEventLogger alloc] init];
});

return _sharedLogger;
}

+ (void)logEvent:(MMEEvent *)event {
if (_enabled) {
- (void)logEvent:(MMEEvent *)event {
if (self.isEnabled) {
NSLog(@"%@", [NSString stringWithFormat:@"Mapbox Telemetry event %@", event]);

[self writeEventToLocalDebugLog:event];
}
}

+ (BOOL)isEnabled {
return _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 (!self.isEnabled) {
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 }];
}
}
});
}

+ (void)setEnabled:(BOOL)enabled {
_enabled = enabled;
}

@end
20 changes: 17 additions & 3 deletions MapboxMobileEvents/MMEEventsManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ - (void)flush {
return;
}

NSDictionary *flushEventAttributes = @{MMEEventKeyEvent: MMEEventTypeFlush,
MMEEventKeyCreated: [self.dateWrapper formattedDateStringForDate:[self.dateWrapper date]]};
MMEEvent *flushEvent = [MMEEvent pushEventWithAttributes:flushEventAttributes];
Copy link
Contributor

Choose a reason for hiding this comment

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

With the changes in this PR so far, I think this should actually be flushEventWithAttributes:. But let's make the change to use the unified debugEventWithAttributes. As a debug event, the event name will be MMEEventTypeLocalDebug (that is done for us by debugEventWithAttributes:) and we could introduce a "debug event type" that we put in the attributes. In this case, that could be a string constant called MMEDebugEventTypeFlush that equals "flush".


[MMEEventLogger.sharedLogger logEvent:flushEvent];

NSArray *events = [self.eventQueue copy];
__weak __typeof__(self) weakSelf = self;
[self.apiClient postEvents:events completionHandler:^(NSError * _Nullable error) {
Expand Down Expand Up @@ -233,6 +239,7 @@ - (void)sendTurnstileEvent {

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

__weak __typeof__(self) weakSelf = self;
[self.apiClient postEvent:turnstileEvent completionHandler:^(NSError * _Nullable error) {
Expand Down Expand Up @@ -281,11 +288,11 @@ - (void)createAndPushEventBasedOnName:(NSString *)name attributes:(NSDictionary
}

- (void)setDebugLoggingEnabled:(BOOL)debugLoggingEnabled {
[[MMEEventLogger class] setEnabled:debugLoggingEnabled];
MMEEventLogger.sharedLogger.enabled = debugLoggingEnabled;
}

- (BOOL)isDebugLoggingEnabled {
return [[MMEEventLogger class] isEnabled];
return [MMEEventLogger.sharedLogger isEnabled];
}

#pragma mark - Internal API
Expand Down Expand Up @@ -361,6 +368,13 @@ - (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]}];

NSString *eventName = [NSString stringWithFormat:@"%@.%@",MMEEventTypePush,event.name];
NSDictionary *pushEventAttributes = @{MMEEventKeyEvent: eventName,
MMEEventKeyCreated: [self.dateWrapper formattedDateStringForDate:[self.dateWrapper date]]};
MMEEvent *pushEvent = [MMEEvent pushEventWithAttributes:pushEventAttributes];

[MMEEventLogger.sharedLogger logEvent:pushEvent];

if (self.eventQueue.count >= self.configuration.eventFlushCountThreshold) {
[self flush];
}
Expand All @@ -375,7 +389,7 @@ - (void)pushDebugEventWithAttributes:(MMEMapboxEventAttributes *)attributes {
[combinedAttributes setObject:[self.dateWrapper formattedDateStringForDate:[self.dateWrapper date]] forKey:@"created"];
[combinedAttributes setObject:self.uniqueIdentifer.rollingInstanceIdentifer forKey:@"instance"];
MMEEvent *debugEvent = [MMEEvent debugEventWithAttributes:combinedAttributes];
[MMEEventLogger logEvent:debugEvent];
[MMEEventLogger.sharedLogger logEvent:debugEvent];
Copy link
Contributor

Choose a reason for hiding this comment

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

For all of the existing invocations of pushDebugEventWithAttributes: we should make sure their attributes have a MMEDebugEventType and MMEDebugEventType{foo} value that makes sense for the debug log entries they represent.

}

#pragma mark - MMELocationManagerDelegate
Expand Down
2 changes: 1 addition & 1 deletion MapboxMobileEvents/MMETrustKitWrapper.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ + (void)configureCertificatePinningValidation {
return;
}

if (![MMEEventLogger isEnabled]) {
if (![MMEEventLogger.sharedLogger isEnabled]) {
void (^loggerBlock)(NSString *) = ^void(NSString *message){};
[TrustKit setLoggerBlock:loggerBlock];
}
Expand Down