Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/profiling fixes #1997

Merged
merged 9 commits into from
Apr 26, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

* Fix: Profiling rate decreased from 300hz to 100hz; fixed profiling traces folder creation on manual sdk init (#1997)
* Ref: Make options.printUncaughtStackTrace primitive type (#1995)

## 6.0.0-alpha.5
Expand Down
2 changes: 0 additions & 2 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun enableAllAutoBreadcrumbs (Z)V
public fun getAnrTimeoutIntervalMillis ()J
public fun getDebugImagesLoader ()Lio/sentry/android/core/IDebugImagesLoader;
public fun getProfilingTracesDirPath ()Ljava/lang/String;
public fun getProfilingTracesIntervalMillis ()I
public fun isAnrEnabled ()Z
public fun isAnrReportInDebug ()Z
Expand All @@ -149,7 +148,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun setEnableAutoActivityLifecycleTracing (Z)V
public fun setEnableSystemEventBreadcrumbs (Z)V
public fun setEnableUserInteractionBreadcrumbs (Z)V
public fun setProfilingTracesDirPath (Ljava/lang/String;)V
public fun setProfilingTracesIntervalMillis (I)V
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import io.sentry.SentryLevel;
import io.sentry.android.fragment.FragmentLifecycleIntegration;
import io.sentry.android.timber.SentryTimberIntegration;
import io.sentry.util.FileUtils;
import io.sentry.util.Objects;
import java.io.BufferedInputStream;
import java.io.File;
Expand Down Expand Up @@ -297,31 +296,12 @@ private static void readDefaultOptionValues(
* @param context the Application context
* @param options the SentryAndroidOptions
*/
@SuppressWarnings("FutureReturnValueIgnored")
private static void initializeCacheDirs(
final @NotNull Context context, final @NotNull SentryAndroidOptions options) {
final File cacheDir = new File(context.getCacheDir(), "sentry");
final File profilingTracesDir = new File(cacheDir, "profiling_traces");
options.setCacheDirPath(cacheDir.getAbsolutePath());
options.setProfilingTracesDirPath(profilingTracesDir.getAbsolutePath());

if (options.isProfilingEnabled()) {
profilingTracesDir.mkdirs();
final File[] oldTracesDirContent = profilingTracesDir.listFiles();

options
.getExecutorService()
.submit(
() -> {
if (oldTracesDirContent == null) return;
// Method trace files are normally deleted at the end of traces, but if that fails
// for some
// reason we try to clear any old files here.
for (File f : oldTracesDirContent) {
FileUtils.deleteRecursively(f);
}
});
}
}

private static boolean isNdkAvailable(final @NotNull BuildInfoProvider buildInfoProvider) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import io.sentry.SpanStatus;
import io.sentry.protocol.SdkVersion;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

/** Sentry SDK options for Android */
public final class SentryAndroidOptions extends SentryOptions {
Expand Down Expand Up @@ -85,11 +84,8 @@ public final class SentryAndroidOptions extends SentryOptions {
*/
private boolean enableActivityLifecycleTracingAutoFinish = true;

/** The cache dir. path for caching profiling traces */
private @Nullable String profilingTracesDirPath;

/** Interval for profiling traces in milliseconds. Defaults to 300 times per second */
private int profilingTracesIntervalMillis = 1_000 / 300;
/** Interval for profiling traces in milliseconds. Defaults to 100 times per second */
private int profilingTracesIntervalMillis = 1_000 / 100;

/** Interface that loads the debug images list */
private @NotNull IDebugImagesLoader debugImagesLoader = NoOpDebugImagesLoader.getInstance();
Expand Down Expand Up @@ -226,24 +222,6 @@ public void enableAllAutoBreadcrumbs(boolean enable) {
enableUserInteractionBreadcrumbs = enable;
}

/**
* Returns the profiling traces dir. path if set
*
* @return the profiling traces dir. path or null if not set
*/
public @Nullable String getProfilingTracesDirPath() {
return profilingTracesDirPath;
}

/**
* Sets the profiling traces dir. path
*
* @param profilingTracesDirPath the profiling traces dir. path
*/
public void setProfilingTracesDirPath(@Nullable String profilingTracesDirPath) {
this.profilingTracesDirPath = profilingTracesDirPath;
}

/**
* Returns the interval for profiling traces in milliseconds.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,16 +163,6 @@ class AndroidOptionsInitializerTest {
assertFalse(File(fixture.sentryOptions.profilingTracesDirPath!!).exists())
}

@Test
fun `profilingTracesDirPath should be created and cleared when profiling is enabled`() {
fixture.initSut(configureOptions = {
isProfilingEnabled = true
})

assertTrue(File(fixture.sentryOptions.profilingTracesDirPath!!).exists())
assertTrue(File(fixture.sentryOptions.profilingTracesDirPath!!).list()!!.isEmpty())
}

@Test
fun `outboxDir should be set at initialization`() {
fixture.initSut()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.whenever
import io.sentry.Sentry
import io.sentry.SentryTracer
import io.sentry.TransactionContext
import io.sentry.test.getCtor
Expand All @@ -28,10 +29,12 @@ class AndroidTransactionProfilerTest {
private lateinit var file: File

private class Fixture {
private val mockDsn = "http://key@localhost/proj"
val buildInfo = mock<BuildInfoProvider> {
whenever(it.sdkInfoVersion).thenReturn(Build.VERSION_CODES.LOLLIPOP)
}
val options = SentryAndroidOptions().apply {
dsn = mockDsn
isProfilingEnabled = true
}
val transaction1 = SentryTracer(TransactionContext("", ""), mock())
Expand All @@ -46,6 +49,7 @@ class AndroidTransactionProfilerTest {
context = ApplicationProvider.getApplicationContext()
file = context.cacheDir
AndroidOptionsInitializer.init(fixture.options, createMockContext())
Sentry.init(fixture.options)
}

@AfterTest
Expand Down
2 changes: 2 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -1137,6 +1137,7 @@ public class io/sentry/SentryOptions {
public fun getMaxSpans ()I
public fun getMaxTraceFileSize ()J
public fun getOutboxPath ()Ljava/lang/String;
public fun getProfilingTracesDirPath ()Ljava/lang/String;
public fun getProguardUuid ()Ljava/lang/String;
public fun getProxy ()Lio/sentry/SentryOptions$Proxy;
public fun getReadTimeoutMillis ()I
Expand Down Expand Up @@ -1208,6 +1209,7 @@ public class io/sentry/SentryOptions {
public fun setMaxTraceFileSize (J)V
public fun setPrintUncaughtStackTrace (Z)V
public fun setProfilingEnabled (Z)V
public fun setProfilingTracesDirPath (Ljava/lang/String;)V
public fun setProguardUuid (Ljava/lang/String;)V
public fun setProxy (Lio/sentry/SentryOptions$Proxy;)V
public fun setReadTimeoutMillis (I)V
Expand Down
24 changes: 24 additions & 0 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import io.sentry.config.PropertiesProviderFactory;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.User;
import io.sentry.util.FileUtils;
import java.io.File;
import java.lang.reflect.InvocationTargetException;
import java.util.Date;
Expand Down Expand Up @@ -190,6 +191,7 @@ private static synchronized void init(
}
}

@SuppressWarnings("FutureReturnValueIgnored")
private static boolean initConfigurations(final @NotNull SentryOptions options) {
if (options.isEnableExternalConfiguration()) {
options.merge(ExternalOptions.from(PropertiesProviderFactory.create(), options.getLogger()));
Expand Down Expand Up @@ -231,6 +233,28 @@ private static boolean initConfigurations(final @NotNull SentryOptions options)
options.setEnvelopeDiskCache(EnvelopeCache.create(options));
}

String profilingTracesDirPath = options.getProfilingTracesDirPath();
if (options.isProfilingEnabled()
&& profilingTracesDirPath != null
&& !profilingTracesDirPath.isEmpty()) {

final File profilingTracesDir = new File(profilingTracesDirPath);
profilingTracesDir.mkdirs();
final File[] oldTracesDirContent = profilingTracesDir.listFiles();

options
.getExecutorService()
.submit(
() -> {
if (oldTracesDirContent == null) return;
// Method trace files are normally deleted at the end of traces, but if that fails
// for some reason we try to clear any old files here.
for (File f : oldTracesDirContent) {
FileUtils.deleteRecursively(f);
}
});
}

return true;
}

Expand Down
21 changes: 21 additions & 0 deletions sentry/src/main/java/io/sentry/SentryOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,9 @@ public class SentryOptions {
/** Control if profiling is enabled or not for transactions */
private boolean profilingEnabled = false;

/** The cache dir. path for caching profiling traces */
private @Nullable String profilingTracesDirPath;

/** Max trace file size in bytes. */
private long maxTraceFileSize = 5 * 1024 * 1024;

Expand Down Expand Up @@ -1468,6 +1471,24 @@ public void setProfilingEnabled(boolean profilingEnabled) {
this.profilingEnabled = profilingEnabled;
}

/**
* Returns the profiling traces dir. path if set
*
* @return the profiling traces dir. path or null if not set
*/
public @Nullable String getProfilingTracesDirPath() {
return profilingTracesDirPath;
}

/**
* Sets the profiling traces dir. path
*
* @param profilingTracesDirPath the profiling traces dir. path
*/
public void setProfilingTracesDirPath(@Nullable String profilingTracesDirPath) {
this.profilingTracesDirPath = profilingTracesDirPath;
}

/**
* Returns a list of origins to which `sentry-trace` header should be sent in HTTP integrations.
*
Expand Down
25 changes: 25 additions & 0 deletions sentry/src/test/java/io/sentry/SentryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,31 @@ class SentryTest {
assertTrue(Sentry.isCrashedLastRun()!!)
}

@Test
fun `profilingTracesDirPath should be created and cleared at initialization when profiling is enabled`() {
val tracesDirPath = getTempPath()
Sentry.init {
it.dsn = dsn
it.isProfilingEnabled = true
it.profilingTracesDirPath = tracesDirPath
}

assertTrue(File(tracesDirPath).exists())
assertTrue(File(tracesDirPath).list()!!.isEmpty())
}

@Test
fun `profilingTracesDirPath should not be created and cleared when profiling is disabled`() {
val tracesDirPath = getTempPath()
Sentry.init {
it.dsn = dsn
it.isProfilingEnabled = false
it.profilingTracesDirPath = tracesDirPath
}

assertFalse(File(tracesDirPath).exists())
}

private fun getTempPath(): String {
val tempFile = Files.createTempDirectory("cache").toFile()
tempFile.delete()
Expand Down