Skip to content

Conversation

@adinauer
Copy link
Member

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 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

Copy link
Member Author

adinauer commented Dec 22, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 22, 2025

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

### Features

- [Trace Metrics 15] Android Metrics batch processor and factory ([#4994](https://github.com/getsentry/sentry-java/pull/4994))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description or adding a skip-changelog label.

Generated by 🚫 dangerJS against 56879b3

@adinauer adinauer changed the title Android Metrics batch processor and factory feat(metrics): [Trace Metrics 15] Android Metrics batch processor and factory Dec 22, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 22, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 320.31 ms 353.78 ms 33.47 ms
Size 1.58 MiB 2.20 MiB 635.46 KiB

@adinauer adinauer force-pushed the 12-19-metrics_manifest_options_for_android branch from 6788ab9 to 8a6b00f Compare December 22, 2025 13:41
@adinauer adinauer force-pushed the 12-19-android_metrics_batch_processor_and_factory branch from c3c0f3b to 0116a6b Compare December 22, 2025 13:41
}

@Override
public void onForeground() {
Copy link

Choose a reason for hiding this comment

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

Bug: The SentryClient does not call close() or flush() on the new metricsBatchProcessor during shutdown, leading to memory leaks, data loss, and potential runtime errors.
Severity: CRITICAL

Suggested Fix

In SentryClient.java, add a call to metricsBatchProcessor.flush(timeoutMillis) within the flush() method and a call to metricsBatchProcessor.close(isRestarting) within the close() method. This will mirror the existing cleanup logic for loggerBatchProcessor.

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-android-core/src/main/java/io/sentry/android/core/AndroidMetricsBatchProcessor.java#L21

Potential issue: When metrics are enabled, the `SentryClient` initializes a
`metricsBatchProcessor` but fails to clean it up during shutdown. The
`SentryClient.close()` method does not call `metricsBatchProcessor.close()`, and
`SentryClient.flush()` does not call `metricsBatchProcessor.flush()`. This leads to
several issues on Android: a memory leak because the `AndroidMetricsBatchProcessor`
remains registered as an `AppStateListener`, potential runtime errors if the app
backgrounds after the client is closed, and data loss as queued metrics are never
flushed.

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.

3 participants