Skip to content

perf(core): [Logs and Metrics Enable Flags 11] Avoid unused Logs worker thread - #5952

Open
adinauer wants to merge 3 commits into
feat/warn-legacy-logs-springfrom
perf/logs-batch-thread-first-use
Open

perf(core): [Logs and Metrics Enable Flags 11] Avoid unused Logs worker thread#5952
adinauer wants to merge 3 commits into
feat/warn-legacy-logs-springfrom
perf/logs-batch-thread-first-use

Conversation

@adinauer

@adinauer adinauer commented Aug 13, 2026

Copy link
Copy Markdown
Member

PR Stack (Logs and Metrics Enable Flags)


📜 Description

Tracks whether LoggerBatchProcessor has accepted its first Log item. Before that point, empty flushes and restart closes do not schedule processor work. Normal close still closes the executor directly.

After the first accepted item, batching, flushing, and restart-close behavior remain unchanged. Items rejected because of shutdown or queue capacity do not mark the processor as used.

💡 Motivation and Context

The aggregate Logs enable flag has been removed, so every SDK client now owns a logger batch processor. Without this guard, lifecycle flushes—particularly Android background callbacks—can start a worker thread even when the application never captures a Log.

💚 How did you test it?

  • ./gradlew :sentry:test :sentry-android-core:testReleaseUnitTest
  • ./gradlew spotlessApply apiDump
  • Added core tests for construction, empty flush, close, restart close, rejected items, and behavior after first use
  • Added an Android background callback test before first use

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Remove the aggregate Metrics enable flag.

#skip-changelog

⚠️ Merge this PR using a merge commit (not squash). Only the collection branch is squash-merged into main.

Track whether the logger batch processor has accepted an item and skip empty flush and restart-close scheduling until then. This prevents SDK initialization and Android background callbacks from starting a worker thread when Logs are unused.

Co-Authored-By: Claude <noreply@anthropic.com>
@sentry

sentry Bot commented Aug 13, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.52.0 (1) release

⚙️ sentry-android Build Distribution Settings

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 314.02 ms 352.98 ms 38.96 ms
Size 0 B 0 B 0 B

Baseline results on branch: feat/warn-legacy-logs-spring

Startup times

Revision Plain With Sentry Diff
c2e4325 310.06 ms 363.22 ms 53.16 ms

App size

Revision Plain With Sentry Diff
c2e4325 0 B 0 B 0 B

Previous results on branch: perf/logs-batch-thread-first-use

Startup times

Revision Plain With Sentry Diff
31a945a 307.74 ms 351.86 ms 44.11 ms

App size

Revision Plain With Sentry Diff
31a945a 0 B 0 B 0 B

Comment on lines 78 to 80
queue.offer(logEvent);
hasAcceptedItem = true;
maybeSchedule(false);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: A race condition between add() and close() can cause log events to be lost if an event is added after the executor has been shut down but before the queue is drained.
Severity: MEDIUM

Suggested Fix

Synchronize access to the shutdown state and executor operations. For example, use a synchronized block around the shutdown check and item queuing in add() and the entire close() method to ensure that add() cannot proceed while a shutdown is in progress.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java#L78-L80

Potential issue: A race condition exists between the `add()` and `close()` methods in
`LoggerBatchProcessor`. A thread calling `add()` can pass the `isShuttingDown` check
just before another thread calling `close()` sets `isShuttingDown` to true and shuts
down the `executorService`. The first thread then adds a log event to the queue but
fails to schedule a flush task because the executor is closed, leading to a
`RejectedExecutionException`. The event remains in the queue but is never processed
because the queue drain in `close()` may have already completed, resulting in the silent
loss of the log event.

Also affects:

  • sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java:116~124

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant