From 543c05b2c57b9681f075ef145eb7cf2dd03cdd20 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 24 Jul 2017 15:31:15 -0400 Subject: [PATCH 1/5] =?UTF-8?q?Add=20a=20=E2=80=9Ccritical=E2=80=9D=20seve?= =?UTF-8?q?rity=20level=20for=20analytics=20events.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit // FREEBIE --- .../src/Messages/TSMessagesManager.m | 16 +-- .../src/Storage/TSStorageManager.m | 21 ++-- SignalServiceKit/src/Util/OWSAnalytics.h | 110 ++++++++++-------- SignalServiceKit/src/Util/OWSAnalytics.m | 23 ++-- 4 files changed, 96 insertions(+), 74 deletions(-) diff --git a/SignalServiceKit/src/Messages/TSMessagesManager.m b/SignalServiceKit/src/Messages/TSMessagesManager.m index d7fc2819d..425a400dc 100644 --- a/SignalServiceKit/src/Messages/TSMessagesManager.m +++ b/SignalServiceKit/src/Messages/TSMessagesManager.m @@ -42,19 +42,19 @@ NS_ASSUME_NONNULL_BEGIN -#define kOWSProdAssertParameterEnvelopeIsLegacy @"envelope_is_legacy" -#define kOWSProdAssertParameterEnvelopeHasContent @"has_content" -#define kOWSProdAssertParameterEnvelopeDescription @"envelope_description" -#define kOWSProdAssertParameterEnvelopeEncryptedLength @"encrypted_length" +#define kOWSAnalyticsParameterEnvelopeIsLegacy @"envelope_is_legacy" +#define kOWSAnalyticsParameterEnvelopeHasContent @"has_content" +#define kOWSAnalyticsParameterEnvelopeDescription @"envelope_description" +#define kOWSAnalyticsParameterEnvelopeEncryptedLength @"encrypted_length" #define AnalyticsParametersFromEnvelope(__envelope) \ ^{ \ NSData *__encryptedData = __envelope.hasContent ? __envelope.content : __envelope.legacyMessage; \ return (@{ \ - kOWSProdAssertParameterEnvelopeIsLegacy : @(__envelope.hasLegacyMessage), \ - kOWSProdAssertParameterEnvelopeHasContent : @(__envelope.hasContent), \ - kOWSProdAssertParameterEnvelopeDescription : [self descriptionForEnvelopeType:__envelope], \ - kOWSProdAssertParameterEnvelopeEncryptedLength : @(__encryptedData.length), \ + kOWSAnalyticsParameterEnvelopeIsLegacy : @(__envelope.hasLegacyMessage), \ + kOWSAnalyticsParameterEnvelopeHasContent : @(__envelope.hasContent), \ + kOWSAnalyticsParameterEnvelopeDescription : [self descriptionForEnvelopeType:__envelope], \ + kOWSAnalyticsParameterEnvelopeEncryptedLength : @(__encryptedData.length), \ }); \ } diff --git a/SignalServiceKit/src/Storage/TSStorageManager.m b/SignalServiceKit/src/Storage/TSStorageManager.m index 8475ec405..cea4330cc 100644 --- a/SignalServiceKit/src/Storage/TSStorageManager.m +++ b/SignalServiceKit/src/Storage/TSStorageManager.m @@ -121,14 +121,16 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; // The best we can try to do is to discard the current database // and behave like a clean install. - OWSProdError(@"storage_error_could_not_load_database"); + // NOTE: This analytics event will never be delivered, since the database isn't working. + OWSProdCritical(@"storage_error_could_not_load_database"); // Try to reset app by deleting database. // Disabled resetting storage until we have better data on why this happens. // [self resetSignalStorage]; if (![self tryToLoadDatabase]) { - OWSProdError(@"storage_error_could_not_load_database_second_attempt"); + // NOTE: This analytics event will never be delivered, since the database isn't working. + OWSProdCritical(@"storage_error_could_not_load_database_second_attempt"); [NSException raise:TSStorageManagerExceptionNameNoDatabase format:@"Failed to initialize database."]; } @@ -187,9 +189,9 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; OWSProdErrorWParams(@"storage_error_deserialization", ^{ return (@{ @"collection" : collection, - kOWSProdAssertParameterNSExceptionName : exception.name, - kOWSProdAssertParameterNSExceptionReason : exception.reason, - kOWSProdAssertParameterNSExceptionClassName : NSStringFromClass([exception class]), + kOWSAnalyticsParameterNSExceptionName : exception.name, + kOWSAnalyticsParameterNSExceptionReason : exception.reason, + kOWSAnalyticsParameterNSExceptionClassName : NSStringFromClass([exception class]), }); }); @throw exception; @@ -260,7 +262,8 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; BOOL success = [ressourceURL setResourceValues:resourcesAttrs error:&error]; if (error || !success) { - OWSProdErrorWNSError(@"storage_error_file_protecion", error); + // NOTE: This analytics event will never be delivered, since the database isn't working. + OWSProdCriticalWNSError(@"storage_error_file_protection", error); return; } } @@ -358,7 +361,8 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; BOOL shouldHavePassword = [NSFileManager.defaultManager fileExistsAtPath:[self dbPath]]; if (shouldHavePassword) { - OWSProdError(@"storage_error_could_not_load_database_second_attempt"); + // NOTE: This analytics event will never be delivered, since the database isn't working. + OWSProdCritical(@"storage_error_could_not_load_database_second_attempt"); } // Try to reset app by deleting database. @@ -377,7 +381,8 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; NSError *keySetError; [SAMKeychain setPassword:newDBPassword forService:keychainService account:keychainDBPassAccount error:&keySetError]; if (keySetError) { - OWSProdErrorWNSError(@"storage_error_could_not_store_database_password", keySetError); + // NOTE: This analytics event will never be delivered, since the database isn't working. + OWSProdCriticalWNSError(@"storage_error_could_not_store_database_password", keySetError); [self deletePasswordFromKeychain]; diff --git a/SignalServiceKit/src/Util/OWSAnalytics.h b/SignalServiceKit/src/Util/OWSAnalytics.h index 10714eeaa..3719fcade 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.h +++ b/SignalServiceKit/src/Util/OWSAnalytics.h @@ -6,23 +6,17 @@ NS_ASSUME_NONNULL_BEGIN // TODO: We probably don't need all of these levels. typedef NS_ENUM(NSUInteger, OWSAnalyticsSeverity) { - OWSAnalyticsSeverityDebug = 0, + // Info events are routine. + // + // It's safe to discard a large fraction of these events. OWSAnalyticsSeverityInfo = 1, - OWSAnalyticsSeverityWarn = 2, + // Error events should never be discarded. OWSAnalyticsSeverityError = 3, - // I suspect we'll stage the development of our analytics, - // initially building only a minimal solution: an endpoint which - // ignores most requests, and sends only the highest-severity - // events as email to developers. - // - // This "critical" level of severity is intended for that purpose (for now). + // Critical events should never be discarded. // - // We might want to have an additional level of severity for - // critical (crashing) bugs that occur during app startup. These - // events should be sent to the service immediately and the app - // should block until that request completes. - OWSAnalyticsSeverityCritical = 4, - OWSAnalyticsSeverityOff = 5 + // Additionally, to avoid losing critical events they should + // be persisted synchronously. + OWSAnalyticsSeverityCritical = 4 }; // This is a placeholder. We don't yet serialize or transmit analytics events. @@ -54,13 +48,32 @@ typedef NS_ENUM(NSUInteger, OWSAnalyticsSeverity) { typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)(); -#define kOWSProdAssertParameterDescription @"description" -#define kOWSProdAssertParameterNSErrorDomain @"nserror_domain" -#define kOWSProdAssertParameterNSErrorCode @"nserror_code" -#define kOWSProdAssertParameterNSErrorDescription @"nserror_description" -#define kOWSProdAssertParameterNSExceptionName @"nsexception_name" -#define kOWSProdAssertParameterNSExceptionReason @"nsexception_reason" -#define kOWSProdAssertParameterNSExceptionClassName @"nsexception_classname" +#define kOWSAnalyticsParameterDescription @"description" +#define kOWSAnalyticsParameterNSErrorDomain @"nserror_domain" +#define kOWSAnalyticsParameterNSErrorCode @"nserror_code" +#define kOWSAnalyticsParameterNSErrorDescription @"nserror_description" +#define kOWSAnalyticsParameterNSExceptionName @"nsexception_name" +#define kOWSAnalyticsParameterNSExceptionReason @"nsexception_reason" +#define kOWSAnalyticsParameterNSExceptionClassName @"nsexception_classname" + +#define AnalyticsParametersFromNSError(__nserror) \ + ^{ \ + return (@{ \ + kOWSAnalyticsParameterNSErrorDomain : (__nserror.domain ?: @"unknown"), \ + kOWSAnalyticsParameterNSErrorCode : @(__nserror.code), \ + kOWSAnalyticsParameterNSErrorDescription : (__nserror.description ?: @"unknown"), \ + }); \ + } + +#define AnalyticsParametersFromNSException(__exception) \ + ^{ \ + return (@{ \ + kOWSAnalyticsParameterNSExceptionName : (__exception.name ?: @"unknown"), \ + kOWSAnalyticsParameterNSExceptionReason : (__exception.reason ?: @"unknown"), \ + kOWSAnalyticsParameterNSExceptionClassName : \ + (__exception ? NSStringFromClass([__exception class]) : @"unknown"), \ + }); \ + } // These methods should be used to assert errors for which we want to fire analytics events. // @@ -116,25 +129,6 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define OWSProdCFail(__analyticsEventName) OWSProdCFailWParams(__analyticsEventName, nil) -#define AnalyticsParametersFromNSError(__nserror) \ - ^{ \ - return (@{ \ - kOWSProdAssertParameterNSErrorDomain : (__nserror.domain ?: @"unknown"), \ - kOWSProdAssertParameterNSErrorCode : @(__nserror.code), \ - kOWSProdAssertParameterNSErrorDescription : (__nserror.description ?: @"unknown"), \ - }); \ - } - -#define AnalyticsParametersFromNSException(__exception) \ - ^{ \ - return (@{ \ - kOWSProdAssertParameterNSExceptionName : (__exception.name ?: @"unknown"), \ - kOWSProdAssertParameterNSExceptionReason : (__exception.reason ?: @"unknown"), \ - kOWSProdAssertParameterNSExceptionClassName : \ - (__exception ? NSStringFromClass([__exception class]) : @"unknown"), \ - }); \ - } - #define OWSProdFailWNSError(__analyticsEventName, __nserror) \ { \ DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __nserror.debugDescription); \ @@ -147,28 +141,32 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() OWSProdFailWParams(__analyticsEventName, AnalyticsParametersFromNSException(__exception)) \ } +#define OWSProdCFail(__analyticsEventName) OWSProdCFailWParams(__analyticsEventName, nil) + #define OWSProdEventWParams(__severityLevel, __analyticsEventName, __parametersBlock) \ { \ NSDictionary *__eventParameters \ = (__parametersBlock ? ((OWSProdAssertParametersBlock)__parametersBlock)() : nil); \ [OWSAnalytics logEvent:__analyticsEventName \ - severity:OWSAnalyticsSeverityCritical \ + severity:__severityLevel \ parameters:__eventParameters \ location:__PRETTY_FUNCTION__ \ line:__LINE__]; \ } -#define OWSProdErrorWParams(__analyticsEventName, __parametersBlock) \ - OWSProdEventWParams(OWSAnalyticsSeverityCritical, __analyticsEventName, __parametersBlock) - -#define OWSProdError(__analyticsEventName) OWSProdEventWParams(OWSAnalyticsSeverityCritical, __analyticsEventName, nil) +#pragma mark - Info Events #define OWSProdInfoWParams(__analyticsEventName, __parametersBlock) \ OWSProdEventWParams(OWSAnalyticsSeverityInfo, __analyticsEventName, __parametersBlock) #define OWSProdInfo(__analyticsEventName) OWSProdEventWParams(OWSAnalyticsSeverityInfo, __analyticsEventName, nil) -#define OWSProdCFail(__analyticsEventName) OWSProdCFailWParams(__analyticsEventName, nil) +#pragma mark - Error Events + +#define OWSProdErrorWParams(__analyticsEventName, __parametersBlock) \ + OWSProdEventWParams(OWSAnalyticsSeverityError, __analyticsEventName, __parametersBlock) + +#define OWSProdError(__analyticsEventName) OWSProdEventWParams(OWSAnalyticsSeverityError, __analyticsEventName, nil) #define OWSProdErrorWNSError(__analyticsEventName, __nserror) \ { \ @@ -182,4 +180,24 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromNSException(__exception)) \ } +#pragma mark - Critical Events + +#define OWSProdCriticalWParams(__analyticsEventName, __parametersBlock) \ + OWSProdEventWParams(OWSAnalyticsSeverityCritical, __analyticsEventName, __parametersBlock) + +#define OWSProdCritical(__analyticsEventName) \ + OWSProdEventWParams(OWSAnalyticsSeverityCritical, __analyticsEventName, nil) + +#define OWSProdCriticalWNSError(__analyticsEventName, __nserror) \ + { \ + DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __nserror.debugDescription); \ + OWSProdCriticalWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) \ + } + +#define OWSProdCriticalWNSException(__analyticsEventName, __exception) \ + { \ + DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __exception); \ + OWSProdCriticalWParams(__analyticsEventName, AnalyticsParametersFromNSException(__exception)) \ + } + NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Util/OWSAnalytics.m b/SignalServiceKit/src/Util/OWSAnalytics.m index 043a008c3..eda5606db 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.m +++ b/SignalServiceKit/src/Util/OWSAnalytics.m @@ -223,7 +223,7 @@ const int kOWSAnalytics_DiscardFrequency = 0; return (long)round(pow(10, floor(log10(value)))); } -- (void)addEvent:(NSString *)eventName properties:(NSDictionary *)properties +- (void)addEvent:(NSString *)eventName async:(BOOL)async properties:(NSDictionary *)properties { OWSAssert(eventName.length > 0); @@ -234,7 +234,7 @@ const int kOWSAnalytics_DiscardFrequency = 0; } #ifndef NO_SIGNAL_ANALYTICS - dispatch_async(self.serialQueue, ^{ + void (^writeEvent)() = ^{ // Add super properties. NSMutableDictionary *eventProperties = (properties ? [properties mutableCopy] : [NSMutableDictionary new]); [eventProperties addEntriesFromDictionary:self.eventSuperProperties]; @@ -250,12 +250,18 @@ const int kOWSAnalytics_DiscardFrequency = 0; DDLogError(@"%@ Event queue overflow.", self.tag); return; } - + [transaction setObject:eventDictionary forKey:eventKey inCollection:kOWSAnalytics_EventsCollection]; }]; [self tryToSyncEvents]; - }); + }; + + if (async) { + dispatch_async(self.serialQueue, writeEvent); + } else { + dispatch_sync(self.serialQueue, writeEvent); + } #endif } @@ -277,18 +283,11 @@ const int kOWSAnalytics_DiscardFrequency = 0; DDLogFlag logFlag; BOOL async = YES; switch (severity) { - case OWSAnalyticsSeverityDebug: - logFlag = DDLogFlagDebug; - break; case OWSAnalyticsSeverityInfo: logFlag = DDLogFlagInfo; break; - case OWSAnalyticsSeverityWarn: - logFlag = DDLogFlagWarning; - break; case OWSAnalyticsSeverityError: logFlag = DDLogFlagError; - async = NO; break; case OWSAnalyticsSeverityCritical: logFlag = DDLogFlagError; @@ -313,7 +312,7 @@ const int kOWSAnalytics_DiscardFrequency = 0; NSMutableDictionary *eventProperties = (parameters ? [parameters mutableCopy] : [NSMutableDictionary new]); eventProperties[@"event_location"] = [NSString stringWithFormat:@"%s:%d", location, line]; - [self addEvent:eventName properties:eventProperties]; + [self addEvent:eventName async:async properties:eventProperties]; } #pragma mark - Logging From 11f52757b257f144fe21a129c6edb7062acae2a0 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 24 Jul 2017 15:35:54 -0400 Subject: [PATCH 2/5] Use background task when sending analytics events. // FREEBIE --- SignalServiceKit/src/Util/OWSAnalytics.m | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/SignalServiceKit/src/Util/OWSAnalytics.m b/SignalServiceKit/src/Util/OWSAnalytics.m index eda5606db..9dd7ebc66 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.m +++ b/SignalServiceKit/src/Util/OWSAnalytics.m @@ -124,15 +124,15 @@ const int kOWSAnalytics_DiscardFrequency = 0; - (void)tryToSyncEvents { - // Don't try to sync if: - // - // * There's no network available. - // * There's already a sync request in flight. - if (!self.reachability.isReachable || self.hasRequestInFlight) { - return; - } - dispatch_async(self.serialQueue, ^{ + // Don't try to sync if: + // + // * There's no network available. + // * There's already a sync request in flight. + if (!self.reachability.isReachable || self.hasRequestInFlight) { + return; + } + __block NSString *firstEventKey = nil; __block NSDictionary *firstEventDictionary = nil; [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { @@ -157,6 +157,12 @@ const int kOWSAnalytics_DiscardFrequency = 0; DDLogDebug(@"%@ trying to deliver event: %@", self.tag, firstEventKey); self.hasRequestInFlight = YES; + + __block UIBackgroundTaskIdentifier task; + task = [UIApplication.sharedApplication beginBackgroundTaskWithExpirationHandler:^{ + [UIApplication.sharedApplication endBackgroundTask:task]; + }]; + // Until we integrate with an analytics platform, behave as though all event delivery succeeds. dispatch_async(dispatch_get_main_queue(), ^{ self.hasRequestInFlight = NO; @@ -169,6 +175,8 @@ const int kOWSAnalytics_DiscardFrequency = 0; }]; } + [UIApplication.sharedApplication endBackgroundTask:task]; + // Wait a second between network requests / retries. dispatch_after( dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1.f * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ From b17a7c5751a7c125b9c09ba5388d86c8a31bdb08 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 24 Jul 2017 16:05:29 -0400 Subject: [PATCH 3/5] Review NSError usage. // FREEBIE --- .../src/Contacts/ContactsUpdater.m | 2 +- SignalServiceKit/src/Devices/OWSDevice.m | 20 +++++++++++++++---- SignalServiceKit/src/Util/OWSAnalytics.h | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/SignalServiceKit/src/Contacts/ContactsUpdater.m b/SignalServiceKit/src/Contacts/ContactsUpdater.m index 5026c19e5..b4583ef60 100644 --- a/SignalServiceKit/src/Contacts/ContactsUpdater.m +++ b/SignalServiceKit/src/Contacts/ContactsUpdater.m @@ -212,7 +212,7 @@ NS_ASSUME_NONNULL_BEGIN NSHTTPURLResponse *response = (NSHTTPURLResponse *)task.response; if (response.statusCode == 413) { failure(OWSErrorWithCodeDescription( - OWSErrorCodeContactsUpdaterRateLimit, OWSSignalServiceKitErrorDomain)); + OWSErrorCodeContactsUpdaterRateLimit, @"Contacts Intersection Rate Limit")); } else { failure(error); } diff --git a/SignalServiceKit/src/Devices/OWSDevice.m b/SignalServiceKit/src/Devices/OWSDevice.m index 3f3e9ba6b..3d329614a 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.m +++ b/SignalServiceKit/src/Devices/OWSDevice.m @@ -88,8 +88,8 @@ uint32_t const OWSDevicePrimaryDeviceId = 1; } } *success = NO; - *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecodeJson, - [NSString stringWithFormat:@"unable to decode date from %@", value]); + DDLogError(@"%@ unable to decode date from %@", self.tag, value); + *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecodeJson, @"Unable to decode date from %@"); return nil; } reverseBlock:^id(id value, BOOL *success, NSError **error) { @@ -101,8 +101,8 @@ uint32_t const OWSDevicePrimaryDeviceId = 1; return result; } } - *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToEncodeJson, - [NSString stringWithFormat:@"unable to encode date from %@", value]); + DDLogError(@"%@ unable to encode date from %@", self.tag, value); + *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToEncodeJson, @"Unable to encode date"); *success = NO; return nil; }]; @@ -178,6 +178,18 @@ uint32_t const OWSDevicePrimaryDeviceId = 1; return self.deviceId == device.deviceId; } +#pragma mark - Logging + ++ (NSString *)tag +{ + return [NSString stringWithFormat:@"[%@]", self.class]; +} + +- (NSString *)tag +{ + return self.class.tag; +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Util/OWSAnalytics.h b/SignalServiceKit/src/Util/OWSAnalytics.h index 3719fcade..40620d5e8 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.h +++ b/SignalServiceKit/src/Util/OWSAnalytics.h @@ -56,12 +56,12 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define kOWSAnalyticsParameterNSExceptionReason @"nsexception_reason" #define kOWSAnalyticsParameterNSExceptionClassName @"nsexception_classname" +// We don't include the error description because it may have PII. #define AnalyticsParametersFromNSError(__nserror) \ ^{ \ return (@{ \ kOWSAnalyticsParameterNSErrorDomain : (__nserror.domain ?: @"unknown"), \ kOWSAnalyticsParameterNSErrorCode : @(__nserror.code), \ - kOWSAnalyticsParameterNSErrorDescription : (__nserror.description ?: @"unknown"), \ }); \ } From fa7a2407bfa5299cbdbd31b9a83b93f694d8849b Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 24 Jul 2017 16:20:00 -0400 Subject: [PATCH 4/5] Respond to CR. // FREEBIE --- .../src/Messages/TSMessagesManager.m | 8 ++++++-- SignalServiceKit/src/Util/OWSAnalytics.h | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Messages/TSMessagesManager.m b/SignalServiceKit/src/Messages/TSMessagesManager.m index 425a400dc..a5b6c8a76 100644 --- a/SignalServiceKit/src/Messages/TSMessagesManager.m +++ b/SignalServiceKit/src/Messages/TSMessagesManager.m @@ -44,7 +44,7 @@ NS_ASSUME_NONNULL_BEGIN #define kOWSAnalyticsParameterEnvelopeIsLegacy @"envelope_is_legacy" #define kOWSAnalyticsParameterEnvelopeHasContent @"has_content" -#define kOWSAnalyticsParameterEnvelopeDescription @"envelope_description" +#define kOWSAnalyticsParameterEnvelopeType @"envelope_type" #define kOWSAnalyticsParameterEnvelopeEncryptedLength @"encrypted_length" #define AnalyticsParametersFromEnvelope(__envelope) \ @@ -53,11 +53,15 @@ NS_ASSUME_NONNULL_BEGIN return (@{ \ kOWSAnalyticsParameterEnvelopeIsLegacy : @(__envelope.hasLegacyMessage), \ kOWSAnalyticsParameterEnvelopeHasContent : @(__envelope.hasContent), \ - kOWSAnalyticsParameterEnvelopeDescription : [self descriptionForEnvelopeType:__envelope], \ + kOWSAnalyticsParameterEnvelopeType : [self descriptionForEnvelopeType:__envelope], \ kOWSAnalyticsParameterEnvelopeEncryptedLength : @(__encryptedData.length), \ }); \ } +// The debug logs can be more verbose than the analytics events. +// +// In this case `descriptionForEnvelope` is valuable enough to +// log but too dangerous to include in the analytics event. #define OWSProdErrorWEnvelope(__analyticsEventName, __envelope) \ { \ DDLogError(@"%s:%d %@: %@", \ diff --git a/SignalServiceKit/src/Util/OWSAnalytics.h b/SignalServiceKit/src/Util/OWSAnalytics.h index 40620d5e8..7e44c8fd6 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.h +++ b/SignalServiceKit/src/Util/OWSAnalytics.h @@ -129,12 +129,20 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define OWSProdCFail(__analyticsEventName) OWSProdCFailWParams(__analyticsEventName, nil) +// The debug logs can be more verbose than the analytics events. +// +// In this case `debugDescription` is valuable enough to +// log but too dangerous to include in the analytics event. #define OWSProdFailWNSError(__analyticsEventName, __nserror) \ { \ DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __nserror.debugDescription); \ OWSProdFailWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) \ } +// The debug logs can be more verbose than the analytics events. +// +// In this case `exception` is valuable enough to +// log but too dangerous to include in the analytics event. #define OWSProdFailWNSException(__analyticsEventName, __exception) \ { \ DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __exception); \ @@ -168,12 +176,20 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define OWSProdError(__analyticsEventName) OWSProdEventWParams(OWSAnalyticsSeverityError, __analyticsEventName, nil) +// The debug logs can be more verbose than the analytics events. +// +// In this case `debugDescription` is valuable enough to +// log but too dangerous to include in the analytics event. #define OWSProdErrorWNSError(__analyticsEventName, __nserror) \ { \ DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __nserror.debugDescription); \ OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) \ } +// The debug logs can be more verbose than the analytics events. +// +// In this case `exception` is valuable enough to +// log but too dangerous to include in the analytics event. #define OWSProdErrorWNSException(__analyticsEventName, __exception) \ { \ DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __exception); \ @@ -194,6 +210,10 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() OWSProdCriticalWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) \ } +// The debug logs can be more verbose than the analytics events. +// +// In this case `exception` is valuable enough to +// log but too dangerous to include in the analytics event. #define OWSProdCriticalWNSException(__analyticsEventName, __exception) \ { \ DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __exception); \ From ef4b1cf4774f42f5ea1c0fc43ebd8b08bf820524 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 24 Jul 2017 17:18:15 -0400 Subject: [PATCH 5/5] Respond to CR. // FREEBIE --- SignalServiceKit/src/Util/OWSAnalytics.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/SignalServiceKit/src/Util/OWSAnalytics.m b/SignalServiceKit/src/Util/OWSAnalytics.m index 9dd7ebc66..3a6e0b396 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.m +++ b/SignalServiceKit/src/Util/OWSAnalytics.m @@ -160,6 +160,8 @@ const int kOWSAnalytics_DiscardFrequency = 0; __block UIBackgroundTaskIdentifier task; task = [UIApplication.sharedApplication beginBackgroundTaskWithExpirationHandler:^{ + self.hasRequestInFlight = NO; + [UIApplication.sharedApplication endBackgroundTask:task]; }];