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

fix: read free_memory when the event is captured, not only at SDK startup #1962

Merged
merged 14 commits into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Features

- Read free_memory when the event is captured, not only at SDK startup (#1962)

### Fixes

- Remove Sentry keys from cached HTTP request headers (#1975)
Expand Down
33 changes: 29 additions & 4 deletions Sources/Sentry/SentryClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#import "SentryCrashDefaultMachineContextWrapper.h"
#import "SentryCrashIntegration.h"
#import "SentryCrashStackEntryMapper.h"
#import "SentryCrashWrapper.h"
#import "SentryDebugImageProvider.h"
#import "SentryDefaultCurrentDateProvider.h"
#import "SentryDependencyContainer.h"
Expand Down Expand Up @@ -55,6 +56,7 @@
@property (nonatomic, strong) SentryThreadInspector *threadInspector;
@property (nonatomic, strong) id<SentryRandom> random;
@property (nonatomic, weak) id<SentryClientAttachmentProcessor> attachmentProcessor;
@property (nonatomic, strong) SentryCrashWrapper *crashWrapper;

@end

Expand Down Expand Up @@ -101,6 +103,8 @@ - (_Nullable instancetype)initWithOptions:(SentryOptions *)options
options:options];

self.random = [SentryDependencyContainer sharedInstance].random;

self.crashWrapper = [SentryCrashWrapper sharedInstance];
}
return self;
}
Expand All @@ -111,13 +115,15 @@ - (instancetype)initWithOptions:(SentryOptions *)options
fileManager:(SentryFileManager *)fileManager
threadInspector:(SentryThreadInspector *)threadInspector
random:(id<SentryRandom>)random
crashWrapper:(SentryCrashWrapper *)crashWrapper
{
self = [self initWithOptions:options];

self.transportAdapter = transportAdapter;
self.fileManager = fileManager;
self.threadInspector = threadInspector;
self.random = random;
self.crashWrapper = crashWrapper;

return self;
}
Expand Down Expand Up @@ -487,10 +493,13 @@ - (SentryEvent *_Nullable)prepareEvent:(SentryEvent *)event

event = [scope applyToEvent:event maxBreadcrumb:self.options.maxBreadcrumbs];

// Remove free_memory if OOM as free_memory stems from the current run and not of the time of
// the OOM.
if ([self isOOM:event isCrashEvent:isCrashEvent]) {
// Remove free_memory if OOM as free_memory stems from the current run and not of
// the time of the OOM.
[self removeFreeMemoryFromDeviceContext:event];
} else {
// Store the actual free memory at the time of this event
[self storeFreeMemoryToDeviceContext:event];
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved
}

// With scope applied, before running callbacks run:
Expand Down Expand Up @@ -638,7 +647,24 @@ - (BOOL)isOOM:(SentryEvent *)event isCrashEvent:(BOOL)isCrashEvent
[exception.mechanism.type isEqualToString:SentryOutOfMemoryMechanismType];
}

- (void)storeFreeMemoryToDeviceContext:(SentryEvent *)event
kevinrenskers marked this conversation as resolved.
Show resolved Hide resolved
{
[self modifyDeviceContext:event
block:^(NSMutableDictionary *device) {
device[SentryDeviceContextFreeMemoryKey] =
@(self.crashWrapper.freeMemory);
}];
}

- (void)removeFreeMemoryFromDeviceContext:(SentryEvent *)event
{
[self modifyDeviceContext:event
block:^(NSMutableDictionary *device) {
[device removeObjectForKey:SentryDeviceContextFreeMemoryKey];
}];
}

- (void)modifyDeviceContext:(SentryEvent *)event block:(void (^)(NSMutableDictionary *))block
{
if (nil == event.context || event.context.count == 0 || nil == event.context[@"device"]) {
return;
Expand All @@ -647,9 +673,8 @@ - (void)removeFreeMemoryFromDeviceContext:(SentryEvent *)event
NSMutableDictionary *context = [[NSMutableDictionary alloc] initWithDictionary:event.context];
NSMutableDictionary *device =
[[NSMutableDictionary alloc] initWithDictionary:context[@"device"]];
[device removeObjectForKey:SentryDeviceContextFreeMemoryKey];
block(device);
context[@"device"] = device;

event.context = context;
}

Expand Down
6 changes: 6 additions & 0 deletions Sources/Sentry/SentryCrashWrapper.m
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#import "SentryCrashWrapper.h"
#import "SentryCrash.h"
#import "SentryCrashMonitor_AppState.h"
#import "SentryCrashMonitor_System.h"
#import "SentryHook.h"
#import <Foundation/Foundation.h>
#import <SentryCrashCachedData.h>
Expand Down Expand Up @@ -69,6 +70,11 @@ - (NSDictionary *)systemInfo
return sharedInfo;
}

- (NSInteger)freeMemory
{
return sentrycrashcm_system_freememory();
}

@end

NS_ASSUME_NONNULL_END
2 changes: 2 additions & 0 deletions Sources/Sentry/include/SentryCrashWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ SENTRY_NO_INIT

- (NSDictionary *)systemInfo;

- (NSInteger)freeMemory;

@end

NS_ASSUME_NONNULL_END
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ SentryCrashMonitorAPI *sentrycrashcm_system_getAPI(void);

bool sentrycrash_isSimulatorBuild(void);

uint64_t sentrycrashcm_system_freememory(void);

#ifdef __cplusplus
}
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,12 @@
return 0;
}

uint64_t
sentrycrashcm_system_freememory(void)
{
return freeMemory();
}

static uint64_t
usableMemory(void)
{
Expand Down
3 changes: 2 additions & 1 deletion Tests/SentryTests/SentryClient+TestInit.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ SentryClient (TestInit)
transportAdapter:(SentryTransportAdapter *)transportAdapter
fileManager:(SentryFileManager *)fileManager
threadInspector:(SentryThreadInspector *)threadInspector
random:(id<SentryRandom>)random;
random:(id<SentryRandom>)random
crashWrapper:(SentryCrashWrapper *)crashWrapper;

@end

Expand Down
17 changes: 16 additions & 1 deletion Tests/SentryTests/SentryClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class SentryClientTest: XCTestCase {

let trace = SentryTracer(transactionContext: TransactionContext(name: "SomeTransaction", operation: "SomeOperation"), hub: nil)
let transaction: Transaction
let crashWrapper = TestSentryCrashWrapper.sharedInstance()

init() {
session = SentrySession(releaseName: "release")
Expand Down Expand Up @@ -59,7 +60,7 @@ class SentryClientTest: XCTestCase {
])
configureOptions(options)

client = Client(options: options, transportAdapter: transportAdapter, fileManager: fileManager, threadInspector: threadInspector, random: random)
client = Client(options: options, transportAdapter: transportAdapter, fileManager: fileManager, threadInspector: threadInspector, random: random, crashWrapper: crashWrapper)
} catch {
XCTFail("Options could not be created")
}
Expand Down Expand Up @@ -497,6 +498,20 @@ class SentryClientTest: XCTestCase {
}
}

func testCaptureCrash_FreeMemory() {
let event = TestData.event
event.threads = nil
event.debugMeta = nil

fixture.crashWrapper.internalFreeMemory = 123_456
fixture.getSut().captureCrash(event, with: fixture.scope)

assertLastSentEventWithAttachment { actual in
let eventFreeMemory = actual.context?["device"]?["free_memory"] as? Int
XCTAssertEqual(eventFreeMemory, 123_456)
}
}

func testCaptureErrorWithUserInfo() {
let expectedValue = "val"
let error = NSError(domain: "domain", code: 0, userInfo: ["key": expectedValue])
Expand Down
2 changes: 2 additions & 0 deletions Tests/SentryTests/SentryCrash/TestSentryCrashWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ SENTRY_NO_INIT

@property (nonatomic, assign) BOOL closeCalled;

@property (nonatomic, assign) NSInteger internalFreeMemory;

@end

NS_ASSUME_NONNULL_END
6 changes: 6 additions & 0 deletions Tests/SentryTests/SentryCrash/TestSentryCrashWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ + (instancetype)sharedInstance
instance.internalIsApplicationInForeground = YES;
instance.installAsyncHooksCalled = NO;
instance.closeCalled = NO;
instance.internalFreeMemory = 0;
return instance;
}

Expand Down Expand Up @@ -57,4 +58,9 @@ - (NSDictionary *)systemInfo
return @{};
}

- (NSInteger)freeMemory
{
return self.internalFreeMemory;
}

@end