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

Enable Backpressure Management by Default #3284

Merged
merged 7 commits into from
Apr 9, 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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

## Unreleased

## Features
### Features

- Update normalization of metrics keys, tags and values ([#3332](https://github.com/getsentry/sentry-java/pull/3332))
- Enable backpressure management by default ([#3284](https://github.com/getsentry/sentry-java/pull/3284))

### Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class SentryHandlerTest {
it.dsn = "http://key@localhost/proj"
it.environment = "manual-environment"
it.setTransportFactory { _, _ -> transport }
it.isEnableBackpressureHandling = false
}
fixture = Fixture(transport = transport)
fixture.logger.severe("testing environment field")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class SentryAppenderTest {
it.dsn = "http://key@localhost/proj"
it.environment = "manual-environment"
it.setTransportFactory(fixture.transportFactory)
it.isEnableBackpressureHandling = false
}
val logger = fixture.getSut()
logger.error("testing environment field")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class SentryAppenderTest {
it.dsn = "http://key@localhost/proj"
it.environment = "manual-environment"
it.setTransportFactory(fixture.transportFactory)
it.isEnableBackpressureHandling = false
}
fixture.logger.error("testing environment field")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class SentryAutoConfigurationTest {
"sentry.enabled=false",
"sentry.send-modules=false",
"sentry.ignored-checkins=slug1,slugB",
"sentry.enable-backpressure-handling=true",
"sentry.enable-backpressure-handling=false",
"sentry.cron.default-checkin-margin=10",
"sentry.cron.default-max-runtime=30",
"sentry.cron.default-timezone=America/New_York",
Expand Down Expand Up @@ -206,7 +206,7 @@ class SentryAutoConfigurationTest {
assertThat(options.isEnabled).isEqualTo(false)
assertThat(options.isSendModules).isEqualTo(false)
assertThat(options.ignoredCheckIns).containsOnly("slug1", "slugB")
assertThat(options.isEnableBackpressureHandling).isEqualTo(true)
assertThat(options.isEnableBackpressureHandling).isEqualTo(false)
assertThat(options.cron).isNotNull
assertThat(options.cron!!.defaultCheckinMargin).isEqualTo(10L)
assertThat(options.cron!!.defaultMaxRuntime).isEqualTo(30L)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import kotlin.test.Test
@SpringBootTest(
classes = [App::class],
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
properties = ["sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true", "sentry.traces-sample-rate=1.0", "sentry.max-request-body-size=medium"]
properties = ["sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true", "sentry.traces-sample-rate=1.0", "sentry.max-request-body-size=medium", "sentry.enable-backpressure-handling=false"]
)
class SentrySpringIntegrationTest {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class SentryAutoConfigurationTest {
"sentry.enabled=false",
"sentry.send-modules=false",
"sentry.ignored-checkins=slug1,slugB",
"sentry.enable-backpressure-handling=true",
"sentry.enable-backpressure-handling=false",
"sentry.cron.default-checkin-margin=10",
"sentry.cron.default-max-runtime=30",
"sentry.cron.default-timezone=America/New_York",
Expand Down Expand Up @@ -205,7 +205,7 @@ class SentryAutoConfigurationTest {
assertThat(options.isEnabled).isEqualTo(false)
assertThat(options.isSendModules).isEqualTo(false)
assertThat(options.ignoredCheckIns).containsOnly("slug1", "slugB")
assertThat(options.isEnableBackpressureHandling).isEqualTo(true)
assertThat(options.isEnableBackpressureHandling).isEqualTo(false)
assertThat(options.cron).isNotNull
assertThat(options.cron!!.defaultCheckinMargin).isEqualTo(10L)
assertThat(options.cron!!.defaultMaxRuntime).isEqualTo(30L)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import kotlin.test.Test
@SpringBootTest(
classes = [App::class],
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
properties = ["sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true", "sentry.traces-sample-rate=1.0", "sentry.max-request-body-size=medium"]
properties = ["sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true", "sentry.traces-sample-rate=1.0", "sentry.max-request-body-size=medium", "sentry.enable-backpressure-handling=false"]
)
class SentrySpringIntegrationTest {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ open class App {
it.setDebug(true)
it.setTransportFactory(transportFactory)
it.enableTracing = true
it.isEnableBackpressureHandling = false
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ private static boolean initConfigurations(final @NotNull SentryOptions options)
options.addPerformanceCollector(new JavaMemoryCollector());
}

if (options.isEnableBackpressureHandling()) {
if (options.isEnableBackpressureHandling() && Platform.isJvm()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Since there's currently no way of disabling backpressure for Android via Metadata I only enable it for backend. There usually shouldn't be as high a volume of transactions on Android.

Copy link
Member Author

Choose a reason for hiding this comment

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

We decided in Backend Daily it's OK to disable backpressure management for Android. If we ever want it we'll have to make it configurable and probably do a separate test phase before enabling it by default there.

options.setBackpressureMonitor(new BackpressureMonitor(options, HubAdapter.getInstance()));
options.getBackpressureMonitor().start();
}
Expand Down
3 changes: 1 addition & 2 deletions sentry/src/main/java/io/sentry/SentryOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,9 @@ public class SentryOptions {
/** Contains a list of monitor slugs for which check-ins should not be sent. */
@ApiStatus.Experimental private @Nullable List<String> ignoredCheckIns = null;

@ApiStatus.Experimental
private @NotNull IBackpressureMonitor backpressureMonitor = NoOpBackpressureMonitor.getInstance();

@ApiStatus.Experimental private boolean enableBackpressureHandling = false;
private boolean enableBackpressureHandling = true;

/** Whether to profile app launches, depending on profilesSampler or profilesSampleRate. */
private boolean enableAppStartProfiling = false;
Expand Down
6 changes: 3 additions & 3 deletions sentry/src/test/java/io/sentry/ExternalOptionsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,9 @@ class ExternalOptionsTest {
}

@Test
fun `creates options with enableBackpressureHandling set to true`() {
withPropertiesFile("enable-backpressure-handling=true") { options ->
assertTrue(options.isEnableBackpressureHandling == true)
fun `creates options with enableBackpressureHandling set to false`() {
withPropertiesFile("enable-backpressure-handling=false") { options ->
assertTrue(options.isEnableBackpressureHandling == false)
}
}

Expand Down
10 changes: 4 additions & 6 deletions sentry/src/test/java/io/sentry/SentryOptionsTest.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.sentry

import io.sentry.backpressure.NoOpBackpressureMonitor
import io.sentry.util.StringUtils
import org.mockito.kotlin.mock
import java.io.File
Expand Down Expand Up @@ -371,7 +370,7 @@ class SentryOptionsTest {
externalOptions.isEnablePrettySerializationOutput = false
externalOptions.isSendModules = false
externalOptions.ignoredCheckIns = listOf("slug1", "slug-B")
externalOptions.isEnableBackpressureHandling = true
externalOptions.isEnableBackpressureHandling = false
externalOptions.cron = SentryOptions.Cron().apply {
defaultCheckinMargin = 10L
defaultMaxRuntime = 30L
Expand Down Expand Up @@ -409,7 +408,7 @@ class SentryOptionsTest {
assertFalse(options.isEnablePrettySerializationOutput)
assertFalse(options.isSendModules)
assertEquals(listOf("slug1", "slug-B"), options.ignoredCheckIns)
assertTrue(options.isEnableBackpressureHandling)
assertFalse(options.isEnableBackpressureHandling)
assertNotNull(options.cron)
assertEquals(10L, options.cron?.defaultCheckinMargin)
assertEquals(30L, options.cron?.defaultMaxRuntime)
Expand Down Expand Up @@ -565,9 +564,8 @@ class SentryOptionsTest {
}

@Test
fun `when options are initialized, enableBackpressureHandling is set to false by default`() {
assertFalse(SentryOptions().isEnableBackpressureHandling)
assertTrue(SentryOptions().backpressureMonitor is NoOpBackpressureMonitor)
fun `when options are initialized, enableBackpressureHandling is set to true by default`() {
assertTrue(SentryOptions().isEnableBackpressureHandling)
}

@Test
Expand Down
Loading