Skip to content

Commit 8838e01

Browse files
krystofwoldrichkrystofwoldrich
authored
feat(profiling): Remove SentryOptions deps from AndroidProfiler (#3051)
1 parent f461caa commit 8838e01

File tree

8 files changed

+89
-53
lines changed

8 files changed

+89
-53
lines changed

‎CHANGELOG.md

+6-2
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,16 @@
22

33
## Unreleased
44

5+
### Features
6+
7+
- (Internal) Extract Android Profiler and Measurements for Hybrid SDKs ([#3016](https://github.com/getsentry/sentry-java/pull/3016))
8+
- (Internal) Remove SentryOptions dependency from AndroidProfiler ([#3051](https://github.com/getsentry/sentry-java/pull/3051))
9+
- (Internal) Add `readBytesFromFile` for use in Hybrid SDKs ([#3052](https://github.com/getsentry/sentry-java/pull/3052))
10+
511
### Fixes
612

713
- Fix SIGSEV, SIGABRT and SIGBUS crashes happening after/around the August Google Play System update, see [#2955](https://github.com/getsentry/sentry-java/issues/2955) for more details (fix provided by Native SDK bump)
8-
- (Internal) Extract Android Profiler and Measurements for Hybrid SDKs ([#3016](https://github.com/getsentry/sentry-java/pull/3016))
914
- Ensure DSN uses http/https protocol ([#3044](https://github.com/getsentry/sentry-java/pull/3044))
10-
- (Internal) Add `readBytesFromFile` for use in Hybrid SDKs ([#3052](https://github.com/getsentry/sentry-java/pull/3052))
1115

1216
### Dependencies
1317

‎sentry-android-core/api/sentry-android-core.api

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public class io/sentry/android/core/AndroidMemoryCollector : io/sentry/ICollecto
5050
}
5151

5252
public class io/sentry/android/core/AndroidProfiler {
53-
public fun <init> (Ljava/lang/String;ILio/sentry/android/core/internal/util/SentryFrameMetricsCollector;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;)V
53+
public fun <init> (Ljava/lang/String;ILio/sentry/android/core/internal/util/SentryFrameMetricsCollector;Lio/sentry/ISentryExecutorService;Lio/sentry/ILogger;Lio/sentry/android/core/BuildInfoProvider;)V
5454
public fun close ()V
5555
public fun endAndCollect (ZLjava/util/List;)Lio/sentry/android/core/AndroidProfiler$ProfileEndData;
5656
public fun start ()Lio/sentry/android/core/AndroidProfiler$ProfileStartData;

‎sentry-android-core/src/main/java/io/sentry/android/core/AndroidProfiler.java

+21-25
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import android.os.Process;
77
import android.os.SystemClock;
88
import io.sentry.CpuCollectionData;
9+
import io.sentry.ILogger;
10+
import io.sentry.ISentryExecutorService;
911
import io.sentry.MemoryCollectionData;
1012
import io.sentry.PerformanceCollectionData;
1113
import io.sentry.SentryLevel;
@@ -86,20 +88,23 @@ public ProfileEndData(
8688
private final @NotNull ArrayDeque<ProfileMeasurementValue> frozenFrameRenderMeasurements =
8789
new ArrayDeque<>();
8890
private final @NotNull Map<String, ProfileMeasurement> measurementsMap = new HashMap<>();
89-
private final @NotNull SentryAndroidOptions options;
9091
private final @NotNull BuildInfoProvider buildInfoProvider;
92+
private final @NotNull ISentryExecutorService executorService;
93+
private final @NotNull ILogger logger;
9194
private boolean isRunning = false;
9295

9396
public AndroidProfiler(
9497
final @NotNull String tracesFilesDirPath,
9598
final int intervalUs,
9699
final @NotNull SentryFrameMetricsCollector frameMetricsCollector,
97-
final @NotNull SentryAndroidOptions sentryAndroidOptions,
100+
final @NotNull ISentryExecutorService executorService,
101+
final @NotNull ILogger logger,
98102
final @NotNull BuildInfoProvider buildInfoProvider) {
99103
this.traceFilesDir =
100104
new File(Objects.requireNonNull(tracesFilesDirPath, "TracesFilesDirPath is required"));
101105
this.intervalUs = intervalUs;
102-
this.options = Objects.requireNonNull(sentryAndroidOptions, "SentryAndroidOptions is required");
106+
this.logger = Objects.requireNonNull(logger, "Logger is required");
107+
this.executorService = Objects.requireNonNull(executorService, "ExecutorService is required.");
103108
this.frameMetricsCollector =
104109
Objects.requireNonNull(frameMetricsCollector, "SentryFrameMetricsCollector is required");
105110
this.buildInfoProvider =
@@ -110,17 +115,13 @@ public AndroidProfiler(
110115
public synchronized @Nullable ProfileStartData start() {
111116
// intervalUs is 0 only if there was a problem in the init
112117
if (intervalUs == 0) {
113-
options
114-
.getLogger()
115-
.log(
116-
SentryLevel.WARNING,
117-
"Disabling profiling because intervaUs is set to %d",
118-
intervalUs);
118+
logger.log(
119+
SentryLevel.WARNING, "Disabling profiling because intervaUs is set to %d", intervalUs);
119120
return null;
120121
}
121122

122123
if (isRunning) {
123-
options.getLogger().log(SentryLevel.WARNING, "Profiling has already started...");
124+
logger.log(SentryLevel.WARNING, "Profiling has already started...");
124125
return null;
125126
}
126127

@@ -182,18 +183,13 @@ public void onFrameMetricCollected(
182183
// We stop profiling after a timeout to avoid huge profiles to be sent
183184
try {
184185
scheduledFinish =
185-
options
186-
.getExecutorService()
187-
.schedule(
188-
() -> timedOutProfilingData = endAndCollect(true, null),
189-
PROFILING_TIMEOUT_MILLIS);
186+
executorService.schedule(
187+
() -> timedOutProfilingData = endAndCollect(true, null), PROFILING_TIMEOUT_MILLIS);
190188
} catch (RejectedExecutionException e) {
191-
options
192-
.getLogger()
193-
.log(
194-
SentryLevel.ERROR,
195-
"Failed to call the executor. Profiling will not be automatically finished. Did you call Sentry.close()?",
196-
e);
189+
logger.log(
190+
SentryLevel.ERROR,
191+
"Failed to call the executor. Profiling will not be automatically finished. Did you call Sentry.close()?",
192+
e);
197193
}
198194

199195
transactionStartNanos = SystemClock.elapsedRealtimeNanos();
@@ -211,7 +207,7 @@ public void onFrameMetricCollected(
211207
return new ProfileStartData(transactionStartNanos, profileStartCpuMillis);
212208
} catch (Throwable e) {
213209
endAndCollect(false, null);
214-
options.getLogger().log(SentryLevel.ERROR, "Unable to start a profile: ", e);
210+
logger.log(SentryLevel.ERROR, "Unable to start a profile: ", e);
215211
isRunning = false;
216212
return null;
217213
}
@@ -227,7 +223,7 @@ public void onFrameMetricCollected(
227223
}
228224

229225
if (!isRunning) {
230-
options.getLogger().log(SentryLevel.WARNING, "Profiler not running");
226+
logger.log(SentryLevel.WARNING, "Profiler not running");
231227
return null;
232228
}
233229

@@ -240,7 +236,7 @@ public void onFrameMetricCollected(
240236
// throws)
241237
Debug.stopMethodTracing();
242238
} catch (Throwable e) {
243-
options.getLogger().log(SentryLevel.ERROR, "Error while stopping profiling: ", e);
239+
logger.log(SentryLevel.ERROR, "Error while stopping profiling: ", e);
244240
} finally {
245241
isRunning = false;
246242
}
@@ -250,7 +246,7 @@ public void onFrameMetricCollected(
250246
long transactionEndCpuMillis = Process.getElapsedCpuTime();
251247

252248
if (traceFile == null) {
253-
options.getLogger().log(SentryLevel.ERROR, "Trace file does not exists");
249+
logger.log(SentryLevel.ERROR, "Trace file does not exists");
254250
return null;
255251
}
256252

‎sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ private void init() {
104104
tracesFilesDirPath,
105105
(int) SECONDS.toMicros(1) / intervalHz,
106106
frameMetricsCollector,
107-
options,
107+
options.getExecutorService(),
108+
options.getLogger(),
108109
buildInfoProvider);
109110
}
110111

‎sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java

+29-16
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import android.view.FrameMetrics;
1414
import android.view.Window;
1515
import androidx.annotation.RequiresApi;
16+
import io.sentry.ILogger;
1617
import io.sentry.SentryLevel;
1718
import io.sentry.SentryOptions;
1819
import io.sentry.android.core.BuildInfoProvider;
@@ -32,7 +33,7 @@
3233
public final class SentryFrameMetricsCollector implements Application.ActivityLifecycleCallbacks {
3334
private final @NotNull BuildInfoProvider buildInfoProvider;
3435
private final @NotNull Set<Window> trackedWindows = new CopyOnWriteArraySet<>();
35-
private final @NotNull SentryOptions options;
36+
private final @NotNull ILogger logger;
3637
private @Nullable Handler handler;
3738
private @Nullable WeakReference<Window> currentWindow;
3839
private final @NotNull Map<String, FrameMetricsCollectorListener> listenerMap =
@@ -54,15 +55,33 @@ public SentryFrameMetricsCollector(
5455
this(context, options, buildInfoProvider, new WindowFrameMetricsManager() {});
5556
}
5657

58+
@SuppressLint("NewApi")
59+
public SentryFrameMetricsCollector(
60+
final @NotNull Context context,
61+
final @NotNull ILogger logger,
62+
final @NotNull BuildInfoProvider buildInfoProvider) {
63+
this(context, logger, buildInfoProvider, new WindowFrameMetricsManager() {});
64+
}
65+
5766
@SuppressWarnings("deprecation")
5867
@SuppressLint({"NewApi", "DiscouragedPrivateApi"})
5968
public SentryFrameMetricsCollector(
6069
final @NotNull Context context,
6170
final @NotNull SentryOptions options,
6271
final @NotNull BuildInfoProvider buildInfoProvider,
6372
final @NotNull WindowFrameMetricsManager windowFrameMetricsManager) {
73+
this(context, options.getLogger(), buildInfoProvider, windowFrameMetricsManager);
74+
}
75+
76+
@SuppressWarnings("deprecation")
77+
@SuppressLint({"NewApi", "DiscouragedPrivateApi"})
78+
public SentryFrameMetricsCollector(
79+
final @NotNull Context context,
80+
final @NotNull ILogger logger,
81+
final @NotNull BuildInfoProvider buildInfoProvider,
82+
final @NotNull WindowFrameMetricsManager windowFrameMetricsManager) {
6483
Objects.requireNonNull(context, "The context is required");
65-
this.options = Objects.requireNonNull(options, "SentryOptions is required");
84+
this.logger = Objects.requireNonNull(logger, "Logger is required");
6685
this.buildInfoProvider =
6786
Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
6887
this.windowFrameMetricsManager =
@@ -81,8 +100,7 @@ public SentryFrameMetricsCollector(
81100
HandlerThread handlerThread =
82101
new HandlerThread("io.sentry.android.core.internal.util.SentryFrameMetricsCollector");
83102
handlerThread.setUncaughtExceptionHandler(
84-
(thread, e) ->
85-
options.getLogger().log(SentryLevel.ERROR, "Error during frames measurements.", e));
103+
(thread, e) -> logger.log(SentryLevel.ERROR, "Error during frames measurements.", e));
86104
handlerThread.start();
87105
handler = new Handler(handlerThread.getLooper());
88106

@@ -100,22 +118,19 @@ public SentryFrameMetricsCollector(
100118
try {
101119
choreographer = Choreographer.getInstance();
102120
} catch (Throwable e) {
103-
options
104-
.getLogger()
105-
.log(
106-
SentryLevel.ERROR,
107-
"Error retrieving Choreographer instance. Slow and frozen frames will not be reported.",
108-
e);
121+
logger.log(
122+
SentryLevel.ERROR,
123+
"Error retrieving Choreographer instance. Slow and frozen frames will not be reported.",
124+
e);
109125
}
110126
});
111127
// Let's get the last frame timestamp from the choreographer private field
112128
try {
113129
choreographerLastFrameTimeField = Choreographer.class.getDeclaredField("mLastFrameTimeNanos");
114130
choreographerLastFrameTimeField.setAccessible(true);
115131
} catch (NoSuchFieldException e) {
116-
options
117-
.getLogger()
118-
.log(SentryLevel.ERROR, "Unable to get the frame timestamp from the choreographer: ", e);
132+
logger.log(
133+
SentryLevel.ERROR, "Unable to get the frame timestamp from the choreographer: ", e);
119134
}
120135
frameMetricsAvailableListener =
121136
(window, frameMetrics, dropCountSinceLastInvocation) -> {
@@ -247,9 +262,7 @@ private void stopTrackingWindow(final @NotNull Window window) {
247262
windowFrameMetricsManager.removeOnFrameMetricsAvailableListener(
248263
window, frameMetricsAvailableListener);
249264
} catch (Exception e) {
250-
options
251-
.getLogger()
252-
.log(SentryLevel.ERROR, "Failed to remove frameMetricsAvailableListener", e);
265+
logger.log(SentryLevel.ERROR, "Failed to remove frameMetricsAvailableListener", e);
253266
}
254267
}
255268
trackedWindows.remove(window);

‎sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt

+17-6
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import io.sentry.ILogger
99
import io.sentry.ISentryExecutorService
1010
import io.sentry.MemoryCollectionData
1111
import io.sentry.PerformanceCollectionData
12+
import io.sentry.SentryExecutorService
1213
import io.sentry.SentryLevel
1314
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector
1415
import io.sentry.profilemeasurements.ProfileMeasurement
@@ -41,7 +42,7 @@ class AndroidProfilerTest {
4142
private lateinit var context: Context
4243

4344
private val className = "io.sentry.android.core.AndroidProfiler"
44-
private val ctorTypes = arrayOf(String::class.java, Int::class.java, SentryFrameMetricsCollector::class.java, SentryAndroidOptions::class.java, BuildInfoProvider::class.java)
45+
private val ctorTypes = arrayOf(String::class.java, Int::class.java, SentryFrameMetricsCollector::class.java, ISentryExecutorService::class.java, ILogger::class.java, BuildInfoProvider::class.java)
4546
private val fixture = Fixture()
4647

4748
private class Fixture {
@@ -86,7 +87,14 @@ class AndroidProfilerTest {
8687
val frameMetricsCollector: SentryFrameMetricsCollector = mock()
8788

8889
fun getSut(interval: Int = 1, buildInfoProvider: BuildInfoProvider = buildInfo): AndroidProfiler {
89-
return AndroidProfiler(options.profilingTracesDirPath!!, interval, frameMetricsCollector, options, buildInfoProvider)
90+
return AndroidProfiler(
91+
options.profilingTracesDirPath!!,
92+
interval,
93+
frameMetricsCollector,
94+
options.executorService,
95+
options.logger,
96+
buildInfoProvider
97+
)
9098
}
9199
}
92100

@@ -135,16 +143,19 @@ class AndroidProfilerTest {
135143
val ctor = className.getCtor(ctorTypes)
136144

137145
assertFailsWith<IllegalArgumentException> {
138-
ctor.newInstance(arrayOf(null, 0, mock(), mock<SentryAndroidOptions>(), mock()))
146+
ctor.newInstance(arrayOf(null, 0, mock(), mock<SentryExecutorService>(), mock<AndroidLogger>(), mock()))
139147
}
140148
assertFailsWith<IllegalArgumentException> {
141-
ctor.newInstance(arrayOf("mock", 0, null, mock<SentryAndroidOptions>(), mock()))
149+
ctor.newInstance(arrayOf("mock", 0, null, mock<SentryExecutorService>(), mock<AndroidLogger>(), mock()))
142150
}
143151
assertFailsWith<IllegalArgumentException> {
144-
ctor.newInstance(arrayOf("mock", 0, mock(), null, mock()))
152+
ctor.newInstance(arrayOf("mock", 0, mock(), null, mock<AndroidLogger>(), mock()))
145153
}
146154
assertFailsWith<IllegalArgumentException> {
147-
ctor.newInstance(arrayOf("mock", 0, mock(), mock<SentryAndroidOptions>(), null))
155+
ctor.newInstance(arrayOf("mock", 0, mock(), mock<SentryExecutorService>(), null, mock()))
156+
}
157+
assertFailsWith<IllegalArgumentException> {
158+
ctor.newInstance(arrayOf("mock", 0, mock(), mock<SentryExecutorService>(), mock<AndroidLogger>(), null))
148159
}
149160
}
150161

‎sentry/api/sentry.api

+9
Original file line numberDiff line numberDiff line change
@@ -1782,6 +1782,15 @@ public final class io/sentry/SentryExceptionFactory {
17821782
public fun getSentryExceptionsFromThread (Lio/sentry/protocol/SentryThread;Lio/sentry/protocol/Mechanism;Ljava/lang/Throwable;)Ljava/util/List;
17831783
}
17841784

1785+
public final class io/sentry/SentryExecutorService : io/sentry/ISentryExecutorService {
1786+
public fun <init> ()V
1787+
public fun close (J)V
1788+
public fun isClosed ()Z
1789+
public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future;
1790+
public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future;
1791+
public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future;
1792+
}
1793+
17851794
public final class io/sentry/SentryInstantDate : io/sentry/SentryDate {
17861795
public fun <init> ()V
17871796
public fun <init> (Ljava/time/Instant;)V

‎sentry/src/main/java/io/sentry/SentryExecutorService.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66
import java.util.concurrent.ScheduledExecutorService;
77
import java.util.concurrent.ThreadFactory;
88
import java.util.concurrent.TimeUnit;
9+
import org.jetbrains.annotations.ApiStatus;
910
import org.jetbrains.annotations.NotNull;
1011
import org.jetbrains.annotations.TestOnly;
1112

12-
final class SentryExecutorService implements ISentryExecutorService {
13+
@ApiStatus.Internal
14+
public final class SentryExecutorService implements ISentryExecutorService {
1315

1416
private final @NotNull ScheduledExecutorService executorService;
1517

@@ -18,7 +20,7 @@ final class SentryExecutorService implements ISentryExecutorService {
1820
this.executorService = executorService;
1921
}
2022

21-
SentryExecutorService() {
23+
public SentryExecutorService() {
2224
this(Executors.newSingleThreadScheduledExecutor(new SentryExecutorServiceThreadFactory()));
2325
}
2426

0 commit comments

Comments
 (0)