Skip to content

Commit e199f18

Browse files
adinaueradinauer
authored
Prevent OOM by disabling TransactionPerformanceCollector for now (#2498)
1 parent fb1c50c commit e199f18

File tree

9 files changed

+162
-111
lines changed

9 files changed

+162
-111
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
### Fixes
1717

1818
- Expand guard against CVE-2018-9492 "Privilege Escalation via Content Provider" ([#2482](https://github.com/getsentry/sentry-java/pull/2482))
19+
- Prevent OOM by disabling TransactionPerformanceCollector for now ([#2498](https://github.com/getsentry/sentry-java/pull/2498))
1920

2021
## 6.12.1
2122

‎sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ class EnvelopeTests : BaseUiTest() {
8888
val slowFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_SLOW_FRAME_RENDERS]
8989
val frozenFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_FROZEN_FRAME_RENDERS]
9090
val frameRates = profilingTraceData.measurementsMap[ProfileMeasurement.ID_SCREEN_FRAME_RATES]!!
91-
val memoryStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_MEMORY_FOOTPRINT]!!
92-
val memoryNativeStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_MEMORY_NATIVE_FOOTPRINT]!!
93-
val cpuStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_CPU_USAGE]!!
91+
// val memoryStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_MEMORY_FOOTPRINT]!!
92+
// val memoryNativeStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_MEMORY_NATIVE_FOOTPRINT]!!
93+
// val cpuStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_CPU_USAGE]!!
9494
// Slow and frozen frames can be null (in case there were none)
9595
if (slowFrames != null) {
9696
assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, slowFrames.unit)
@@ -101,12 +101,12 @@ class EnvelopeTests : BaseUiTest() {
101101
// There could be no slow/frozen frames, but we expect at least one frame rate
102102
assertEquals(ProfileMeasurement.UNIT_HZ, frameRates.unit)
103103
assertTrue(frameRates.values.isNotEmpty())
104-
assertEquals(ProfileMeasurement.UNIT_BYTES, memoryStats.unit)
105-
assertTrue(memoryStats.values.isNotEmpty())
106-
assertEquals(ProfileMeasurement.UNIT_BYTES, memoryNativeStats.unit)
107-
assertTrue(memoryNativeStats.values.isNotEmpty())
108-
assertEquals(ProfileMeasurement.UNIT_PERCENT, cpuStats.unit)
109-
assertTrue(cpuStats.values.isNotEmpty())
104+
// assertEquals(ProfileMeasurement.UNIT_BYTES, memoryStats.unit)
105+
// assertTrue(memoryStats.values.isNotEmpty())
106+
// assertEquals(ProfileMeasurement.UNIT_BYTES, memoryNativeStats.unit)
107+
// assertTrue(memoryNativeStats.values.isNotEmpty())
108+
// assertEquals(ProfileMeasurement.UNIT_PERCENT, cpuStats.unit)
109+
// assertTrue(cpuStats.values.isNotEmpty())
110110

111111
// We should find the transaction id that started the profiling in the list of transactions
112112
val transactionData = profilingTraceData.transactions

‎sentry/api/sentry.api‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,12 @@ public final class io/sentry/DateUtils {
183183
public static fun toUtilDateNotNull (Lio/sentry/SentryDate;)Ljava/util/Date;
184184
}
185185

186+
public final class io/sentry/DefaultTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector {
187+
public fun <init> (Lio/sentry/SentryOptions;)V
188+
public fun start (Lio/sentry/ITransaction;)V
189+
public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData;
190+
}
191+
186192
public final class io/sentry/DiagnosticLogger : io/sentry/ILogger {
187193
public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/ILogger;)V
188194
public fun getLogger ()Lio/sentry/ILogger;
@@ -853,6 +859,12 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction {
853859
public fun traceContext ()Lio/sentry/TraceContext;
854860
}
855861

862+
public final class io/sentry/NoOpTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector {
863+
public static fun getInstance ()Lio/sentry/NoOpTransactionPerformanceCollector;
864+
public fun start (Lio/sentry/ITransaction;)V
865+
public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData;
866+
}
867+
856868
public final class io/sentry/NoOpTransactionProfiler : io/sentry/ITransactionProfiler {
857869
public static fun getInstance ()Lio/sentry/NoOpTransactionProfiler;
858870
public fun onTransactionFinish (Lio/sentry/ITransaction;Lio/sentry/PerformanceCollectionData;)Lio/sentry/ProfilingTraceData;
@@ -2055,10 +2067,9 @@ public final class io/sentry/TransactionOptions {
20552067
public fun setWaitForChildren (Z)V
20562068
}
20572069

2058-
public final class io/sentry/TransactionPerformanceCollector {
2059-
public fun <init> (Lio/sentry/SentryOptions;)V
2060-
public fun start (Lio/sentry/ITransaction;)V
2061-
public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData;
2070+
public abstract interface class io/sentry/TransactionPerformanceCollector {
2071+
public abstract fun start (Lio/sentry/ITransaction;)V
2072+
public abstract fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData;
20622073
}
20632074

20642075
public final class io/sentry/TypeCheckHint {

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

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package io.sentry;
2+
3+
import io.sentry.util.Objects;
4+
import java.util.List;
5+
import java.util.Map;
6+
import java.util.Timer;
7+
import java.util.TimerTask;
8+
import java.util.concurrent.ConcurrentHashMap;
9+
import java.util.concurrent.atomic.AtomicBoolean;
10+
import org.jetbrains.annotations.ApiStatus;
11+
import org.jetbrains.annotations.NotNull;
12+
import org.jetbrains.annotations.Nullable;
13+
14+
@ApiStatus.Internal
15+
public final class DefaultTransactionPerformanceCollector
16+
implements TransactionPerformanceCollector {
17+
private static final long TRANSACTION_COLLECTION_INTERVAL_MILLIS = 100;
18+
private static final long TRANSACTION_COLLECTION_TIMEOUT_MILLIS = 30000;
19+
private final @NotNull Object timerLock = new Object();
20+
private volatile @Nullable Timer timer = null;
21+
private final @NotNull Map<String, PerformanceCollectionData> performanceDataMap =
22+
new ConcurrentHashMap<>();
23+
private final @NotNull List<ICollector> collectors;
24+
private final @NotNull SentryOptions options;
25+
private final @NotNull AtomicBoolean isStarted = new AtomicBoolean(false);
26+
27+
public DefaultTransactionPerformanceCollector(final @NotNull SentryOptions options) {
28+
this.options = Objects.requireNonNull(options, "The options object is required.");
29+
this.collectors = options.getCollectors();
30+
}
31+
32+
@Override
33+
@SuppressWarnings("FutureReturnValueIgnored")
34+
public void start(final @NotNull ITransaction transaction) {
35+
if (collectors.isEmpty()) {
36+
options
37+
.getLogger()
38+
.log(
39+
SentryLevel.INFO,
40+
"No collector found. Performance stats will not be captured during transactions.");
41+
return;
42+
}
43+
44+
if (!performanceDataMap.containsKey(transaction.getEventId().toString())) {
45+
performanceDataMap.put(transaction.getEventId().toString(), new PerformanceCollectionData());
46+
options
47+
.getExecutorService()
48+
.schedule(
49+
() -> {
50+
PerformanceCollectionData data = stop(transaction);
51+
if (data != null) {
52+
performanceDataMap.put(transaction.getEventId().toString(), data);
53+
}
54+
},
55+
TRANSACTION_COLLECTION_TIMEOUT_MILLIS);
56+
}
57+
if (!isStarted.getAndSet(true)) {
58+
synchronized (timerLock) {
59+
for (ICollector collector : collectors) {
60+
collector.setup();
61+
}
62+
if (timer == null) {
63+
timer = new Timer(true);
64+
}
65+
// We schedule the timer to start after a delay, so we let some time pass between setup()
66+
// and collect() calls.
67+
// This way ICollectors that collect average stats based on time intervals, like
68+
// AndroidCpuCollector, can have an actual time interval to evaluate.
69+
timer.scheduleAtFixedRate(
70+
new TimerTask() {
71+
@Override
72+
public void run() {
73+
synchronized (timerLock) {
74+
for (ICollector collector : collectors) {
75+
collector.collect(performanceDataMap.values());
76+
}
77+
// We commit data after calling all collectors.
78+
// This way we avoid issues caused by having multiple cpu or memory collectors.
79+
for (PerformanceCollectionData data : performanceDataMap.values()) {
80+
data.commitData();
81+
}
82+
}
83+
}
84+
},
85+
TRANSACTION_COLLECTION_INTERVAL_MILLIS,
86+
TRANSACTION_COLLECTION_INTERVAL_MILLIS);
87+
}
88+
}
89+
}
90+
91+
@Override
92+
public @Nullable PerformanceCollectionData stop(final @NotNull ITransaction transaction) {
93+
synchronized (timerLock) {
94+
PerformanceCollectionData data =
95+
performanceDataMap.remove(transaction.getEventId().toString());
96+
if (performanceDataMap.isEmpty() && isStarted.getAndSet(false)) {
97+
if (timer != null) {
98+
timer.cancel();
99+
timer = null;
100+
}
101+
}
102+
return data;
103+
}
104+
}
105+
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package io.sentry;
2+
3+
import org.jetbrains.annotations.NotNull;
4+
import org.jetbrains.annotations.Nullable;
5+
6+
public final class NoOpTransactionPerformanceCollector implements TransactionPerformanceCollector {
7+
8+
private static final NoOpTransactionPerformanceCollector instance =
9+
new NoOpTransactionPerformanceCollector();
10+
11+
public static NoOpTransactionPerformanceCollector getInstance() {
12+
return instance;
13+
}
14+
15+
private NoOpTransactionPerformanceCollector() {}
16+
17+
@Override
18+
public void start(@NotNull ITransaction transaction) {}
19+
20+
@Override
21+
public @Nullable PerformanceCollectionData stop(@NotNull ITransaction transaction) {
22+
return null;
23+
}
24+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ public class SentryOptions {
397397

398398
/** Performance collector that collect performance stats while transactions run. */
399399
private final @NotNull TransactionPerformanceCollector transactionPerformanceCollector =
400-
new TransactionPerformanceCollector(this);
400+
NoOpTransactionPerformanceCollector.getInstance();
401401

402402
/**
403403
* Adds an event processor

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

Lines changed: 4 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -1,102 +1,12 @@
11
package io.sentry;
22

3-
import io.sentry.util.Objects;
4-
import java.util.List;
5-
import java.util.Map;
6-
import java.util.Timer;
7-
import java.util.TimerTask;
8-
import java.util.concurrent.ConcurrentHashMap;
9-
import java.util.concurrent.atomic.AtomicBoolean;
10-
import org.jetbrains.annotations.ApiStatus;
113
import org.jetbrains.annotations.NotNull;
124
import org.jetbrains.annotations.Nullable;
135

14-
@ApiStatus.Internal
15-
public final class TransactionPerformanceCollector {
16-
private static final long TRANSACTION_COLLECTION_INTERVAL_MILLIS = 100;
17-
private static final long TRANSACTION_COLLECTION_TIMEOUT_MILLIS = 30000;
18-
private final @NotNull Object timerLock = new Object();
19-
private volatile @Nullable Timer timer = null;
20-
private final @NotNull Map<String, PerformanceCollectionData> performanceDataMap =
21-
new ConcurrentHashMap<>();
22-
private final @NotNull List<ICollector> collectors;
23-
private final @NotNull SentryOptions options;
24-
private final @NotNull AtomicBoolean isStarted = new AtomicBoolean(false);
6+
public interface TransactionPerformanceCollector {
257

26-
public TransactionPerformanceCollector(final @NotNull SentryOptions options) {
27-
this.options = Objects.requireNonNull(options, "The options object is required.");
28-
this.collectors = options.getCollectors();
29-
}
8+
void start(@NotNull ITransaction transaction);
309

31-
@SuppressWarnings("FutureReturnValueIgnored")
32-
public void start(final @NotNull ITransaction transaction) {
33-
if (collectors.isEmpty()) {
34-
options
35-
.getLogger()
36-
.log(
37-
SentryLevel.INFO,
38-
"No collector found. Performance stats will not be captured during transactions.");
39-
return;
40-
}
41-
42-
if (!performanceDataMap.containsKey(transaction.getEventId().toString())) {
43-
performanceDataMap.put(transaction.getEventId().toString(), new PerformanceCollectionData());
44-
options
45-
.getExecutorService()
46-
.schedule(
47-
() -> {
48-
PerformanceCollectionData data = stop(transaction);
49-
if (data != null) {
50-
performanceDataMap.put(transaction.getEventId().toString(), data);
51-
}
52-
},
53-
TRANSACTION_COLLECTION_TIMEOUT_MILLIS);
54-
}
55-
if (!isStarted.getAndSet(true)) {
56-
synchronized (timerLock) {
57-
for (ICollector collector : collectors) {
58-
collector.setup();
59-
}
60-
if (timer == null) {
61-
timer = new Timer(true);
62-
}
63-
// We schedule the timer to start after a delay, so we let some time pass between setup()
64-
// and collect() calls.
65-
// This way ICollectors that collect average stats based on time intervals, like
66-
// AndroidCpuCollector, can have an actual time interval to evaluate.
67-
timer.scheduleAtFixedRate(
68-
new TimerTask() {
69-
@Override
70-
public void run() {
71-
synchronized (timerLock) {
72-
for (ICollector collector : collectors) {
73-
collector.collect(performanceDataMap.values());
74-
}
75-
// We commit data after calling all collectors.
76-
// This way we avoid issues caused by having multiple cpu or memory collectors.
77-
for (PerformanceCollectionData data : performanceDataMap.values()) {
78-
data.commitData();
79-
}
80-
}
81-
}
82-
},
83-
TRANSACTION_COLLECTION_INTERVAL_MILLIS,
84-
TRANSACTION_COLLECTION_INTERVAL_MILLIS);
85-
}
86-
}
87-
}
88-
89-
public @Nullable PerformanceCollectionData stop(final @NotNull ITransaction transaction) {
90-
synchronized (timerLock) {
91-
PerformanceCollectionData data =
92-
performanceDataMap.remove(transaction.getEventId().toString());
93-
if (performanceDataMap.isEmpty() && isStarted.getAndSet(false)) {
94-
if (timer != null) {
95-
timer.cancel();
96-
timer = null;
97-
}
98-
}
99-
return data;
100-
}
101-
}
10+
@Nullable
11+
PerformanceCollectionData stop(@NotNull ITransaction transaction);
10212
}

‎sentry/src/test/java/io/sentry/SentryTracerTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class SentryTracerTest {
3434
options.environment = "environment"
3535
options.release = "[email protected]"
3636
hub = spy(Hub(options))
37-
transactionPerformanceCollector = spy(TransactionPerformanceCollector(options))
37+
transactionPerformanceCollector = spy(DefaultTransactionPerformanceCollector(options))
3838
hub.bindClient(mock())
3939
}
4040

‎sentry/src/test/java/io/sentry/TransactionPerformanceCollectorTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import kotlin.test.assertTrue
2424

2525
class TransactionPerformanceCollectorTest {
2626

27-
private val className = "io.sentry.TransactionPerformanceCollector"
27+
private val className = "io.sentry.DefaultTransactionPerformanceCollector"
2828
private val ctorTypes: Array<Class<*>> = arrayOf(SentryOptions::class.java)
2929
private val fixture = Fixture()
3030

@@ -70,7 +70,7 @@ class TransactionPerformanceCollectorTest {
7070
}
7171
transaction1 = SentryTracer(TransactionContext("", ""), hub)
7272
transaction2 = SentryTracer(TransactionContext("", ""), hub)
73-
val collector = TransactionPerformanceCollector(options)
73+
val collector = DefaultTransactionPerformanceCollector(options)
7474
val timer: Timer? = collector.getProperty("timer") ?: Timer(true)
7575
mockTimer = spy(timer)
7676
collector.injectForField("timer", mockTimer)

0 commit comments

Comments
 (0)