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

Concurrent profiling 3 - truncation reason #2247

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

### Features

- Concurrent profiling 3 - added truncation reason ([#2247](https://github.com/getsentry/sentry-java/pull/2247))
- Concurrent profiling 2 - added list of transactions ([#2218](https://github.com/getsentry/sentry-java/pull/2218))
- Concurrent profiling 1 - added envelope payload data format ([#2216](https://github.com/getsentry/sentry-java/pull/2216))
- Send source for transactions ([#2180](https://github.com/getsentry/sentry-java/pull/2180))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import java.util.concurrent.Future;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.TestOnly;

final class AndroidTransactionProfiler implements ITransactionProfiler {

Expand Down Expand Up @@ -304,7 +303,10 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
options.getProguardUuid(),
versionName,
versionCode,
options.getEnvironment());
options.getEnvironment(),
isTimeout
? ProfilingTraceData.TRUNCATION_REASON_TIMEOUT
: ProfilingTraceData.TRUNCATION_REASON_NORMAL);
}

/**
Expand All @@ -327,9 +329,4 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
return null;
}
}

@TestOnly
void setTimedOutProfilingData(@Nullable ProfilingTraceData data) {
this.timedOutProfilingData = data;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,17 @@ import android.content.Context
import android.os.Build
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.argumentCaptor
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.never
import com.nhaarman.mockitokotlin2.spy
import com.nhaarman.mockitokotlin2.times
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import io.sentry.ILogger
import io.sentry.ISentryExecutorService
import io.sentry.ProfilingTraceData
import io.sentry.SentryLevel
import io.sentry.SentryTracer
import io.sentry.TransactionContext
Expand Down Expand Up @@ -39,7 +44,7 @@ class AndroidTransactionProfilerTest {
whenever(it.sdkInfoVersion).thenReturn(Build.VERSION_CODES.LOLLIPOP)
}
val mockLogger = mock<ILogger>()
val options = SentryAndroidOptions().apply {
val options = spy(SentryAndroidOptions()).apply {
dsn = mockDsn
profilesSampleRate = 1.0
isDebug = true
Expand Down Expand Up @@ -229,25 +234,49 @@ class AndroidTransactionProfilerTest {
fun `onTransactionFinish returns timedOutData to the timed out transaction once, even after other transactions`() {
val profiler = fixture.getSut(context)

val executorService = mock<ISentryExecutorService>()
val captor = argumentCaptor<Runnable>()
whenever(executorService.schedule(captor.capture(), any())).thenReturn(null)
whenever(fixture.options.executorService).thenReturn(executorService)
// Start and finish first transaction profiling
profiler.onTransactionStart(fixture.transaction1)
val traceData = profiler.onTransactionFinish(fixture.transaction1)

// Set timed out data
profiler.setTimedOutProfilingData(traceData)
// Set timed out data by calling the timeout scheduled job
captor.firstValue.run()

// Start and finish second transaction profiling
// Start and finish second transaction. Since profiler returned data, it means no other profiling is running
profiler.onTransactionStart(fixture.transaction2)
assertEquals(fixture.transaction2.eventId.toString(), profiler.onTransactionFinish(fixture.transaction2)!!.transactionId)

// First transaction finishes: timed out data is returned
val traceData2 = profiler.onTransactionFinish(fixture.transaction1)
assertEquals(traceData, traceData2)
val traceData = profiler.onTransactionFinish(fixture.transaction1)
assertEquals(traceData!!.transactionId, fixture.transaction1.eventId.toString())

// If first transaction is finished again, nothing is returned
assertNull(profiler.onTransactionFinish(fixture.transaction1))
}

@Test
fun `timedOutData has timeout truncation reason`() {
val profiler = fixture.getSut(context)

val executorService = mock<ISentryExecutorService>()
val captor = argumentCaptor<Runnable>()
whenever(executorService.schedule(captor.capture(), any())).thenReturn(null)
whenever(fixture.options.executorService).thenReturn(executorService)

// Start and finish first transaction profiling
profiler.onTransactionStart(fixture.transaction1)

// Set timed out data by calling the timeout scheduled job
captor.firstValue.run()

// First transaction finishes: timed out data is returned
val traceData = profiler.onTransactionFinish(fixture.transaction1)
assertEquals(traceData!!.transactionId, fixture.transaction1.eventId.toString())
assertEquals(ProfilingTraceData.TRUNCATION_REASON_TIMEOUT, traceData.truncationReason)
}

@Test
fun `profiling stops and returns data only when the last transaction finishes`() {
val profiler = fixture.getSut(context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ class EnvelopeTests : BaseUiTest() {
assertEquals(transaction3.eventId.toString(), transactionItem.eventId.toString())
assertEquals(profilingTraceData.transactionId, transactionItem.eventId.toString())
assertTrue(profilingTraceData.transactionName == "e2etests2")
assertTrue(profilingTraceData.truncationReason == "normal")

// The transaction list is not ordered, since it's stored using a map to be able to quickly check the
// existence of a certain id. So we order the list to make more meaningful checks on timestamps.
Expand Down
7 changes: 6 additions & 1 deletion sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -711,8 +711,11 @@ public final class io/sentry/OutboxSender : io/sentry/IEnvelopeSender {
}

public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io/sentry/JsonUnknown {
public static final field TRUNCATION_REASON_BACKGROUNDED Ljava/lang/String;
public static final field TRUNCATION_REASON_NORMAL Ljava/lang/String;
public static final field TRUNCATION_REASON_TIMEOUT Ljava/lang/String;
public fun <init> (Ljava/io/File;Lio/sentry/ITransaction;)V
public fun <init> (Ljava/io/File;Ljava/util/List;Lio/sentry/ITransaction;Ljava/lang/String;ILjava/lang/String;Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Boolean;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
public fun <init> (Ljava/io/File;Ljava/util/List;Lio/sentry/ITransaction;Ljava/lang/String;ILjava/lang/String;Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Boolean;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
public fun getAndroidApiLevel ()I
public fun getBuildId ()Ljava/lang/String;
public fun getCpuArchitecture ()Ljava/lang/String;
Expand All @@ -734,6 +737,7 @@ public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io
public fun getTransactionId ()Ljava/lang/String;
public fun getTransactionName ()Ljava/lang/String;
public fun getTransactions ()Ljava/util/List;
public fun getTruncationReason ()Ljava/lang/String;
public fun getUnknown ()Ljava/util/Map;
public fun getVersionCode ()Ljava/lang/String;
public fun getVersionName ()Ljava/lang/String;
Expand Down Expand Up @@ -791,6 +795,7 @@ public final class io/sentry/ProfilingTraceData$JsonKeys {
public static final field TRANSACTION_ID Ljava/lang/String;
public static final field TRANSACTION_LIST Ljava/lang/String;
public static final field TRANSACTION_NAME Ljava/lang/String;
public static final field TRUNCATION_REASON Ljava/lang/String;
public static final field VERSION_CODE Ljava/lang/String;
public static final field VERSION_NAME Ljava/lang/String;
public fun <init> ()V
Expand Down
34 changes: 32 additions & 2 deletions sentry/src/main/java/io/sentry/ProfilingTraceData.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ public final class ProfilingTraceData implements JsonUnknown, JsonSerializable {
*/
private static final String DEFAULT_ENVIRONMENT = "production";

@ApiStatus.Internal public static final String TRUNCATION_REASON_NORMAL = "normal";
@ApiStatus.Internal public static final String TRUNCATION_REASON_TIMEOUT = "timeout";
// Backgrounded reason is not used, yet, but it's one of the possible values
@ApiStatus.Internal public static final String TRUNCATION_REASON_BACKGROUNDED = "backgrounded";

private @NotNull File traceFile;
private @Nullable Callable<List<Integer>> deviceCpuFrequenciesReader;

Expand Down Expand Up @@ -56,6 +61,7 @@ public final class ProfilingTraceData implements JsonUnknown, JsonSerializable {
private @NotNull String traceId;
private @NotNull String profileId;
private @NotNull String environment;
private @NotNull String truncationReason;

// Stacktrace (file)
/** Profile trace encoded with Base64 */
Expand Down Expand Up @@ -83,7 +89,8 @@ public ProfilingTraceData(@NotNull File traceFile, @NotNull ITransaction transac
null,
null,
null,
null);
null,
TRUNCATION_REASON_NORMAL);
}

public ProfilingTraceData(
Expand All @@ -102,7 +109,8 @@ public ProfilingTraceData(
@Nullable String buildId,
@Nullable String versionName,
@Nullable String versionCode,
@Nullable String environment) {
@Nullable String environment,
@NotNull String truncationReason) {
this.traceFile = traceFile;
this.cpuArchitecture = cpuArchitecture;
this.deviceCpuFrequenciesReader = deviceCpuFrequenciesReader;
Expand Down Expand Up @@ -135,6 +143,16 @@ public ProfilingTraceData(
this.traceId = transaction.getSpanContext().getTraceId().toString();
this.profileId = UUID.randomUUID().toString();
this.environment = environment != null ? environment : DEFAULT_ENVIRONMENT;
this.truncationReason = truncationReason;
if (!isTruncationReasonValid()) {
this.truncationReason = TRUNCATION_REASON_NORMAL;
}
}

private boolean isTruncationReasonValid() {
return truncationReason.equals(TRUNCATION_REASON_NORMAL)
|| truncationReason.equals(TRUNCATION_REASON_TIMEOUT)
|| truncationReason.equals(TRUNCATION_REASON_BACKGROUNDED);
}

private @Nullable Map<String, Object> unknown;
Expand Down Expand Up @@ -235,6 +253,10 @@ public boolean isDeviceIsEmulator() {
return devicePhysicalMemoryBytes;
}

public @NotNull String getTruncationReason() {
return truncationReason;
}

public void setAndroidApiLevel(int androidApiLevel) {
this.androidApiLevel = androidApiLevel;
}
Expand Down Expand Up @@ -351,6 +373,7 @@ public static final class JsonKeys {
public static final String PROFILE_ID = "profile_id";
public static final String ENVIRONMENT = "environment";
public static final String SAMPLED_PROFILE = "sampled_profile";
public static final String TRUNCATION_REASON = "truncation_reason";
}

@Override
Expand Down Expand Up @@ -382,6 +405,7 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger)
writer.name(JsonKeys.TRACE_ID).value(traceId);
writer.name(JsonKeys.PROFILE_ID).value(profileId);
writer.name(JsonKeys.ENVIRONMENT).value(environment);
writer.name(JsonKeys.TRUNCATION_REASON).value(truncationReason);
if (sampledProfile != null) {
writer.name(JsonKeys.SAMPLED_PROFILE).value(sampledProfile);
}
Expand Down Expand Up @@ -552,6 +576,12 @@ public static final class Deserializer implements JsonDeserializer<ProfilingTrac
data.environment = environment;
}
break;
case JsonKeys.TRUNCATION_REASON:
String truncationReason = reader.nextStringOrNull();
if (truncationReason != null) {
data.truncationReason = truncationReason;
}
break;
case JsonKeys.SAMPLED_PROFILE:
String sampledProfile = reader.nextStringOrNull();
if (sampledProfile != null) {
Expand Down