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: Disable SIGTERM reporting by default #4025

Merged
merged 12 commits into from
Jun 3, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 19 additions & 18 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,24 @@ on:
branches:
- main
paths:
- 'Sources/**'
- 'Tests/**'
- 'test-server/**'
- 'Samples/**'
- '.github/workflows/lint.yml'
- 'scripts/ci-select-xcode.sh'
- "Sources/**"
- "Tests/**"
- "test-server/**"
- "Samples/**"
- ".github/workflows/lint.yml"
- "scripts/ci-select-xcode.sh"
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved

pull_request:
paths:
- 'Sources/**'
- 'Tests/**'
- 'test-server/**'
- 'Samples/**'
- '.github/workflows/lint.yml'
- 'scripts/ci-select-xcode.sh'
- 'Sentry.xcodeproj/**'
- '*.podspec'
- 'Gemfile.lock'
- "Sources/**"
- "Tests/**"
- "test-server/**"
- "Samples/**"
- ".github/workflows/lint.yml"
- "scripts/ci-select-xcode.sh"
- "Sentry.xcodeproj/**"
- "*.podspec"
- "Gemfile.lock"

# https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-a-fallback-value
concurrency:
Expand Down Expand Up @@ -50,10 +50,11 @@ jobs:
name: pod lint ${{ matrix.podspec}} ${{ matrix.library_type }} ${{ matrix.platform}}
runs-on: macos-13
strategy:
fail-fast: false
matrix:
podspec: ['Sentry', 'SentrySwiftUI']
platform: ['ios', 'macos', 'tvos', 'watchos']
library_type: ['dynamic', 'static']
podspec: ["Sentry", "SentrySwiftUI"]
platform: ["ios", "macos", "tvos", "watchos"]
library_type: ["dynamic", "static"]

steps:
- uses: actions/checkout@v4
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
- Add start time to network request breadcrumbs (#4008)
- Add C++ exception support for `__cxa_rethrow` (#3996)
- Add beforeCaptureScreenshot callback (#4016)
- Disable SIGTERM reporting by default (#4025). We added support
for SIGTERM reporting in the last release and enabled it by default.
For some users, SIGTERM events were verbose and not actionable.
Therefore, we disable it per default in this release. If you'd like
to receive SIGTERM events, set the option `enableSigtermReporting = true`.

### Improvements

Expand Down
1 change: 1 addition & 0 deletions Samples/iOS-Swift/iOS-Swift/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
options.beforeSend = { event in
return event
}
options.enableSigtermReporting = true
options.beforeCaptureScreenshot = { _ in
return true
}
Expand Down
17 changes: 17 additions & 0 deletions Sources/Sentry/Public/SentryOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,23 @@ NS_SWIFT_NAME(Options)
*/
@property (nonatomic, assign) BOOL enableCrashHandler;

#if !TARGET_OS_WATCH

/**
* When enabled, the SDK reports SIGTERM signals to Sentry.
*
* It's crucial for developers to understand that the OS sends a SIGTERM to their app as a prelude
* to a graceful shutdown, before resorting to a SIGKILL. This SIGKILL, which your app can't catch
* or ignore, is a direct order to terminate your app's process immediately. Developers should be
* aware that their app can receive a SIGTERM in various scenarios, such as CPU or disk overuse,
* watchdog terminations, or when the OS updates your app.
*
* @note The default value is @c NO.
*/
@property (nonatomic, assign) BOOL enableSigtermReporting;

#endif // !TARGET_OS_WATCH

/**
* How many breadcrumbs do you want to keep in memory?
* @note Default is @c 100 .
Expand Down
13 changes: 12 additions & 1 deletion Sources/Sentry/SentryCrashIntegration.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#import "SentryCrashIntegration.h"
#import "SentryCrashInstallationReporter.h"

#include "SentryCrashMonitor_Signal.h"
#import "SentryCrashWrapper.h"
#import "SentryDispatchQueueWrapper.h"
#import "SentryEvent.h"
Expand Down Expand Up @@ -87,7 +89,13 @@ - (BOOL)installWithOptions:(nonnull SentryOptions *)options
self.scopeObserver =
[[SentryCrashScopeObserver alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs];

[self startCrashHandler:options.cacheDirectoryPath];
BOOL enableSigtermReporting = NO;
#if !TARGET_OS_WATCH
enableSigtermReporting = options.enableSigtermReporting;
#endif // !TARGET_OS_WATCH

[self startCrashHandler:options.cacheDirectoryPath
enableSigtermReporting:enableSigtermReporting];

[self configureScope];

Expand All @@ -100,6 +108,7 @@ - (SentryIntegrationOption)integrationOptions
}

- (void)startCrashHandler:(NSString *)cacheDirectory
enableSigtermReporting:(BOOL)enableSigtermReporting
{
void (^block)(void) = ^{
BOOL canSendReports = NO;
Expand All @@ -116,6 +125,8 @@ - (void)startCrashHandler:(NSString *)cacheDirectory
canSendReports = YES;
}

sentrycrashcm_setEnableSigtermReporting(enableSigtermReporting);

[installation install:cacheDirectory];

// We need to send the crashed event together with the crashed session in the same envelope
Expand Down
8 changes: 8 additions & 0 deletions Sources/Sentry/SentryOptions.m
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ - (instancetype)init
self.enabled = YES;
self.shutdownTimeInterval = 2.0;
self.enableCrashHandler = YES;
#if !TARGET_OS_WATCH
self.enableSigtermReporting = NO;
#endif // !TARGET_OS_WATCH
self.diagnosticLevel = kSentryLevelDebug;
self.debug = NO;
self.maxBreadcrumbs = defaultMaxBreadcrumbs;
Expand Down Expand Up @@ -316,6 +319,11 @@ - (BOOL)validateOptions:(NSDictionary<NSString *, id> *)options
[self setBool:options[@"enableCrashHandler"]
block:^(BOOL value) { self->_enableCrashHandler = value; }];

#if !TARGET_OS_WATCH
[self setBool:options[@"enableSigtermReporting"]
block:^(BOOL value) { self->_enableSigtermReporting = value; }];
#endif // !TARGET_OS_WATCH

if ([options[@"maxBreadcrumbs"] isKindOfClass:[NSNumber class]]) {
self.maxBreadcrumbs = [options[@"maxBreadcrumbs"] unsignedIntValue];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
// ============================================================================

static volatile bool g_isEnabled = false;
static bool g_isSigtermReportingEnabled = false;

static SentryCrash_MonitorContext g_monitorContext;
static SentryCrashStackCursor g_stackCursor;
Expand Down Expand Up @@ -163,6 +164,11 @@ installSignalHandler(void)
action.sa_sigaction = &handleSignal;

for (int i = 0; i < fatalSignalsCount; i++) {
if (fatalSignals[i] == SIGTERM && !g_isSigtermReportingEnabled) {
SentryCrashLOG_DEBUG("SIGTERM handling disabled. Skipping assigning handler.");
continue;
}

SentryCrashLOG_DEBUG("Assigning handler for signal %d", fatalSignals[i]);
if (sigaction(fatalSignals[i], &action, &g_previousSignalHandlers[i]) != 0) {
char sigNameBuff[30];
Expand Down Expand Up @@ -203,6 +209,11 @@ uninstallSignalHandler(void)
int fatalSignalsCount = sentrycrashsignal_numFatalSignals();

for (int i = 0; i < fatalSignalsCount; i++) {
if (fatalSignals[i] == SIGTERM && !g_isSigtermReportingEnabled) {
SentryCrashLOG_DEBUG("SIGTERM handling disabled. Skipping restoring handler.");
continue;
}

SentryCrashLOG_DEBUG("Restoring original handler for signal %d", fatalSignals[i]);
sigaction(fatalSignals[i], &g_previousSignalHandlers[i], NULL);
}
Expand Down Expand Up @@ -246,6 +257,14 @@ addContextualInfoToEvent(struct SentryCrash_MonitorContext *eventContext)

#endif

void
sentrycrashcm_setEnableSigtermReporting(bool enabled)
{
#if SentryCrashCRASH_HAS_SIGNAL
g_isSigtermReportingEnabled = enabled;
#endif
}

SentryCrashMonitorAPI *
sentrycrashcm_signal_getAPI(void)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ extern "C" {

#include "SentryCrashMonitor.h"

/** Wether to assign the signal handler for SIGTERM or not.
*/
void sentrycrashcm_setEnableSigtermReporting(bool enabled);

/** Access the Monitor API.
*/
SentryCrashMonitorAPI *sentrycrashcm_signal_getAPI(void);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ static const int g_fatalSignals[] = {
// SIGTERM can be caught and is usually sent by iOS and variants
// when Apple wants to try and gracefully shutdown the app
// before sending a SIGKILL (which can't be caught).
// Some areas I've seen this happen are:
// Some areas this might happen are:
// - When the OS updates an app.
// - In some circumstances for Watchdog Events.
// - Resource overuse (CPU, Disk, ...).
Expand Down
5 changes: 5 additions & 0 deletions Tests/SentryTests/SentryOptionsTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ - (void)testDiagnosticlevelWith:(NSObject *)level expected:(SentryLevel)expected
XCTAssertEqual(expected, options.diagnosticLevel);
}

- (void)testEnableSigtermReporting
{
[self testBooleanField:@"enableSigtermReporting" defaultValue:NO];
}

- (void)testValidEnabled
{
[self testEnabledWith:@YES expected:YES];
Expand Down
Loading