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

AndroidTransactionProfiler is now initialized the first time a transa… #2009

Merged
merged 5 commits into from
Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

## Unreleased

* Fix: Profiling rate decreased from 300hz to 100hz; fixed profiling traces folder creation on manual sdk init (#1997)
* Fix: Profiling rate decreased from 300hz to 100hz (#1997)
* Fix: Android profiling initializes on first profile start (#2009)
* Fix: Allow disabling sending of client reports via Android Manifest and external options (#2007)
* Ref: Upgrade Spring Boot dependency to 2.5.13 (#2011)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ final class AndroidTransactionProfiler implements ITransactionProfiler {
private final @NotNull BuildInfoProvider buildInfoProvider;
private final @Nullable PackageInfo packageInfo;
private long transactionStartNanos = 0;
private boolean isInitialized = false;

public AndroidTransactionProfiler(
final @NotNull Context context,
Expand All @@ -59,6 +60,14 @@ public AndroidTransactionProfiler(
this.buildInfoProvider =
Objects.requireNonNull(buildInfoProvider, "The BuildInfoProvider is required.");
this.packageInfo = ContextUtils.getPackageInfo(context, options.getLogger());
}

private void init() {
// We initialize it only once
if (isInitialized) {
return;
}
isInitialized = true;
final String tracesFilesDirPath = options.getProfilingTracesDirPath();
if (!options.isProfilingEnabled()) {
options.getLogger().log(SentryLevel.INFO, "Profiling is disabled in options.");
Expand Down Expand Up @@ -93,7 +102,10 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
// Debug.startMethodTracingSampling() is only available since Lollipop
if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) return;

// traceFilesDir is null or intervalUs is 0 only if there was a problem in the constructor, but
// Let's initialize trace folder and profiling interval
init();

// traceFilesDir is null or intervalUs is 0 only if there was a problem in the init, but
// we already logged that
if (traceFilesDir == null || intervalUs == 0 || !traceFilesDir.exists()) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ import android.os.Build
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.never
import com.nhaarman.mockitokotlin2.times
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import io.sentry.ILogger
import io.sentry.SentryLevel
import io.sentry.SentryTracer
import io.sentry.TransactionContext
import io.sentry.test.getCtor
Expand All @@ -31,9 +36,12 @@ class AndroidTransactionProfilerTest {
val buildInfo = mock<BuildInfoProvider> {
whenever(it.sdkInfoVersion).thenReturn(Build.VERSION_CODES.LOLLIPOP)
}
val mockLogger = mock<ILogger>()
val options = SentryAndroidOptions().apply {
dsn = mockDsn
isProfilingEnabled = true
isDebug = true
setLogger(mockLogger)
}
val transaction1 = SentryTracer(TransactionContext("", ""), mock())
val transaction2 = SentryTracer(TransactionContext("", ""), mock())
Expand All @@ -45,7 +53,7 @@ class AndroidTransactionProfilerTest {
@BeforeTest
fun `set up`() {
context = ApplicationProvider.getApplicationContext()
AndroidOptionsInitializer.init(fixture.options, context)
AndroidOptionsInitializer.init(fixture.options, context, fixture.mockLogger, false, false)
// Profiler doesn't start if the folder doesn't exists.
// Usually it's generated when calling Sentry.init, but for tests we can create it manually.
File(fixture.options.profilingTracesDirPath).mkdirs()
Expand Down Expand Up @@ -101,6 +109,68 @@ class AndroidTransactionProfilerTest {
assertNull(traceData)
}

@Test
fun `profiler evaluates isProfiling options only on first transaction profiling`() {
fixture.options.apply {
isProfilingEnabled = false
}

// We create the profiler, and nothing goes wrong
val profiler = fixture.getSut(context)
verify(fixture.mockLogger, never()).log(SentryLevel.INFO, "Profiling is disabled in options.")

// Regardless of how many times the profiler is started, the option is evaluated and logged only once
profiler.onTransactionStart(fixture.transaction1)
profiler.onTransactionStart(fixture.transaction1)
verify(fixture.mockLogger, times(1)).log(SentryLevel.INFO, "Profiling is disabled in options.")
}

@Test
fun `profiler evaluates profilingTracesDirPath options only on first transaction profiling`() {
fixture.options.apply {
profilingTracesDirPath = ""
}

// We create the profiler, and nothing goes wrong
val profiler = fixture.getSut(context)
verify(fixture.mockLogger, never()).log(
SentryLevel.WARNING,
"Disabling profiling because no profiling traces dir path is defined in options."
)

// Regardless of how many times the profiler is started, the option is evaluated and logged only once
profiler.onTransactionStart(fixture.transaction1)
profiler.onTransactionStart(fixture.transaction1)
verify(fixture.mockLogger, times(1)).log(
SentryLevel.WARNING,
"Disabling profiling because no profiling traces dir path is defined in options."
)
}

@Test
fun `profiler evaluates profilingTracesIntervalMillis options only on first transaction profiling`() {
fixture.options.apply {
profilingTracesIntervalMillis = 0
}

// We create the profiler, and nothing goes wrong
val profiler = fixture.getSut(context)
verify(fixture.mockLogger, never()).log(
SentryLevel.WARNING,
"Disabling profiling because trace interval is set to %d milliseconds",
0L
)

// Regardless of how many times the profiler is started, the option is evaluated and logged only once
profiler.onTransactionStart(fixture.transaction1)
profiler.onTransactionStart(fixture.transaction1)
verify(fixture.mockLogger, times(1)).log(
SentryLevel.WARNING,
"Disabling profiling because trace interval is set to %d milliseconds",
0L
)
}

@Test
fun `profiler on tracesDirPath null`() {
fixture.options.apply {
Expand Down