Skip to content

Commit ec49153

Browse files
runningcodeclaude
andcommitted
perf: Share one executor between log and metrics batch processors (JAVA-653)
The log and metrics batch processors each created their own SentryExecutorService, so an app using both spawned two threads for work that is identical in shape (a 5s flush loop that hands envelopes to the transport). SentryClient now owns a single executor, created only when logs or metrics are enabled, and injects it into both processors via the default factories. The restart shutdown path tolerates the shared executor already being closed by the sibling processor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ae3e4c7 commit ec49153

7 files changed

Lines changed: 80 additions & 5 deletions

File tree

sentry/api/sentry.api

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3018,6 +3018,7 @@ public final class io/sentry/SentryClient : io/sentry/ISentryClient {
30183018
public fun close ()V
30193019
public fun close (Z)V
30203020
public fun flush (J)V
3021+
public fun getBatchProcessorExecutorService ()Lio/sentry/ISentryExecutorService;
30213022
public fun getRateLimiter ()Lio/sentry/transport/RateLimiter;
30223023
public fun isEnabled ()Z
30233024
public fun isHealthy ()Z
@@ -5475,6 +5476,7 @@ public class io/sentry/metrics/MetricsBatchProcessor : io/sentry/metrics/IMetric
54755476
public static final field MAX_QUEUE_SIZE I
54765477
protected final field options Lio/sentry/SentryOptions;
54775478
public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/ISentryClient;)V
5479+
public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/ISentryClient;Lio/sentry/ISentryExecutorService;)V
54785480
public fun add (Lio/sentry/SentryMetricsEvent;)V
54795481
public fun close (Z)V
54805482
public fun flush (J)V

sentry/src/main/java/io/sentry/SentryClient.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ public final class SentryClient implements ISentryClient {
4545
private final @NotNull ILoggerBatchProcessor loggerBatchProcessor;
4646
private final @NotNull IMetricsBatchProcessor metricsBatchProcessor;
4747

48+
// Single executor shared by the log and metrics batch processors, so they don't spawn a thread
49+
// each. Created only when logs or metrics are enabled, and closed when those processors close.
50+
private final @Nullable ISentryExecutorService batchProcessorExecutorService;
51+
4852
@Override
4953
public boolean isEnabled() {
5054
return enabled;
@@ -62,6 +66,14 @@ public SentryClient(final @NotNull SentryOptions options) {
6266

6367
final RequestDetailsResolver requestDetailsResolver = new RequestDetailsResolver(options);
6468
transport = transportFactory.create(options, requestDetailsResolver.resolve());
69+
70+
// must be set before the batch processors are created, as they read it from this client
71+
if (options.getLogs().isEnabled() || options.getMetrics().isEnabled()) {
72+
batchProcessorExecutorService = new SentryExecutorService(options);
73+
} else {
74+
batchProcessorExecutorService = null;
75+
}
76+
6577
if (options.getLogs().isEnabled()) {
6678
loggerBatchProcessor =
6779
options.getLogs().getLoggerBatchProcessorFactory().create(options, this);
@@ -76,6 +88,16 @@ public SentryClient(final @NotNull SentryOptions options) {
7688
}
7789
}
7890

91+
/**
92+
* The executor shared by the log and metrics batch processors. Only present (non-null) when logs
93+
* or metrics are enabled, which is the only time the batch processors request it.
94+
*/
95+
@ApiStatus.Internal
96+
public @NotNull ISentryExecutorService getBatchProcessorExecutorService() {
97+
return Objects.requireNonNull(
98+
batchProcessorExecutorService, "batch processor executor service is not available");
99+
}
100+
79101
private boolean shouldApplyScopeData(
80102
final @NotNull SentryBaseEvent event, final @NotNull Hint hint) {
81103
if (HintUtils.shouldApplyScopeData(hint)) {

sentry/src/main/java/io/sentry/logger/DefaultLoggerBatchProcessorFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ public final class DefaultLoggerBatchProcessorFactory implements ILoggerBatchPro
88
@Override
99
public @NotNull ILoggerBatchProcessor create(
1010
@NotNull SentryOptions options, @NotNull SentryClient client) {
11-
return new LoggerBatchProcessor(options, client);
11+
return new LoggerBatchProcessor(options, client, client.getBatchProcessorExecutorService());
1212
}
1313
}

sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,13 @@ public void close(final boolean isRestarting) {
8484
isShuttingDown = true;
8585
if (isRestarting) {
8686
maybeSchedule(true);
87-
executorService.submit(() -> executorService.close(options.getShutdownTimeoutMillis()));
87+
try {
88+
executorService.submit(() -> executorService.close(options.getShutdownTimeoutMillis()));
89+
} catch (RejectedExecutionException e) {
90+
// the shared executor may already be shutting down (e.g. closed by the metrics batch
91+
// processor); close it directly instead
92+
executorService.close(options.getShutdownTimeoutMillis());
93+
}
8894
} else {
8995
executorService.close(options.getShutdownTimeoutMillis());
9096
while (!queue.isEmpty()) {

sentry/src/main/java/io/sentry/metrics/DefaultMetricsBatchProcessorFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ public final class DefaultMetricsBatchProcessorFactory implements IMetricsBatchP
88
@Override
99
public @NotNull IMetricsBatchProcessor create(
1010
final @NotNull SentryOptions options, final @NotNull SentryClient client) {
11-
return new MetricsBatchProcessor(options, client);
11+
return new MetricsBatchProcessor(options, client, client.getBatchProcessorExecutorService());
1212
}
1313
}

sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
import java.util.concurrent.RejectedExecutionException;
2020
import java.util.concurrent.TimeUnit;
2121
import java.util.concurrent.atomic.AtomicBoolean;
22+
import org.jetbrains.annotations.ApiStatus;
2223
import org.jetbrains.annotations.NotNull;
2324
import org.jetbrains.annotations.Nullable;
25+
import org.jetbrains.annotations.TestOnly;
2426

2527
@Open
2628
public class MetricsBatchProcessor implements IMetricsBatchProcessor {
@@ -40,10 +42,19 @@ public class MetricsBatchProcessor implements IMetricsBatchProcessor {
4042

4143
public MetricsBatchProcessor(
4244
final @NotNull SentryOptions options, final @NotNull ISentryClient client) {
45+
this(options, client, new SentryExecutorService(options));
46+
}
47+
48+
@ApiStatus.Internal
49+
@TestOnly
50+
public MetricsBatchProcessor(
51+
final @NotNull SentryOptions options,
52+
final @NotNull ISentryClient client,
53+
final @NotNull ISentryExecutorService executorService) {
4354
this.options = options;
4455
this.client = client;
4556
this.queue = new ConcurrentLinkedQueue<>();
46-
this.executorService = new SentryExecutorService(options);
57+
this.executorService = executorService;
4758
}
4859

4960
@Override
@@ -74,7 +85,13 @@ public void close(final boolean isRestarting) {
7485
isShuttingDown = true;
7586
if (isRestarting) {
7687
maybeSchedule(true);
77-
executorService.submit(() -> executorService.close(options.getShutdownTimeoutMillis()));
88+
try {
89+
executorService.submit(() -> executorService.close(options.getShutdownTimeoutMillis()));
90+
} catch (RejectedExecutionException e) {
91+
// the shared executor may already be shutting down (e.g. closed by the log batch
92+
// processor); close it directly instead
93+
executorService.close(options.getShutdownTimeoutMillis());
94+
}
7895
} else {
7996
executorService.close(options.getShutdownTimeoutMillis());
8097
while (!queue.isEmpty()) {

sentry/src/test/java/io/sentry/SentryClientTest.kt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import io.sentry.protocol.SentryTransaction
2929
import io.sentry.protocol.User
3030
import io.sentry.protocol.ViewHierarchy
3131
import io.sentry.test.callMethod
32+
import io.sentry.test.getProperty
3233
import io.sentry.test.injectForField
3334
import io.sentry.transport.ITransport
3435
import io.sentry.transport.ITransportGate
@@ -177,6 +178,33 @@ class SentryClientTest {
177178
assertTrue(sut.isEnabled)
178179
}
179180

181+
@Test
182+
fun `log and metrics batch processors share a single executor`() {
183+
// real default factories (not the fixture mocks) so the shared executor is actually wired
184+
val options =
185+
SentryOptions().apply {
186+
dsn = dsnString
187+
logs.isEnabled = true
188+
metrics.isEnabled = true
189+
}
190+
val client = SentryClient(options)
191+
try {
192+
val loggerExecutor =
193+
client
194+
.getProperty<ILoggerBatchProcessor>("loggerBatchProcessor")
195+
.getProperty<ISentryExecutorService>("executorService")
196+
val metricsExecutor =
197+
client
198+
.getProperty<IMetricsBatchProcessor>("metricsBatchProcessor")
199+
.getProperty<ISentryExecutorService>("executorService")
200+
201+
assertSame(client.batchProcessorExecutorService, loggerExecutor)
202+
assertSame(loggerExecutor, metricsExecutor)
203+
} finally {
204+
client.close()
205+
}
206+
}
207+
180208
@Test
181209
fun `when client is closed with isRestarting false, transport waits`() {
182210
val sut = fixture.getSut { options -> options.logs.isEnabled = true }

0 commit comments

Comments
 (0)