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.

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

feat: Track timezone changes as breadcrumbs #1930

Merged
merged 34 commits into from
Jul 6, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
cd2e0c0
feat: Add automatic timezone change reporting as a breadcrumb
kevinrenskers Jul 4, 2022
69c98c5
Add a test for the new breadcrumb
kevinrenskers Jul 4, 2022
b035930
Format code
getsentry-bot Jul 4, 2022
1434409
Add changelog entry
kevinrenskers Jul 4, 2022
1ecd302
Indent with 4 spaces for all source files
kevinrenskers Jul 4, 2022
543bedb
Fix testTimezoneChangeBreadcrumb by using the Swiftified name
kevinrenskers Jul 4, 2022
b90c52d
Actually fix the tests
kevinrenskers Jul 4, 2022
4112017
Improve the start method's logic, prevent duplicate call in case of iOS
kevinrenskers Jul 4, 2022
ab971ed
Undo indentation change, it'll be in a separate PR
kevinrenskers Jul 4, 2022
de3547d
Merge branch 'master' into feat/timezone-change-breadcrumbs
kevinrenskers Jul 4, 2022
81102cc
Feature complete, but test is flaky
kevinrenskers Jul 5, 2022
4118523
Format code
getsentry-bot Jul 5, 2022
57d86c1
Merge branch 'master' into feat/timezone-change-breadcrumbs
kevinrenskers Jul 5, 2022
694ed71
timezoneOffset is nullable and shouldn't stop SentryAppState from bei…
kevinrenskers Jul 5, 2022
9d5b0ac
Only run the code in iOS, because it doesn't compile for macOS
kevinrenskers Jul 5, 2022
82fe669
Format code
getsentry-bot Jul 5, 2022
f453208
Deal with timezoneOffset == nil
kevinrenskers Jul 5, 2022
8d13cd4
Commit first set of suggestions
kevinrenskers Jul 5, 2022
b83f59c
Format code
getsentry-bot Jul 5, 2022
1a4f01e
Add more tests
kevinrenskers Jul 5, 2022
a0b6087
It works, but tests need to be fixed
kevinrenskers Jul 5, 2022
cb42c7e
Format code
getsentry-bot Jul 5, 2022
6c7b428
Don't cache fileManager, as it can change when the hub changes
kevinrenskers Jul 5, 2022
9f9e79a
Use currentDateProvider
kevinrenskers Jul 5, 2022
c585e80
Format code
getsentry-bot Jul 5, 2022
f808338
Undo
kevinrenskers Jul 5, 2022
4f7edd3
Tests compile again
kevinrenskers Jul 5, 2022
97acf8d
Fix warning
kevinrenskers Jul 5, 2022
bc9d991
testTimezoneChangeBreadcrumb is still problematic
kevinrenskers Jul 5, 2022
8ce9385
Added more tests, and the whole suit runs successfully
kevinrenskers Jul 6, 2022
70b982e
Format code
getsentry-bot Jul 6, 2022
0ad0783
Add more filemanager tests
kevinrenskers Jul 6, 2022
0996314
Format code
getsentry-bot Jul 6, 2022
2571228
One more test
kevinrenskers Jul 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Features

- Track timezone changes as breadcrumbs (#1930)

## 7.19.0

### Features
Expand Down
11 changes: 11 additions & 0 deletions Sources/Sentry/SentryAppState.m
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ - (instancetype)initWithReleaseName:(NSString *)releaseName
vendorId:(NSString *)vendorId
isDebugging:(BOOL)isDebugging
systemBootTimestamp:(NSDate *)systemBootTimestamp
timezoneOffset:(NSTimeInterval)timezoneOffset
{
if (self = [super init]) {
_releaseName = releaseName;
Expand All @@ -26,6 +27,7 @@ - (instancetype)initWithReleaseName:(NSString *)releaseName
_isActive = NO;
_wasTerminated = NO;
_isANROngoing = NO;
_timezoneOffset = @(timezoneOffset);
}
return self;
}
Expand Down Expand Up @@ -90,6 +92,11 @@ - (nullable instancetype)initWithJSONObject:(NSDictionary *)jsonObject
} else {
_isANROngoing = [isANROngoing boolValue];
}

id timezoneOffset = [jsonObject valueForKey:@"timezone_offset"];
if (timezoneOffset != nil && [timezoneOffset isKindOfClass:[NSNumber class]]) {
_timezoneOffset = timezoneOffset;
}
}
return self;
}
Expand All @@ -108,6 +115,10 @@ - (nullable instancetype)initWithJSONObject:(NSDictionary *)jsonObject
[data setValue:@(self.wasTerminated) forKey:@"was_terminated"];
[data setValue:@(self.isANROngoing) forKey:@"is_anr_ongoing"];

if (self.timezoneOffset != nil) {
[data setValue:self.timezoneOffset forKey:@"timezone_offset"];
}

return data;
}

Expand Down
3 changes: 2 additions & 1 deletion Sources/Sentry/SentryAppStateManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ - (SentryAppState *)buildCurrentAppState
osVersion:UIDevice.currentDevice.systemVersion
vendorId:vendorId
isDebugging:isDebugging
systemBootTimestamp:self.sysctl.systemBootTimestamp];
systemBootTimestamp:self.sysctl.systemBootTimestamp
timezoneOffset:[NSTimeZone localTimeZone].secondsFromGMT];
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved
}

- (SentryAppState *)loadCurrentAppState
Expand Down
56 changes: 56 additions & 0 deletions Sources/Sentry/SentrySystemEventBreadcrumbs.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#import "SentrySystemEventBreadcrumbs.h"
#import "SentryAppState.h"
#import "SentryAppStateManager.h"
#import "SentryBreadcrumb.h"
#import "SentryDependencyContainer.h"
#import "SentryLog.h"
#import "SentrySDK.h"

Expand Down Expand Up @@ -45,6 +48,7 @@ - (void)start:(UIDevice *)currentDevice
}
[self initKeyboardVisibilityObserver];
[self initScreenshotObserver];
[self initTimezoneObserver];
}
#endif

Expand Down Expand Up @@ -177,6 +181,58 @@ - (void)initScreenshotObserver
name:UIApplicationUserDidTakeScreenshotNotification
object:nil];
}

- (NSNumber *_Nullable)storedTimezoneOffset
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved
{
SentryAppStateManager *appStateManager =
[SentryDependencyContainer sharedInstance].appStateManager;
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved
SentryAppState *currentState = [appStateManager loadCurrentAppState];
return currentState.timezoneOffset;
}

- (void)initTimezoneObserver
{
// Detect if the stored timezone is different from the current one;
// if so, then we also send a breadcrumb
NSNumber *_Nullable storedTimezoneOffset = [self storedTimezoneOffset];

if (storedTimezoneOffset == nil) {
[self updateStoredTimezone];
} else if (storedTimezoneOffset.doubleValue != [NSTimeZone localTimeZone].secondsFromGMT) {
[self timezoneEventTriggered];
}

// Posted when the timezone of the device changed
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved
[[NSNotificationCenter defaultCenter] addObserver:self
selector:@selector(timezoneEventTriggered)
name:NSSystemTimeZoneDidChangeNotification
object:nil];
}

- (void)timezoneEventTriggered
{
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved
SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] initWithLevel:kSentryLevelInfo
category:@"device.event"];
crumb.type = @"system";
crumb.data = @{
@"action" : @"TIMEZONE_CHANGE",
@"previous" : [self storedTimezoneOffset] ?: @(0),
@"new" : @([NSTimeZone localTimeZone].secondsFromGMT)
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved
};
[SentrySDK addBreadcrumb:crumb];

[self updateStoredTimezone];
}

- (void)updateStoredTimezone
{
SentryAppStateManager *appStateManager =
[SentryDependencyContainer sharedInstance].appStateManager;
[appStateManager updateAppState:^(SentryAppState *appState) {
appState.timezoneOffset = @([NSTimeZone localTimeZone].secondsFromGMT);
}];
}

#endif

@end
5 changes: 4 additions & 1 deletion Sources/Sentry/include/SentryAppState.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ SENTRY_NO_INIT
osVersion:(NSString *)osVersion
vendorId:(NSString *)vendorId
isDebugging:(BOOL)isDebugging
systemBootTimestamp:(NSDate *)systemBootTimestamp;
systemBootTimestamp:(NSDate *)systemBootTimestamp
timezoneOffset:(NSTimeInterval)timezoneOffset;

/**
* Initializes SentryAppState from a JSON object.
Expand Down Expand Up @@ -42,6 +43,8 @@ SENTRY_NO_INIT

@property (nonatomic, assign) BOOL isANROngoing;

@property (nonatomic, copy) NSNumber *_Nullable timezoneOffset;
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

m: We use three different data types. In the constructor, timezoneOffset is NSTimeInterval, here it is NSNumber and [NSTimeZone localTimeZone].secondsFromGMT] is NSInteger. Can we please align these. I would suggest to use NSInteger everywhere and renaming this to secondsFromGMT.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right, the NSTimeInterval was from a different property I used for a while, that's easily changed to NSInteger.

But changing the NSNumber to NSInteger.. can NSInteger be null? I really need to refresh my knowledge of optionals in Obj-C 😅 I made it an NSNumber specifically because of the optionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and renaming this to secondsFromGMT

I think timezoneOffset is a lot more clear. secondsFromGMT doesn't really tell you that this about a timezone thing.


@end

NS_ASSUME_NONNULL_END
2 changes: 1 addition & 1 deletion Tests/SentryTests/Helper/SentryAppStateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class SentryAppStateTests: XCTestCase {
let date = Date(timeIntervalSince1970: 0.1)
let expectedDate = Date(timeIntervalSince1970: 0)

let sut = SentryAppState(releaseName: "", osVersion: "", vendorId: "", isDebugging: false, systemBootTimestamp: date)
let sut = SentryAppState(releaseName: "", osVersion: "", vendorId: "", isDebugging: false, systemBootTimestamp: date, timezoneOffset: 0)
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved

XCTAssertEqual(expectedDate, sut.systemBootTimestamp)
}
Expand Down
4 changes: 2 additions & 2 deletions Tests/SentryTests/Helper/SentryFileManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -444,15 +444,15 @@ class SentryFileManagerTests: XCTestCase {

setImmutableForAppState(immutable: true)

sut.store(SentryAppState(releaseName: "", osVersion: "", vendorId: "", isDebugging: false, systemBootTimestamp: fixture.currentDateProvider.date()))
sut.store(SentryAppState(releaseName: "", osVersion: "", vendorId: "", isDebugging: false, systemBootTimestamp: fixture.currentDateProvider.date(), timezoneOffset: 0))

assertValidAppStateStored()
}

func testStoreFaultyAppState_AppStateIsNotOverwritten() {
sut.store(TestData.appState)

sut.store(AppStateWithFaultySerialization(releaseName: "", osVersion: "", vendorId: "", isDebugging: false, systemBootTimestamp: fixture.currentDateProvider.date()))
sut.store(AppStateWithFaultySerialization(releaseName: "", osVersion: "", vendorId: "", isDebugging: false, systemBootTimestamp: fixture.currentDateProvider.date(), timezoneOffset: 0))

assertValidAppStateStored()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,11 @@ class SentrySystemEventBreadcrumbsTest: XCTestCase {
return _orientation
}
}

override func tearDown() {
super.tearDown()
clearTestState()
SentryDependencyContainer.sharedInstance().appStateManager.removeCurrentAppState()
}

func testBatteryLevelBreadcrumb() {
Expand Down Expand Up @@ -218,6 +219,14 @@ class SentrySystemEventBreadcrumbsTest: XCTestCase {
NotificationCenter.default.post(Notification(name: UIApplication.userDidTakeScreenshotNotification))
assertBreadcrumbAction(scope: scope, action: "UIApplicationUserDidTakeScreenshotNotification")
}

func testTimezoneChangeBreadcrumb() {
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved
let scope = Scope()
sut = fixture.getSut(scope: scope, currentDevice: nil)

NotificationCenter.default.post(Notification(name: NSNotification.Name.NSSystemTimeZoneDidChange))
assertBreadcrumbAction(scope: scope, action: "TIMEZONE_CHANGE")
}

private func assertBreadcrumbAction(scope: Scope, action: String) {
let ser = scope.serialize()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class SentryOutOfMemoryTrackerTests: XCTestCase {

let actual = fixture.fileManager.readAppState()

let appState = SentryAppState(releaseName: fixture.options.releaseName ?? "", osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.sysctl.systemBootTimestamp)
let appState = SentryAppState(releaseName: fixture.options.releaseName ?? "", osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.sysctl.systemBootTimestamp, timezoneOffset: 0)

XCTAssertEqual(appState, actual)
XCTAssertEqual(1, fixture.dispatchQueue.dispatchAsyncCalled)
Expand Down Expand Up @@ -94,23 +94,23 @@ class SentryOutOfMemoryTrackerTests: XCTestCase {
}

func testDifferentAppVersions_NoOOM() {
givenPreviousAppState(appState: SentryAppState(releaseName: "0.9.0", osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date()))
givenPreviousAppState(appState: SentryAppState(releaseName: "0.9.0", osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date(), timezoneOffset: 0))

sut.start()

assertNoOOMSent()
}

func testDifferentOSVersions_NoOOM() {
givenPreviousAppState(appState: SentryAppState(releaseName: fixture.options.releaseName ?? "", osVersion: "1.0.0", vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date()))
givenPreviousAppState(appState: SentryAppState(releaseName: fixture.options.releaseName ?? "", osVersion: "1.0.0", vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date(), timezoneOffset: 0))

sut.start()

assertNoOOMSent()
}

func testDifferentVendorId_NoOOM() {
givenPreviousAppState(appState: SentryAppState(releaseName: fixture.options.releaseName ?? "", osVersion: "1.0.0", vendorId: "0987654321", isDebugging: false, systemBootTimestamp: fixture.currentDate.date()))
givenPreviousAppState(appState: SentryAppState(releaseName: fixture.options.releaseName ?? "", osVersion: "1.0.0", vendorId: "0987654321", isDebugging: false, systemBootTimestamp: fixture.currentDate.date(), timezoneOffset: 0))

sut.start()

Expand Down Expand Up @@ -142,7 +142,7 @@ class SentryOutOfMemoryTrackerTests: XCTestCase {
}

func testCrashReport_NoOOM() {
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date())
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date(), timezoneOffset: 0)
givenPreviousAppState(appState: appState)
fixture.crashWrapper.internalCrashedLastLaunch = true

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class SentryAppStartTrackerTests: XCTestCase {

func testSecondStart_AfterSystemReboot_IsColdStart() {
let previousBootTime = fixture.currentDate.date().addingTimeInterval(-1)
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: previousBootTime)
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: previousBootTime, timezoneOffset: 0)
givenPreviousAppState(appState: appState)

startApp()
Expand All @@ -81,7 +81,7 @@ class SentryAppStartTrackerTests: XCTestCase {
}

func testAppUpgrade_IsColdStart() {
let appState = SentryAppState(releaseName: "0.9.0", osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date())
let appState = SentryAppState(releaseName: "0.9.0", osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date(), timezoneOffset: 0)
givenPreviousAppState(appState: appState)

startApp()
Expand All @@ -107,7 +107,7 @@ class SentryAppStartTrackerTests: XCTestCase {
sendAppMeasurement()
terminateApp()

let appState = SentryAppState(releaseName: "1.0.0", osVersion: "14.4.1", vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: self.fixture.currentDate.date())
let appState = SentryAppState(releaseName: "1.0.0", osVersion: "14.4.1", vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: self.fixture.currentDate.date(), timezoneOffset: 0)
givenPreviousAppState(appState: appState)

startApp()
Expand All @@ -119,7 +119,7 @@ class SentryAppStartTrackerTests: XCTestCase {
* Test if the user changes the time of his phone and the previous boot time is in the future.
*/
func testAppLaunches_PreviousBootTimeInFuture_NoAppStartUp() {
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date().addingTimeInterval(1))
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date().addingTimeInterval(1), timezoneOffset: 0)
givenPreviousAppState(appState: appState)

startApp()
Expand Down Expand Up @@ -233,7 +233,7 @@ class SentryAppStartTrackerTests: XCTestCase {
private func givenSystemNotRebooted() {
let systemBootTimestamp = fixture.currentDate.date()
fixture.sysctl.setProcessStartTimestamp(value: fixture.currentDate.date())
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: systemBootTimestamp)
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: systemBootTimestamp, timezoneOffset: 0)
givenPreviousAppState(appState: appState)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ class SentryCrashIntegrationTests: XCTestCase {

#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
private func givenOOMAppState() {
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: UIDevice.current.identifierForVendor?.uuidString ?? "", isDebugging: false, systemBootTimestamp: fixture.currentDateProvider.date())
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: UIDevice.current.identifierForVendor?.uuidString ?? "", isDebugging: false, systemBootTimestamp: fixture.currentDateProvider.date(), timezoneOffset: 0)
appState.isActive = true
fixture.fileManager.store(appState)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class SentrySessionGeneratorTests: XCTestCase {
sentryCrash.internalCrashedLastLaunch = false

#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
let appState = SentryAppState(releaseName: options.releaseName!, osVersion: UIDevice.current.systemVersion, vendorId: "12345678-1234-1234-1234-1234567890AB", isDebugging: false, systemBootTimestamp: Date())
let appState = SentryAppState(releaseName: options.releaseName!, osVersion: UIDevice.current.systemVersion, vendorId: "12345678-1234-1234-1234-1234567890AB", isDebugging: false, systemBootTimestamp: Date(), timezoneOffset: 0)
appState.isActive = true
fileManager.store(appState)

Expand Down
2 changes: 1 addition & 1 deletion Tests/SentryTests/Protocol/TestData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class TestData {
static var someUUID = "12345678-1234-1234-1234-12344567890AB"

static var appState: SentryAppState {
return SentryAppState(releaseName: "1.0.0", osVersion: "14.4.1", vendorId: someUUID, isDebugging: false, systemBootTimestamp: timestamp)
return SentryAppState(releaseName: "1.0.0", osVersion: "14.4.1", vendorId: someUUID, isDebugging: false, systemBootTimestamp: timestamp, timezoneOffset: 0)
}

static var oomEvent: Event {
Expand Down