diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 353ae01d0c..01ed1c7ff6 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -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; } @@ -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, diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 064ed9f6d6..37ac344d2c 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -517,7 +517,7 @@ public final class io/sentry/OutboxSender : io/sentry/IEnvelopeSender { } public final class io/sentry/ProfilingTraceData { - public fun (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 (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; @@ -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 } diff --git a/sentry/src/main/java/io/sentry/ProfilingTraceData.java b/sentry/src/main/java/io/sentry/ProfilingTraceData.java index a389dd3ddd..fb4bce5434 100644 --- a/sentry/src/main/java/io/sentry/ProfilingTraceData.java +++ b/sentry/src/main/java/io/sentry/ProfilingTraceData.java @@ -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; @@ -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> deviceCpuFrequenciesReader; // Device metadata private final int android_api_level; @@ -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 device_cpu_frequencies; + private @NotNull List device_cpu_frequencies = new ArrayList<>(); private final @NotNull String device_physical_memory_bytes; private final @NotNull String platform; private final @NotNull String build_id; @@ -52,17 +55,18 @@ public ProfilingTraceData( @NotNull ITransaction transaction, @NotNull String durationNanos, int sdkInt, + @NotNull Callable> deviceCpuFrequenciesReader, @Nullable String deviceManufacturer, @Nullable String deviceModel, @Nullable String deviceOsVersion, @Nullable Boolean deviceIsEmulator, - @NotNull List 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; @@ -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 = ""; @@ -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<>(); + } + } } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 9864dfa66b..d1c437472d 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -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) { diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index b110f6491c..ea53ec686d 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -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))) { diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index b2dca722ad..29b4e40ac9 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -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) } @@ -1328,7 +1328,7 @@ class SentryClientTest { val profilingTraceItem = actual.items.firstOrNull { item -> item.header.type == SentryItemType.Profile } - assertNotNull(profilingTraceItem) + assertNotNull(profilingTraceItem?.data) }, isNull() ) diff --git a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt index 87a3bc4ccb..da4ba2cde0 100644 --- a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt +++ b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt @@ -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) @@ -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 { whenever(it.traceFile).thenReturn(file) } - assertNull(SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock())?.data) + assertFailsWith("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("Dropping profiling trace data, because the file ${file.path} doesn't exists") { + SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock()).data + } } @Test @@ -265,7 +269,7 @@ class SentryEnvelopeItemTest { } val exception = assertFailsWith { - SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock())?.data + SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock()).data } assertEquals(