Skip to content

Commit f0a7fca

Browse files
stefanosianostefanosiano
authored
Merge 1287e69 into adee765 
2 parents adee765 + 1287e69 commit f0a7fca

10 files changed

Lines changed: 127 additions & 142 deletions

File tree

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

Lines changed: 33 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@
1111
import android.os.Debug;
1212
import android.os.Process;
1313
import android.os.SystemClock;
14+
import io.sentry.HubAdapter;
1415
import io.sentry.ITransaction;
1516
import io.sentry.ITransactionProfiler;
1617
import io.sentry.ProfilingTraceData;
1718
import io.sentry.ProfilingTransactionData;
1819
import io.sentry.SentryLevel;
1920
import io.sentry.android.core.internal.util.CpuInfoUtils;
20-
import io.sentry.util.CollectionUtils;
2121
import io.sentry.util.Objects;
2222
import java.io.File;
2323
import java.util.ArrayList;
@@ -48,7 +48,6 @@ final class AndroidTransactionProfiler implements ITransactionProfiler {
4848
private @Nullable File traceFile = null;
4949
private @Nullable File traceFilesDir = null;
5050
private @Nullable Future<?> scheduledFinish = null;
51-
private volatile @Nullable ProfilingTraceData timedOutProfilingData = null;
5251
private final @NotNull Context context;
5352
private final @NotNull SentryAndroidOptions options;
5453
private final @NotNull BuildInfoProvider buildInfoProvider;
@@ -137,9 +136,7 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
137136
scheduledFinish =
138137
options
139138
.getExecutorService()
140-
.schedule(
141-
() -> timedOutProfilingData = onTransactionFinish(transaction, true),
142-
PROFILING_TIMEOUT_MILLIS);
139+
.schedule(() -> onTransactionFinish(transaction, true), PROFILING_TIMEOUT_MILLIS);
143140

144141
transactionStartNanos = SystemClock.elapsedRealtimeNanos();
145142
profileStartCpuMillis = Process.getElapsedCpuTime();
@@ -166,53 +163,28 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
166163
}
167164

168165
@Override
169-
public synchronized @Nullable ProfilingTraceData onTransactionFinish(
170-
@NotNull ITransaction transaction) {
171-
return onTransactionFinish(transaction, false);
166+
public synchronized void onTransactionFinish(@NotNull ITransaction transaction) {
167+
onTransactionFinish(transaction, false);
172168
}
173169

174170
@SuppressLint("NewApi")
175-
private synchronized @Nullable ProfilingTraceData onTransactionFinish(
171+
private synchronized void onTransactionFinish(
176172
@NotNull ITransaction transaction, boolean isTimeout) {
177173

178174
// onTransactionStart() is only available since Lollipop
179175
// and SystemClock.elapsedRealtimeNanos() since Jelly Bean
180-
if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) return null;
181-
182-
final ProfilingTraceData profilingData = timedOutProfilingData;
176+
if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) return;
183177

184-
// Transaction finished, but it's not in the current profile
178+
// Transaction finished, but it's not in the current profile. We can skip it
185179
if (!transactionMap.containsKey(transaction.getEventId().toString())) {
186-
// We check if we cached a profiling data due to a timeout with this profile in it
187-
// If so, we return it back, otherwise we would simply lose it
188-
if (profilingData != null) {
189-
// Don't use method reference. This can cause issues on Android
190-
List<String> ids =
191-
CollectionUtils.map(profilingData.getTransactions(), (data) -> data.getId());
192-
if (ids.contains(transaction.getEventId().toString())) {
193-
timedOutProfilingData = null;
194-
return profilingData;
195-
} else {
196-
// Another transaction is finishing before the timed out one
197-
options
198-
.getLogger()
199-
.log(
200-
SentryLevel.INFO,
201-
"A timed out profiling data exists, but the finishing transaction %s (%s) is not part of it",
202-
transaction.getName(),
203-
transaction.getSpanContext().getTraceId().toString());
204-
return null;
205-
}
206-
}
207-
// A transaction is finishing, but it's not profiled. We can skip it
208180
options
209181
.getLogger()
210182
.log(
211183
SentryLevel.INFO,
212184
"Transaction %s (%s) finished, but was not currently being profiled. Skipping",
213185
transaction.getName(),
214186
transaction.getSpanContext().getTraceId().toString());
215-
return null;
187+
return;
216188
}
217189

218190
if (transactionsCounter > 0) {
@@ -239,7 +211,7 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
239211
Process.getElapsedCpuTime(),
240212
profileStartCpuMillis);
241213
}
242-
return null;
214+
return;
243215
}
244216

245217
Debug.stopMethodTracing();
@@ -259,7 +231,7 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
259231

260232
if (traceFile == null) {
261233
options.getLogger().log(SentryLevel.ERROR, "Trace file does not exists");
262-
return null;
234+
return;
263235
}
264236

265237
String versionName = "";
@@ -287,26 +259,29 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
287259

288260
// cpu max frequencies are read with a lambda because reading files is involved, so it will be
289261
// done in the background when the trace file is read
290-
return new ProfilingTraceData(
291-
traceFile,
292-
transactionList,
293-
transaction,
294-
Long.toString(transactionDurationNanos),
295-
buildInfoProvider.getSdkInfoVersion(),
296-
abis != null && abis.length > 0 ? abis[0] : "",
297-
() -> CpuInfoUtils.getInstance().readMaxFrequencies(),
298-
buildInfoProvider.getManufacturer(),
299-
buildInfoProvider.getModel(),
300-
buildInfoProvider.getVersionRelease(),
301-
buildInfoProvider.isEmulator(),
302-
totalMem,
303-
options.getProguardUuid(),
304-
versionName,
305-
versionCode,
306-
options.getEnvironment(),
307-
isTimeout
308-
? ProfilingTraceData.TRUNCATION_REASON_TIMEOUT
309-
: ProfilingTraceData.TRUNCATION_REASON_NORMAL);
262+
ProfilingTraceData profilingTraceData =
263+
new ProfilingTraceData(
264+
traceFile,
265+
transactionList,
266+
transaction,
267+
Long.toString(transactionDurationNanos),
268+
buildInfoProvider.getSdkInfoVersion(),
269+
abis != null && abis.length > 0 ? abis[0] : "",
270+
() -> CpuInfoUtils.getInstance().readMaxFrequencies(),
271+
buildInfoProvider.getManufacturer(),
272+
buildInfoProvider.getModel(),
273+
buildInfoProvider.getVersionRelease(),
274+
buildInfoProvider.isEmulator(),
275+
totalMem,
276+
options.getProguardUuid(),
277+
versionName,
278+
versionCode,
279+
options.getEnvironment(),
280+
isTimeout
281+
? ProfilingTraceData.TRUNCATION_REASON_TIMEOUT
282+
: ProfilingTraceData.TRUNCATION_REASON_NORMAL);
283+
284+
HubAdapter.getInstance().captureProfile(profilingTraceData);
310285
}
311286

312287
/**

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

Lines changed: 55 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import androidx.test.core.app.ApplicationProvider
66
import androidx.test.ext.junit.runners.AndroidJUnit4
77
import com.nhaarman.mockitokotlin2.any
88
import com.nhaarman.mockitokotlin2.argumentCaptor
9+
import com.nhaarman.mockitokotlin2.check
910
import com.nhaarman.mockitokotlin2.mock
1011
import com.nhaarman.mockitokotlin2.never
1112
import com.nhaarman.mockitokotlin2.spy
@@ -16,6 +17,7 @@ import io.sentry.IHub
1617
import io.sentry.ILogger
1718
import io.sentry.ISentryExecutorService
1819
import io.sentry.ProfilingTraceData
20+
import io.sentry.Sentry
1921
import io.sentry.SentryLevel
2022
import io.sentry.SentryTracer
2123
import io.sentry.TransactionContext
@@ -28,7 +30,6 @@ import kotlin.test.Test
2830
import kotlin.test.assertEquals
2931
import kotlin.test.assertFailsWith
3032
import kotlin.test.assertNotNull
31-
import kotlin.test.assertNull
3233
import kotlin.test.assertTrue
3334

3435
@RunWith(AndroidJUnit4::class)
@@ -63,6 +64,7 @@ class AndroidTransactionProfilerTest {
6364
transaction1 = SentryTracer(TransactionContext("", ""), hub)
6465
transaction2 = SentryTracer(TransactionContext("", ""), hub)
6566
transaction3 = SentryTracer(TransactionContext("", ""), hub)
67+
Sentry.setCurrentHub(hub)
6668
return AndroidTransactionProfiler(context, options, buildInfoProvider)
6769
}
6870
}
@@ -100,8 +102,13 @@ class AndroidTransactionProfilerTest {
100102
fun `profiler profiles current transaction`() {
101103
val profiler = fixture.getSut(context)
102104
profiler.onTransactionStart(fixture.transaction1)
103-
val traceData = profiler.onTransactionFinish(fixture.transaction1)
104-
assertEquals(fixture.transaction1.eventId.toString(), traceData!!.transactionId)
105+
profiler.onTransactionFinish(fixture.transaction1)
106+
107+
verify(fixture.hub).captureProfile(
108+
check {
109+
assertEquals(it.transactionId, fixture.transaction1.eventId.toString())
110+
}
111+
)
105112
}
106113

107114
@Test
@@ -111,8 +118,8 @@ class AndroidTransactionProfilerTest {
111118
}
112119
val profiler = fixture.getSut(context, buildInfo)
113120
profiler.onTransactionStart(fixture.transaction1)
114-
val traceData = profiler.onTransactionFinish(fixture.transaction1)
115-
assertNull(traceData)
121+
profiler.onTransactionFinish(fixture.transaction1)
122+
verify(fixture.hub, never()).captureProfile(any())
116123
}
117124

118125
@Test
@@ -122,8 +129,8 @@ class AndroidTransactionProfilerTest {
122129
}
123130
val profiler = fixture.getSut(context)
124131
profiler.onTransactionStart(fixture.transaction1)
125-
val traceData = profiler.onTransactionFinish(fixture.transaction1)
126-
assertNull(traceData)
132+
profiler.onTransactionFinish(fixture.transaction1)
133+
verify(fixture.hub, never()).captureProfile(any())
127134
}
128135

129136
@Test
@@ -195,8 +202,8 @@ class AndroidTransactionProfilerTest {
195202
}
196203
val profiler = fixture.getSut(context)
197204
profiler.onTransactionStart(fixture.transaction1)
198-
val traceData = profiler.onTransactionFinish(fixture.transaction1)
199-
assertNull(traceData)
205+
profiler.onTransactionFinish(fixture.transaction1)
206+
verify(fixture.hub, never()).captureProfile(any())
200207
}
201208

202209
@Test
@@ -206,8 +213,8 @@ class AndroidTransactionProfilerTest {
206213
}
207214
val profiler = fixture.getSut(context)
208215
profiler.onTransactionStart(fixture.transaction1)
209-
val traceData = profiler.onTransactionFinish(fixture.transaction1)
210-
assertNull(traceData)
216+
profiler.onTransactionFinish(fixture.transaction1)
217+
verify(fixture.hub, never()).captureProfile(any())
211218
}
212219

213220
@Test
@@ -217,8 +224,8 @@ class AndroidTransactionProfilerTest {
217224
}
218225
val profiler = fixture.getSut(context)
219226
profiler.onTransactionStart(fixture.transaction1)
220-
val traceData = profiler.onTransactionFinish(fixture.transaction1)
221-
assertNull(traceData)
227+
profiler.onTransactionFinish(fixture.transaction1)
228+
verify(fixture.hub, never()).captureProfile(any())
222229
}
223230

224231
@Test
@@ -235,34 +242,8 @@ class AndroidTransactionProfilerTest {
235242
@Test
236243
fun `onTransactionFinish works only if previously started`() {
237244
val profiler = fixture.getSut(context)
238-
val traceData = profiler.onTransactionFinish(fixture.transaction1)
239-
assertNull(traceData)
240-
}
241-
242-
@Test
243-
fun `onTransactionFinish returns timedOutData to the timed out transaction once, even after other transactions`() {
244-
val profiler = fixture.getSut(context)
245-
246-
val executorService = mock<ISentryExecutorService>()
247-
val captor = argumentCaptor<Runnable>()
248-
whenever(executorService.schedule(captor.capture(), any())).thenReturn(null)
249-
whenever(fixture.options.executorService).thenReturn(executorService)
250-
// Start and finish first transaction profiling
251-
profiler.onTransactionStart(fixture.transaction1)
252-
253-
// Set timed out data by calling the timeout scheduled job
254-
captor.firstValue.run()
255-
256-
// Start and finish second transaction. Since profiler returned data, it means no other profiling is running
257-
profiler.onTransactionStart(fixture.transaction2)
258-
assertEquals(fixture.transaction2.eventId.toString(), profiler.onTransactionFinish(fixture.transaction2)!!.transactionId)
259-
260-
// First transaction finishes: timed out data is returned
261-
val traceData = profiler.onTransactionFinish(fixture.transaction1)
262-
assertEquals(traceData!!.transactionId, fixture.transaction1.eventId.toString())
263-
264-
// If first transaction is finished again, nothing is returned
265-
assertNull(profiler.onTransactionFinish(fixture.transaction1))
245+
profiler.onTransactionFinish(fixture.transaction1)
246+
verify(fixture.hub, never()).captureProfile(any())
266247
}
267248

268249
@Test
@@ -281,9 +262,13 @@ class AndroidTransactionProfilerTest {
281262
captor.firstValue.run()
282263

283264
// First transaction finishes: timed out data is returned
284-
val traceData = profiler.onTransactionFinish(fixture.transaction1)
285-
assertEquals(traceData!!.transactionId, fixture.transaction1.eventId.toString())
286-
assertEquals(ProfilingTraceData.TRUNCATION_REASON_TIMEOUT, traceData.truncationReason)
265+
profiler.onTransactionFinish(fixture.transaction1)
266+
verify(fixture.hub).captureProfile(
267+
check {
268+
assertEquals(it.transactionId, fixture.transaction1.eventId.toString())
269+
assertEquals(ProfilingTraceData.TRUNCATION_REASON_TIMEOUT, it.truncationReason)
270+
}
271+
)
287272
}
288273

289274
@Test
@@ -292,11 +277,15 @@ class AndroidTransactionProfilerTest {
292277
profiler.onTransactionStart(fixture.transaction1)
293278
profiler.onTransactionStart(fixture.transaction2)
294279

295-
var traceData = profiler.onTransactionFinish(fixture.transaction2)
296-
assertNull(traceData)
280+
profiler.onTransactionFinish(fixture.transaction2)
281+
verify(fixture.hub, never()).captureProfile(any())
297282

298-
traceData = profiler.onTransactionFinish(fixture.transaction1)
299-
assertEquals(fixture.transaction1.eventId.toString(), traceData!!.transactionId)
283+
profiler.onTransactionFinish(fixture.transaction1)
284+
verify(fixture.hub).captureProfile(
285+
check {
286+
assertEquals(it.transactionId, fixture.transaction1.eventId.toString())
287+
}
288+
)
300289
}
301290

302291
@Test
@@ -305,20 +294,26 @@ class AndroidTransactionProfilerTest {
305294
profiler.onTransactionStart(fixture.transaction1)
306295
profiler.onTransactionStart(fixture.transaction2)
307296

308-
var traceData = profiler.onTransactionFinish(fixture.transaction1)
309-
assertNull(traceData)
297+
profiler.onTransactionFinish(fixture.transaction1)
298+
verify(fixture.hub, never()).captureProfile(any())
310299

311300
profiler.onTransactionStart(fixture.transaction3)
312-
traceData = profiler.onTransactionFinish(fixture.transaction3)
313-
assertNull(traceData)
314-
traceData = profiler.onTransactionFinish(fixture.transaction2)
315-
assertEquals(fixture.transaction2.eventId.toString(), traceData!!.transactionId)
316-
val expectedTransactions = listOf(
317-
fixture.transaction1.eventId.toString(),
318-
fixture.transaction3.eventId.toString(),
319-
fixture.transaction2.eventId.toString()
301+
profiler.onTransactionFinish(fixture.transaction3)
302+
verify(fixture.hub, never()).captureProfile(any())
303+
304+
profiler.onTransactionFinish(fixture.transaction2)
305+
verify(fixture.hub).captureProfile(
306+
check {
307+
val expectedTransactions = listOf(
308+
fixture.transaction1.eventId.toString(),
309+
fixture.transaction3.eventId.toString(),
310+
fixture.transaction2.eventId.toString()
311+
)
312+
assertEquals(it.transactionId, fixture.transaction2.eventId.toString())
313+
314+
assertTrue(it.transactions.map { it.id }.containsAll(expectedTransactions))
315+
assertTrue(expectedTransactions.containsAll(it.transactions.map { it.id }))
316+
}
320317
)
321-
assertTrue(traceData.transactions.map { it.id }.containsAll(expectedTransactions))
322-
assertTrue(expectedTransactions.containsAll(traceData.transactions.map { it.id }))
323318
}
324319
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package io.sentry.android.core
33
import io.sentry.ITransaction
44
import io.sentry.ITransactionProfiler
55
import io.sentry.NoOpTransactionProfiler
6-
import io.sentry.ProfilingTraceData
76
import io.sentry.protocol.DebugImage
87
import kotlin.test.Test
98
import kotlin.test.assertEquals
@@ -110,6 +109,6 @@ class SentryAndroidOptionsTest {
110109

111110
private class CustomTransactionProfiler : ITransactionProfiler {
112111
override fun onTransactionStart(transaction: ITransaction) {}
113-
override fun onTransactionFinish(transaction: ITransaction): ProfilingTraceData? = null
112+
override fun onTransactionFinish(transaction: ITransaction) {}
114113
}
115114
}

0 commit comments

Comments
 (0)