Skip to content

Commit

Permalink
all operations involving file reads were moved to background threads
Browse files Browse the repository at this point in the history
  • Loading branch information
stefanosiano committed Mar 24, 2022
1 parent 2c1fe98 commit 9f90ba0
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,8 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
scheduledFinish = null;
}

if (traceFile == null || !traceFile.exists()) {
options
.getLogger()
.log(
SentryLevel.ERROR,
"Trace file %s does not exists",
traceFile == null ? "null" : traceFile.getPath());
if (traceFile == null) {
options.getLogger().log(SentryLevel.ERROR, "Trace file does not exists");
return null;
}

Expand All @@ -221,16 +216,18 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
totalMem = Long.toString(memInfo.totalMem);
}

// cpu max frequencies are read with a lambda because reading files is involved, so it will be
// done in the background when the trace file is read
return new ProfilingTraceData(
traceFile,
transaction,
Long.toString(transactionDurationNanos),
buildInfoProvider.getSdkInfoVersion(),
() -> CpuInfoUtils.getInstance().readMaxFrequencies(),
buildInfoProvider.getManufacturer(),
buildInfoProvider.getModel(),
buildInfoProvider.getVersionRelease(),
buildInfoProvider.isEmulator(),
CpuInfoUtils.getInstance().readMaxFrequencies(),
totalMem,
options.getProguardUuid(),
versionName,
Expand Down
3 changes: 2 additions & 1 deletion sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ public final class io/sentry/OutboxSender : io/sentry/IEnvelopeSender {
}

public final class io/sentry/ProfilingTraceData {
public fun <init> (Ljava/io/File;Lio/sentry/ITransaction;Ljava/lang/String;ILjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Boolean;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
public fun <init> (Ljava/io/File;Lio/sentry/ITransaction;Ljava/lang/String;ILjava/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 getAndroid_api_level ()I
public fun getBuild_id ()Ljava/lang/String;
public fun getDevice_cpu_frequencies ()Ljava/util/List;
Expand All @@ -541,6 +541,7 @@ public final class io/sentry/ProfilingTraceData {
public fun getVersion_code ()Ljava/lang/String;
public fun getVersion_name ()Ljava/lang/String;
public fun isDevice_is_emulator ()Z
public fun readDeviceCpuFrequencies ()V
public fun setSampled_profile (Ljava/lang/String;)V
}

Expand Down
17 changes: 14 additions & 3 deletions sentry/src/main/java/io/sentry/ProfilingTraceData.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package io.sentry;

import java.io.File;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.UUID;
import java.util.concurrent.Callable;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -13,6 +15,7 @@ public final class ProfilingTraceData {

// This field is transient so that it's ignored by Gson
private final transient @NotNull File traceFile;
private final transient @NotNull Callable<List<Integer>> deviceCpuFrequenciesReader;

// Device metadata
private final int android_api_level;
Expand All @@ -23,7 +26,7 @@ public final class ProfilingTraceData {
private final @NotNull String device_os_name;
private final @NotNull String device_os_version;
private final boolean device_is_emulator;
private final @NotNull List<Integer> device_cpu_frequencies;
private @NotNull List<Integer> device_cpu_frequencies = new ArrayList<>();
private final @NotNull String device_physical_memory_bytes;
private final @NotNull String platform;
private final @NotNull String build_id;
Expand Down Expand Up @@ -52,17 +55,18 @@ public ProfilingTraceData(
@NotNull ITransaction transaction,
@NotNull String durationNanos,
int sdkInt,
@NotNull Callable<List<Integer>> deviceCpuFrequenciesReader,
@Nullable String deviceManufacturer,
@Nullable String deviceModel,
@Nullable String deviceOsVersion,
@Nullable Boolean deviceIsEmulator,
@NotNull List<Integer> deviceCpuFrequencies,
@Nullable String devicePhysicalMemoryBytes,
@Nullable String buildId,
@Nullable String versionName,
@Nullable String versionCode,
@Nullable String environment) {
this.traceFile = traceFile;
this.deviceCpuFrequenciesReader = deviceCpuFrequenciesReader;

// Device metadata
this.android_api_level = sdkInt;
Expand All @@ -71,7 +75,6 @@ public ProfilingTraceData(
this.device_model = deviceModel != null ? deviceModel : "";
this.device_os_version = deviceOsVersion != null ? deviceOsVersion : "";
this.device_is_emulator = deviceIsEmulator != null ? deviceIsEmulator : false;
this.device_cpu_frequencies = deviceCpuFrequencies;
this.device_physical_memory_bytes =
devicePhysicalMemoryBytes != null ? devicePhysicalMemoryBytes : "0";
this.device_os_build_number = "";
Expand Down Expand Up @@ -189,4 +192,12 @@ public boolean isDevice_is_emulator() {
public void setSampled_profile(@Nullable String sampledProfile) {
this.sampled_profile = sampledProfile;
}

public void readDeviceCpuFrequencies() {
try {
this.device_cpu_frequencies = deviceCpuFrequenciesReader.call();
} catch (Exception e) {
this.device_cpu_frequencies = new ArrayList<>();
}
}
}
4 changes: 1 addition & 3 deletions sentry/src/main/java/io/sentry/SentryClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,7 @@ private boolean shouldApplyScopeData(
final SentryEnvelopeItem profilingTraceItem =
SentryEnvelopeItem.fromProfilingTrace(
profilingTraceData, options.getMaxTraceFileSize(), options.getSerializer());
if (profilingTraceItem != null) {
envelopeItems.add(profilingTraceItem);
}
envelopeItems.add(profilingTraceItem);
}

if (attachments != null) {
Expand Down
12 changes: 6 additions & 6 deletions sentry/src/main/java/io/sentry/SentryEnvelopeItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -204,29 +204,29 @@ public static SentryEnvelopeItem fromAttachment(
return new SentryEnvelopeItem(itemHeader, () -> cachedItem.getBytes());
}

public static @Nullable SentryEnvelopeItem fromProfilingTrace(
public static @NotNull SentryEnvelopeItem fromProfilingTrace(
final @NotNull ProfilingTraceData profilingTraceData,
final long maxTraceFileSize,
final @NotNull ISerializer serializer)
throws SentryEnvelopeException {

File traceFile = profilingTraceData.getTraceFile();
if (!traceFile.exists()) {
return null;
}

// Using CachedItem, so we read the trace file in the background
final CachedItem cachedItem =
new CachedItem(
() -> {
if (!traceFile.exists()) {
return null;
throw new SentryEnvelopeException(
String.format(
"Dropping profiling trace data, because the file '%s' doesn't exists",
traceFile.getName()));
}
// The payload of the profile item is a json including the trace file encoded with
// base64
byte[] traceFileBytes = readBytesFromFile(traceFile.getPath(), maxTraceFileSize);
String base64Trace = Base64.encodeToString(traceFileBytes, NO_WRAP | NO_PADDING);
profilingTraceData.setSampled_profile(base64Trace);
profilingTraceData.readDeviceCpuFrequencies();

try (final ByteArrayOutputStream stream = new ByteArrayOutputStream();
final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) {
Expand Down
6 changes: 3 additions & 3 deletions sentry/src/test/java/io/sentry/SentryClientTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ class SentryClientTest {

var attachment = Attachment("hello".toByteArray(), "hello.txt", "text/plain", true)
val profilingTraceFile = Files.createTempFile("trace", ".trace").toFile()
var profilingTraceData = ProfilingTraceData(profilingTraceFile, sentryTracer, "1", 0, "", "", "", false, emptyList(), "", "", "", "", "")
var profilingNonExistingTraceData = ProfilingTraceData(File("non_existent.trace"), sentryTracer, "1", 0, "", "", "", false, emptyList(), "", "", "", "", "")
var profilingTraceData = ProfilingTraceData(profilingTraceFile, sentryTracer, "1", 0, { emptyList() }, "", "", "", false, "", "", "", "", "")
var profilingNonExistingTraceData = ProfilingTraceData(File("non_existent.trace"), sentryTracer, "1", 0, { emptyList() }, "", "", "", false, "", "", "", "", "")

fun getSut() = SentryClient(sentryOptions)
}
Expand Down Expand Up @@ -1328,7 +1328,7 @@ class SentryClientTest {
val profilingTraceItem = actual.items.firstOrNull { item ->
item.header.type == SentryItemType.Profile
}
assertNotNull(profilingTraceItem)
assertNotNull(profilingTraceItem?.data)
},
isNull()
)
Expand Down
18 changes: 11 additions & 7 deletions sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ class SentryEnvelopeItemTest {
}

file.writeBytes(fixture.bytes)
val traceDataBytes = SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, serializer)?.data
val traceDataBytes = SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, serializer).data
val reader = BufferedReader(InputStreamReader(ByteArrayInputStream(traceDataBytes)))
val deserData = serializer.deserialize(reader, ProfilingTraceData::class.java)
assertNotNull(deserData)
Expand All @@ -238,22 +238,26 @@ class SentryEnvelopeItemTest {
assert(file.exists())
val traceData = SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock())
assert(file.exists())
traceData?.data
traceData.data
assertFalse(file.exists())
}

@Test
fun `fromProfilingTrace with invalid file returns null`() {
fun `fromProfilingTrace with invalid file throws`() {
val file = File(fixture.pathname)
val profilingTraceData = mock<ProfilingTraceData> {
whenever(it.traceFile).thenReturn(file)
}

assertNull(SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock())?.data)
assertFailsWith<SentryEnvelopeException>("Dropping profiling trace data, because the file ${file.path} doesn't exists") {
SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock()).data
}
file.writeBytes(fixture.bytes)
assertNotNull(SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock())?.data)
assertNotNull(SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock()).data)
file.setReadable(false)
assertNull(SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock())?.data)
assertFailsWith<SentryEnvelopeException>("Dropping profiling trace data, because the file ${file.path} doesn't exists") {
SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock()).data
}
}

@Test
Expand All @@ -265,7 +269,7 @@ class SentryEnvelopeItemTest {
}

val exception = assertFailsWith<SentryEnvelopeException> {
SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock())?.data
SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock()).data
}

assertEquals(
Expand Down

0 comments on commit 9f90ba0

Please sign in to comment.