Skip to content

Commit

Permalink
feat: Disable SIGTERM reporting by default (#4025)
Browse files Browse the repository at this point in the history
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 by default.

Fixes GH-4013
  • Loading branch information
philipphofmann authored Jun 3, 2024
1 parent 9fef8d1 commit 5eaadc5
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 20 deletions.
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"

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
19 changes: 19 additions & 0 deletions Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_Signal.c
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

0 comments on commit 5eaadc5

Please sign in to comment.