getsentry / sentry-java

A Sentry SDK for Java, Android and other JVM languages.
https://docs.sentry.io/
MIT License
1.14k stars 432 forks source link

Suggestions for internals refactoring #2515

Open marandaneto opened 1 year ago

marandaneto commented 1 year ago

Description

Some places use System.currentTimeMillis() directly (such as AndroidMemoryCollector) instead of CurrentDateProvider which makes it untestable.

DefaultAndroidEventProcessor already reads memory data, but now we also have AndroidMemoryCollector, maybe we could refactor DefaultAndroidEventProcessor to use AndroidMemoryCollector.

CurrentActivityHolder holds the current activity thru the CurrentActivityIntegration, maybe we could refactor other integrations such as ActivityLifecycleIntegration to use the holder as well.

marandaneto commented 1 year ago

Looper.getMainLooper() Also being used directly instead of MainLooperHandler for testability.

marandaneto commented 1 year ago

@romtsn @markushi added to milestone v7, let me know if that's fine. I am afraid that we will keep using the static methods making the code difficult to be tested.

marandaneto commented 1 year ago

Change activity.runOnUiThread to using MainLooperHandler as well.

markushi commented 1 year ago

Let's remove the milestone and re-priortize this.

markushi commented 1 year ago

Let's check if there are any breaking API changes due to the mentioned refactorings.

romtsn commented 1 year ago

Let's call each other out when we review PRs and try to fix this as we go