Skip to content

Commit

Permalink
Merge branch 'main' into feat/async-ndk-loadlib
Browse files Browse the repository at this point in the history
  • Loading branch information
markushi authored Oct 14, 2024
2 parents 063612a + a9c767e commit 5c35128
Show file tree
Hide file tree
Showing 16 changed files with 593 additions and 50 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
### Fixes

- Deprecate `enableTracing` option ([#3777](https://github.com/getsentry/sentry-java/pull/3777))
- Vendor `java.util.Random` and replace `java.security.SecureRandom` usages ([#3783](https://github.com/getsentry/sentry-java/pull/3783))
- Fix potential ANRs due to NDK scope sync ([#3754](https://github.com/getsentry/sentry-java/pull/3754))
- Fix potential ANRs due to NDK System.loadLibrary calls ([#3670](https://github.com/getsentry/sentry-java/pull/3670))

## 7.15.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
import io.sentry.protocol.SentryTransaction;
import io.sentry.protocol.User;
import io.sentry.util.HintUtils;
import io.sentry.util.Random;
import java.io.File;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -83,7 +83,7 @@ public final class AnrV2EventProcessor implements BackfillingEventProcessor {

private final @NotNull SentryExceptionFactory sentryExceptionFactory;

private final @Nullable SecureRandom random;
private final @Nullable Random random;

public AnrV2EventProcessor(
final @NotNull Context context,
Expand All @@ -96,7 +96,7 @@ public AnrV2EventProcessor(
final @NotNull Context context,
final @NotNull SentryAndroidOptions options,
final @NotNull BuildInfoProvider buildInfoProvider,
final @Nullable SecureRandom random) {
final @Nullable Random random) {
this.context = ContextUtils.getApplicationContext(context);
this.options = options;
this.buildInfoProvider = buildInfoProvider;
Expand Down Expand Up @@ -180,7 +180,7 @@ private boolean sampleReplay(final @NotNull SentryEvent event) {

try {
// we have to sample here with the old sample rate, because it may change between app launches
final @NotNull SecureRandom random = this.random != null ? this.random : new SecureRandom();
final @NotNull Random random = this.random != null ? this.random : new Random();
final double replayErrorSampleRateDouble = Double.parseDouble(replayErrorSampleRate);
if (replayErrorSampleRateDouble < random.nextDouble()) {
options
Expand Down
2 changes: 1 addition & 1 deletion sentry-android-ndk/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,6 @@ dependencies {

testImplementation(kotlin(Config.kotlinStdLib, KotlinCompilerVersion.VERSION))
testImplementation(Config.TestLibs.kotlinTestJunit)

testImplementation(Config.TestLibs.mockitoKotlin)
testImplementation(projects.sentryTestSupport)
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,18 @@ public NdkScopeObserver(final @NotNull SentryOptions options) {
@Override
public void setUser(final @Nullable User user) {
try {
if (user == null) {
// remove user if its null
nativeScope.removeUser();
} else {
nativeScope.setUser(user.getId(), user.getEmail(), user.getIpAddress(), user.getUsername());
}
options
.getExecutorService()
.submit(
() -> {
if (user == null) {
// remove user if its null
nativeScope.removeUser();
} else {
nativeScope.setUser(
user.getId(), user.getEmail(), user.getIpAddress(), user.getUsername());
}
});
} catch (Throwable e) {
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setUser has an error.");
}
Expand All @@ -45,24 +51,36 @@ public void setUser(final @Nullable User user) {
@Override
public void addBreadcrumb(final @NotNull Breadcrumb crumb) {
try {
String level = null;
if (crumb.getLevel() != null) {
level = crumb.getLevel().name().toLowerCase(Locale.ROOT);
}
final String timestamp = DateUtils.getTimestamp(crumb.getTimestamp());
options
.getExecutorService()
.submit(
() -> {
String level = null;
if (crumb.getLevel() != null) {
level = crumb.getLevel().name().toLowerCase(Locale.ROOT);
}
final String timestamp = DateUtils.getTimestamp(crumb.getTimestamp());

String data = null;
try {
final Map<String, Object> dataRef = crumb.getData();
if (!dataRef.isEmpty()) {
data = options.getSerializer().serialize(dataRef);
}
} catch (Throwable e) {
options.getLogger().log(SentryLevel.ERROR, e, "Breadcrumb data is not serializable.");
}
String data = null;
try {
final Map<String, Object> dataRef = crumb.getData();
if (!dataRef.isEmpty()) {
data = options.getSerializer().serialize(dataRef);
}
} catch (Throwable e) {
options
.getLogger()
.log(SentryLevel.ERROR, e, "Breadcrumb data is not serializable.");
}

nativeScope.addBreadcrumb(
level, crumb.getMessage(), crumb.getCategory(), crumb.getType(), timestamp, data);
nativeScope.addBreadcrumb(
level,
crumb.getMessage(),
crumb.getCategory(),
crumb.getType(),
timestamp,
data);
});
} catch (Throwable e) {
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync addBreadcrumb has an error.");
}
Expand All @@ -71,7 +89,7 @@ public void addBreadcrumb(final @NotNull Breadcrumb crumb) {
@Override
public void setTag(final @NotNull String key, final @NotNull String value) {
try {
nativeScope.setTag(key, value);
options.getExecutorService().submit(() -> nativeScope.setTag(key, value));
} catch (Throwable e) {
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setTag(%s) has an error.", key);
}
Expand All @@ -80,7 +98,7 @@ public void setTag(final @NotNull String key, final @NotNull String value) {
@Override
public void removeTag(final @NotNull String key) {
try {
nativeScope.removeTag(key);
options.getExecutorService().submit(() -> nativeScope.removeTag(key));
} catch (Throwable e) {
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync removeTag(%s) has an error.", key);
}
Expand All @@ -89,7 +107,7 @@ public void removeTag(final @NotNull String key) {
@Override
public void setExtra(final @NotNull String key, final @NotNull String value) {
try {
nativeScope.setExtra(key, value);
options.getExecutorService().submit(() -> nativeScope.setExtra(key, value));
} catch (Throwable e) {
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setExtra(%s) has an error.", key);
}
Expand All @@ -98,7 +116,7 @@ public void setExtra(final @NotNull String key, final @NotNull String value) {
@Override
public void removeExtra(final @NotNull String key) {
try {
nativeScope.removeExtra(key);
options.getExecutorService().submit(() -> nativeScope.removeExtra(key));
} catch (Throwable e) {
options
.getLogger()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@ import io.sentry.JsonSerializer
import io.sentry.SentryLevel
import io.sentry.SentryOptions
import io.sentry.protocol.User
import io.sentry.test.DeferredExecutorService
import io.sentry.test.ImmediateExecutorService
import org.mockito.kotlin.any
import org.mockito.kotlin.anyOrNull
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.verify
import kotlin.test.Test

Expand All @@ -17,6 +22,7 @@ class NdkScopeObserverTest {
val nativeScope = mock<INativeScope>()
val options = SentryOptions().apply {
setSerializer(JsonSerializer(mock()))
executorService = ImmediateExecutorService()
}

fun getSut(): NdkScopeObserver {
Expand Down Expand Up @@ -111,4 +117,36 @@ class NdkScopeObserverTest {
eq(data)
)
}

@Test
fun `scope sync utilizes executor service`() {
val executorService = DeferredExecutorService()
fixture.options.executorService = executorService
val sut = fixture.getSut()

sut.setTag("a", "b")
sut.removeTag("a")
sut.setExtra("a", "b")
sut.removeExtra("a")
sut.setUser(User())
sut.addBreadcrumb(Breadcrumb())

// as long as the executor service is not run, the scope sync is not called
verify(fixture.nativeScope, never()).setTag(any(), any())
verify(fixture.nativeScope, never()).removeTag(any())
verify(fixture.nativeScope, never()).setExtra(any(), any())
verify(fixture.nativeScope, never()).removeExtra(any())
verify(fixture.nativeScope, never()).setUser(any(), any(), any(), any())
verify(fixture.nativeScope, never()).addBreadcrumb(any(), any(), any(), any(), any(), any())

// when the executor service is run, the scope sync is called
executorService.runAll()

verify(fixture.nativeScope).setTag(any(), any())
verify(fixture.nativeScope).removeTag(any())
verify(fixture.nativeScope).setExtra(any(), any())
verify(fixture.nativeScope).removeExtra(any())
verify(fixture.nativeScope).setUser(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull())
verify(fixture.nativeScope).addBreadcrumb(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ import io.sentry.transport.ICurrentDateProvider
import io.sentry.util.FileUtils
import io.sentry.util.HintUtils
import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
import io.sentry.util.Random
import java.io.Closeable
import java.io.File
import java.security.SecureRandom
import java.util.LinkedList
import java.util.concurrent.atomic.AtomicBoolean
import kotlin.LazyThreadSafetyMode.NONE
Expand Down Expand Up @@ -78,7 +78,7 @@ public class ReplayIntegration(
private var hub: IHub? = null
private var recorder: Recorder? = null
private var gestureRecorder: GestureRecorder? = null
private val random by lazy { SecureRandom() }
private val random by lazy { Random() }
private val rootViewsSpy by lazy(NONE) { RootViewsSpy.install() }

// TODO: probably not everything has to be thread-safe here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ import io.sentry.android.replay.util.submitSafely
import io.sentry.protocol.SentryId
import io.sentry.transport.ICurrentDateProvider
import io.sentry.util.FileUtils
import io.sentry.util.Random
import java.io.File
import java.security.SecureRandom
import java.util.Date
import java.util.concurrent.ScheduledExecutorService

internal class BufferCaptureStrategy(
private val options: SentryOptions,
private val hub: IHub?,
private val dateProvider: ICurrentDateProvider,
private val random: SecureRandom,
private val random: Random,
executor: ScheduledExecutorService? = null,
replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
) : BaseCaptureStrategy(options, hub, dateProvider, executor = executor, replayCacheProvider = replayCacheProvider) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package io.sentry.android.replay.util

import java.security.SecureRandom
import io.sentry.util.Random

internal fun SecureRandom.sample(rate: Double?): Boolean {
internal fun Random.sample(rate: Double?): Boolean {
if (rate != null) {
return !(rate < this.nextDouble()) // bad luck
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import io.sentry.android.replay.capture.BufferCaptureStrategyTest.Fixture.Compan
import io.sentry.protocol.SentryId
import io.sentry.transport.CurrentDateProvider
import io.sentry.transport.ICurrentDateProvider
import io.sentry.util.Random
import org.awaitility.kotlin.await
import org.junit.Rule
import org.junit.rules.TemporaryFolder
Expand All @@ -35,7 +36,6 @@ import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import java.io.File
import java.security.SecureRandom
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
Expand Down Expand Up @@ -97,7 +97,7 @@ class BufferCaptureStrategyTest {
options,
hub,
dateProvider,
SecureRandom(),
Random(),
mock {
doAnswer { invocation ->
(invocation.arguments[0] as Runnable).run()
Expand Down
13 changes: 13 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -5808,6 +5808,19 @@ public final class io/sentry/util/PropagationTargetsUtils {
public static fun contain (Ljava/util/List;Ljava/net/URI;)Z
}

public final class io/sentry/util/Random : java/io/Serializable {
public fun <init> ()V
public fun <init> (J)V
public fun nextBoolean ()Z
public fun nextBytes ([B)V
public fun nextDouble ()D
public fun nextFloat ()F
public fun nextInt ()I
public fun nextInt (I)I
public fun nextLong ()J
public fun setSeed (J)V
}

public final class io/sentry/util/SampleRateUtils {
public fun <init> ()V
public static fun isValidProfilesSampleRate (Ljava/lang/Double;)Z
Expand Down
6 changes: 3 additions & 3 deletions sentry/src/main/java/io/sentry/SentryClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
import io.sentry.util.CheckInUtils;
import io.sentry.util.HintUtils;
import io.sentry.util.Objects;
import io.sentry.util.Random;
import io.sentry.util.TracingUtils;
import java.io.Closeable;
import java.io.IOException;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -40,7 +40,7 @@ public final class SentryClient implements ISentryClient, IMetricsClient {

private final @NotNull SentryOptions options;
private final @NotNull ITransport transport;
private final @Nullable SecureRandom random;
private final @Nullable Random random;
private final @NotNull SortBreadcrumbsByDate sortBreadcrumbsByDate = new SortBreadcrumbsByDate();
private final @NotNull IMetricsAggregator metricsAggregator;

Expand All @@ -67,7 +67,7 @@ public boolean isEnabled() {
? new MetricsAggregator(options, this)
: NoopMetricsAggregator.getInstance();

this.random = options.getSampleRate() == null ? null : new SecureRandom();
this.random = options.getSampleRate() == null ? null : new Random();
}

private boolean shouldApplyScopeData(
Expand Down
8 changes: 4 additions & 4 deletions sentry/src/main/java/io/sentry/TracesSampler.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.sentry;

import io.sentry.util.Objects;
import java.security.SecureRandom;
import io.sentry.util.Random;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.TestOnly;
Expand All @@ -10,14 +10,14 @@ final class TracesSampler {
private static final @NotNull Double DEFAULT_TRACES_SAMPLE_RATE = 1.0;

private final @NotNull SentryOptions options;
private final @NotNull SecureRandom random;
private final @NotNull Random random;

public TracesSampler(final @NotNull SentryOptions options) {
this(Objects.requireNonNull(options, "options are required"), new SecureRandom());
this(Objects.requireNonNull(options, "options are required"), new Random());
}

@TestOnly
TracesSampler(final @NotNull SentryOptions options, final @NotNull SecureRandom random) {
TracesSampler(final @NotNull SentryOptions options, final @NotNull Random random) {
this.options = options;
this.random = random;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ public void setReplayId(@NotNull SentryId replayId) {

@SuppressWarnings("FutureReturnValueIgnored")
private void serializeToDisk(final @NotNull Runnable task) {
if (Thread.currentThread().getName().contains("SentryExecutor")) {
// we're already on the sentry executor thread, so we can just execute it directly
task.run();
return;
}

try {
options
.getExecutorService()
Expand Down
Loading

0 comments on commit 5c35128

Please sign in to comment.