From 6dcdb4a6d6d06f581f9a7f8486ddf4c031e5beeb Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 2 Feb 2022 17:41:41 +0100 Subject: [PATCH 01/14] First pull request for profiling traces: added a transaction listener (noOp on java sdk) that starts/stops android profiling when a transaction starts or is finished added an envelope generation method from android trace file added following SentryOptions: profilingTracesDirPath, profilingEnabled, maxTraceFileSize, profilingTracesIntervalMs, transactionListener --- .../api/sentry-android-core.api | 12 ++ .../core/AndroidOptionsInitializer.java | 3 + .../core/AndroidTraceTransactionListener.java | 148 ++++++++++++++++++ .../sentry/android/core/util/FileUtils.java | 48 ++++++ sentry/api/sentry.api | 29 ++++ sentry/src/main/java/io/sentry/Hub.java | 10 +- .../java/io/sentry/ITransactionListener.java | 11 ++ .../io/sentry/NoOpTransactionListener.java | 18 +++ .../main/java/io/sentry/SentryEnvelope.java | 18 +++ .../java/io/sentry/SentryEnvelopeItem.java | 115 ++++++++------ .../main/java/io/sentry/SentryItemType.java | 2 + .../main/java/io/sentry/SentryOptions.java | 112 +++++++++++++ .../src/main/java/io/sentry/SentryTracer.java | 2 + .../sentry/transport/ReusableCountLatch.java | 8 +- .../java/io/sentry/util/SentryExecutors.java | 13 ++ sentry/src/test/java/io/sentry/HubTest.kt | 2 +- .../test/java/io/sentry/OutboxSenderTest.kt | 1 + .../java/io/sentry/SentryEnvelopeItemTest.kt | 4 +- 18 files changed, 504 insertions(+), 52 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/util/FileUtils.java create mode 100644 sentry/src/main/java/io/sentry/ITransactionListener.java create mode 100644 sentry/src/main/java/io/sentry/NoOpTransactionListener.java create mode 100644 sentry/src/main/java/io/sentry/util/SentryExecutors.java diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 132691edb0a..234248f1308 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -12,6 +12,12 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V } +public final class io/sentry/android/core/AndroidTraceTransactionListener : io/sentry/ITransactionListener { + public fun (Lio/sentry/SentryOptions;)V + public fun onTransactionEnd (Lio/sentry/ITransaction;)V + public fun onTransactionStart (Lio/sentry/ITransaction;)V +} + public final class io/sentry/android/core/AnrIntegration : io/sentry/Integration, java/io/Closeable { public fun (Landroid/content/Context;)V public fun close ()V @@ -180,6 +186,12 @@ public final class io/sentry/android/core/util/DeviceOrientations { public static fun getOrientation (I)Lio/sentry/protocol/Device$DeviceOrientation; } +public final class io/sentry/android/core/util/FileUtils { + public fun ()V + public static fun deleteRecursively (Ljava/io/File;)Z + public static fun resolve (Ljava/io/File;Ljava/lang/String;)Ljava/io/File; +} + public final class io/sentry/android/core/util/MainThreadChecker { public static fun isMainThread ()Z public static fun isMainThread (Lio/sentry/protocol/SentryThread;)Z diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index e35980e721a..32f3f7607ac 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -116,6 +116,7 @@ static void init( options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); + options.setTransactionListener(new AndroidTraceTransactionListener(options)); } private static void installDefaultIntegrations( @@ -249,7 +250,9 @@ private static void readDefaultOptionValues( private static void initializeCacheDirs( final @NotNull Context context, final @NotNull SentryOptions 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()); } private static boolean isNdkAvailable(final @NotNull IBuildInfoProvider buildInfoProvider) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java new file mode 100644 index 00000000000..788f143296f --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java @@ -0,0 +1,148 @@ +package io.sentry.android.core; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; + +import android.os.Build; +import android.os.Debug; +import io.sentry.ITransaction; +import io.sentry.ITransactionListener; +import io.sentry.Sentry; +import io.sentry.SentryEnvelope; +import io.sentry.SentryLevel; +import io.sentry.SentryOptions; +import io.sentry.android.core.util.FileUtils; +import io.sentry.protocol.SentryId; +import io.sentry.util.Objects; +import io.sentry.util.SentryExecutors; +import java.io.File; +import java.io.IOException; +import java.util.UUID; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class AndroidTraceTransactionListener implements ITransactionListener { + + /** + * This appears to correspond to the buffer size of the data part of the file, excluding the key + * part. Once the buffer is full, new records are ignored, but the resulting trace file will be + * valid. + * + *

30 second traces can require a buffer of a few MB. 8MB is the default buffer size for + * [Debug.startMethodTracingSampling], but 3 should be enough for most cases. We can adjust this + * in the future if we notice that traces are being truncated in some applications. + */ + private static final int BUFFER_SIZE_BYTES = 3_000_000; + + private @Nullable File traceFile = null; + private @Nullable File traceFilesDir = null; + private boolean startedMethodTracing = false; + private @Nullable ITransaction activeTransaction = null; + + private @NotNull final SentryOptions options; + + public AndroidTraceTransactionListener(final @NotNull SentryOptions options) { + this.options = Objects.requireNonNull(options, "SentryOptions is required"); + final String tracesFilesDirPath = this.options.getProfilingTracesDirPath(); + if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) { + this.options + .getLogger() + .log(SentryLevel.ERROR, "No profiling traces dir path is defined in options."); + return; + } + + traceFilesDir = new File(tracesFilesDirPath); + // 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. + FileUtils.deleteRecursively(traceFilesDir); + traceFilesDir.mkdirs(); + } + + @Override + @SuppressWarnings("FutureReturnValueIgnored") + public synchronized void onTransactionStart(ITransaction transaction) { + + // Debug.startMethodTracingSampling() is only available since Lollipop + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) return; + + // Let's be sure to end any running trace + if (activeTransaction != null) onTransactionEnd(activeTransaction); + + traceFile = FileUtils.resolve(traceFilesDir, "sentry-" + UUID.randomUUID() + ".trace"); + + if (traceFile == null) { + options.getLogger().log(SentryLevel.DEBUG, "Could not create a trace file"); + return; + } + + if (traceFile.exists()) { + options + .getLogger() + .log(SentryLevel.DEBUG, "Trace file already exists: %s", traceFile.getPath()); + return; + } + + long intervalMs = options.getProfilingTracesIntervalMs(); + if (intervalMs <= 0) { + options + .getLogger() + .log(SentryLevel.DEBUG, "Profiling trace interval is set to %d milliseconds", intervalMs); + return; + } + + // We stop the trace after 30 seconds, since such a long trace is very probably a trace + // that will never end due to an error + SentryExecutors.tracingExecutor.schedule( + () -> onTransactionEnd(transaction), 30_000, MILLISECONDS); + + startedMethodTracing = true; + int intervalUs = (int) MILLISECONDS.toMicros(intervalMs); + activeTransaction = transaction; + Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs); + } + + @Override + public synchronized void onTransactionEnd(ITransaction transaction) { + // In case a previous timeout tries to end a newer transaction we simply ignore it + if (transaction != activeTransaction) return; + + if (startedMethodTracing) { + startedMethodTracing = false; + + Debug.stopMethodTracing(); + + if (traceFile == null || !traceFile.exists()) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Trace file %s does not exists", + traceFile == null ? "null" : traceFile.getPath()); + return; + } + + // todo should I use transaction.getEventId() instead of new SentryId()? + // Or should I add the transaction id as an header to the envelope? + // Or should I simply ignore the transaction entirely (wouldn't make any sense)? + // And how to check if a trace is from a startup? + try { + SentryEnvelope envelope = + SentryEnvelope.from( + new SentryId(), + traceFile.getPath(), + traceFile.getName(), + options.getSdkVersion(), + options.getMaxTraceFileSize(), + true); + Sentry.getCurrentHub().captureEnvelope(envelope); + } catch (IOException e) { + options.getLogger().log(SentryLevel.ERROR, "Failed to capture session.", e); + return; + } + } + + if (traceFile != null) traceFile.delete(); + + activeTransaction = null; + traceFile = null; + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/util/FileUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/util/FileUtils.java new file mode 100644 index 00000000000..19a8c456611 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/util/FileUtils.java @@ -0,0 +1,48 @@ +package io.sentry.android.core.util; + +import java.io.File; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class FileUtils { + + /** + * Deletes the file or directory denoted by a path. If it is a directory, all files and directory + * inside it are deleted recursively. Note that if this operation fails then partial deletion may + * have taken place. + * + * @param file file or directory to delete + * @return true if the file/directory is successfully deleted, false otherwise + */ + public static boolean deleteRecursively(@Nullable File file) { + if (file == null || !file.exists()) { + return true; + } + if (file.isFile()) { + return file.delete(); + } + File[] children = file.listFiles(); + if (children == null) return true; + for (File f : children) { + if (!deleteRecursively(f)) return false; + } + return true; + } + + /** + * If the relative path denotes an absolute path, it is returned back. Otherwise it is returned + * the file relative to f For instance, `File.resolve(new File("/foo/bar"), "gav")` is `new + * File("/foo/bar/gav")` While `File.resolve(new File("/foo/bar"), "/gav")` is `new File("/gav")`. + * + * @return concatenated this and [relative] paths, or just [relative] if it's absolute. + */ + public static @Nullable File resolve(@Nullable File f, @Nullable String relative) { + if (f == null || relative == null) return null; + File relativeFile = new File(relative); + // If relative path is absolute we return the relative file directly + if (relative.length() > 0 && relative.charAt(0) == File.separatorChar) return relativeFile; + // Otherwise we return the file relative to the parent f + return new File(f, relative); + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 7733ebdcc34..35a46d985cb 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -341,6 +341,11 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan { public abstract fun setRequest (Lio/sentry/protocol/Request;)V } +public abstract interface class io/sentry/ITransactionListener { + public abstract fun onTransactionEnd (Lio/sentry/ITransaction;)V + public abstract fun onTransactionStart (Lio/sentry/ITransaction;)V +} + public abstract interface class io/sentry/ITransportFactory { public abstract fun create (Lio/sentry/SentryOptions;Lio/sentry/RequestDetails;)Lio/sentry/transport/ITransport; } @@ -479,6 +484,12 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { public fun traceState ()Lio/sentry/TraceState; } +public final class io/sentry/NoOpTransactionListener : io/sentry/ITransactionListener { + public static fun getInstance ()Lio/sentry/NoOpTransactionListener; + public fun onTransactionEnd (Lio/sentry/ITransaction;)V + public fun onTransactionStart (Lio/sentry/ITransaction;)V +} + public final class io/sentry/NoOpTransportFactory : io/sentry/ITransportFactory { public fun create (Lio/sentry/SentryOptions;Lio/sentry/RequestDetails;)Lio/sentry/transport/ITransport; public static fun getInstance ()Lio/sentry/NoOpTransportFactory; @@ -711,6 +722,7 @@ public final class io/sentry/SentryEnvelope { public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SdkVersion;Ljava/lang/Iterable;)V public static fun from (Lio/sentry/ISerializer;Lio/sentry/SentryBaseEvent;Lio/sentry/protocol/SdkVersion;)Lio/sentry/SentryEnvelope; public static fun from (Lio/sentry/ISerializer;Lio/sentry/Session;Lio/sentry/protocol/SdkVersion;)Lio/sentry/SentryEnvelope; + public static fun from (Lio/sentry/protocol/SentryId;Ljava/lang/String;Ljava/lang/String;Lio/sentry/protocol/SdkVersion;JZ)Lio/sentry/SentryEnvelope; public fun getHeader ()Lio/sentry/SentryEnvelopeHeader; public fun getItems ()Ljava/lang/Iterable; } @@ -737,6 +749,7 @@ public final class io/sentry/SentryEnvelopeItem { public static fun fromAttachment (Lio/sentry/Attachment;J)Lio/sentry/SentryEnvelopeItem; public static fun fromEvent (Lio/sentry/ISerializer;Lio/sentry/SentryBaseEvent;)Lio/sentry/SentryEnvelopeItem; public static fun fromSession (Lio/sentry/ISerializer;Lio/sentry/Session;)Lio/sentry/SentryEnvelopeItem; + public static fun fromTraceFile (Ljava/lang/String;Ljava/lang/String;JZ)Lio/sentry/SentryEnvelopeItem; public static fun fromUserFeedback (Lio/sentry/ISerializer;Lio/sentry/UserFeedback;)Lio/sentry/SentryEnvelopeItem; public fun getData ()[B public fun getEvent (Lio/sentry/ISerializer;)Lio/sentry/SentryEvent; @@ -794,7 +807,9 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/ public final class io/sentry/SentryItemType : java/lang/Enum { public static final field Attachment Lio/sentry/SentryItemType; public static final field Event Lio/sentry/SentryItemType; + public static final field InteractionTrace Lio/sentry/SentryItemType; public static final field Session Lio/sentry/SentryItemType; + public static final field SessionTrace Lio/sentry/SentryItemType; public static final field Transaction Lio/sentry/SentryItemType; public static final field Unknown Lio/sentry/SentryItemType; public static final field UserFeedback Lio/sentry/SentryItemType; @@ -851,7 +866,10 @@ public class io/sentry/SentryOptions { public fun getMaxQueueSize ()I public fun getMaxRequestBodySize ()Lio/sentry/SentryOptions$RequestSize; public fun getMaxSpans ()I + public fun getMaxTraceFileSize ()J public fun getOutboxPath ()Ljava/lang/String; + public fun getProfilingTracesDirPath ()Ljava/lang/String; + public fun getProfilingTracesIntervalMs ()I public fun getProguardUuid ()Ljava/lang/String; public fun getProxy ()Lio/sentry/SentryOptions$Proxy; public fun getReadTimeoutMillis ()I @@ -868,6 +886,7 @@ public class io/sentry/SentryOptions { public fun getTracesSampleRate ()Ljava/lang/Double; public fun getTracesSampler ()Lio/sentry/SentryOptions$TracesSamplerCallback; public fun getTracingOrigins ()Ljava/util/List; + public fun getTransactionListener ()Lio/sentry/ITransactionListener; public fun getTransportFactory ()Lio/sentry/ITransportFactory; public fun getTransportGate ()Lio/sentry/transport/ITransportGate; public fun isAttachServerName ()Z @@ -882,6 +901,7 @@ public class io/sentry/SentryOptions { public fun isEnableSessionTracking ()Z public fun isEnableShutdownHook ()Z public fun isEnableUncaughtExceptionHandler ()Z + public fun isProfilingEnabled ()Z public fun isSendDefaultPii ()Z public fun isTraceSampling ()Z public fun isTracingEnabled ()Z @@ -918,6 +938,10 @@ public class io/sentry/SentryOptions { public fun setMaxQueueSize (I)V public fun setMaxRequestBodySize (Lio/sentry/SentryOptions$RequestSize;)V public fun setMaxSpans (I)V + public fun setMaxTraceFileSize (J)V + public fun setProfilingEnabled (Z)V + public fun setProfilingTracesDirPath (Ljava/lang/String;)V + public fun setProfilingTracesIntervalMs (I)V public fun setProguardUuid (Ljava/lang/String;)V public fun setProxy (Lio/sentry/SentryOptions$Proxy;)V public fun setReadTimeoutMillis (I)V @@ -935,6 +959,7 @@ public class io/sentry/SentryOptions { public fun setTraceSampling (Z)V public fun setTracesSampleRate (Ljava/lang/Double;)V public fun setTracesSampler (Lio/sentry/SentryOptions$TracesSamplerCallback;)V + public fun setTransactionListener (Lio/sentry/ITransactionListener;)V public fun setTransportFactory (Lio/sentry/ITransportFactory;)V public fun setTransportGate (Lio/sentry/transport/ITransportGate;)V } @@ -2063,6 +2088,10 @@ public final class io/sentry/util/Platform { public static fun isJvm ()Z } +public final class io/sentry/util/SentryExecutors { + public static final field tracingExecutor Ljava/util/concurrent/ScheduledExecutorService; +} + public final class io/sentry/util/StringUtils { public static fun byteCountToString (J)Ljava/lang/String; public static fun capitalize (Ljava/lang/String;)Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 2decd790fbd..1cb7eeb273c 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -55,7 +55,7 @@ private static void validateOptions(final @NotNull SentryOptions options) { Objects.requireNonNull(options, "SentryOptions is required."); if (options.getDsn() == null || options.getDsn().isEmpty()) { throw new IllegalArgumentException( - "Hub requires a DSN to be instantiated. Considering using the NoOpHub is no DSN is available."); + "Hub requires a DSN to be instantiated. Considering using the NoOpHub if no DSN is available."); } } @@ -658,6 +658,14 @@ public void flush(long timeoutMillis) { startTimestamp, waitForChildren, transactionFinishedCallback); + + // todo Should the listener be called only when a transaction is sampled? + // The listener is called only if the transaction exists, as the transaction will needs to + // stop it + if (options.isProfilingEnabled()) { + final ITransactionListener transactionListener = options.getTransactionListener(); + transactionListener.onTransactionStart(transaction); + } } if (bindToScope) { configureScope(scope -> scope.setTransaction(transaction)); diff --git a/sentry/src/main/java/io/sentry/ITransactionListener.java b/sentry/src/main/java/io/sentry/ITransactionListener.java new file mode 100644 index 00000000000..1623280d43a --- /dev/null +++ b/sentry/src/main/java/io/sentry/ITransactionListener.java @@ -0,0 +1,11 @@ +package io.sentry; + +import org.jetbrains.annotations.ApiStatus; + +/** Used for performing operations when a transaction is started or ended. */ +@ApiStatus.Internal +public interface ITransactionListener { + void onTransactionStart(ITransaction transaction); + + void onTransactionEnd(ITransaction transaction); +} diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionListener.java b/sentry/src/main/java/io/sentry/NoOpTransactionListener.java new file mode 100644 index 00000000000..38145525ed8 --- /dev/null +++ b/sentry/src/main/java/io/sentry/NoOpTransactionListener.java @@ -0,0 +1,18 @@ +package io.sentry; + +public final class NoOpTransactionListener implements ITransactionListener { + + private static final NoOpTransactionListener instance = new NoOpTransactionListener(); + + private NoOpTransactionListener() {} + + public static NoOpTransactionListener getInstance() { + return instance; + } + + @Override + public void onTransactionStart(ITransaction transaction) {} + + @Override + public void onTransactionEnd(ITransaction transaction) {} +} diff --git a/sentry/src/main/java/io/sentry/SentryEnvelope.java b/sentry/src/main/java/io/sentry/SentryEnvelope.java index 5a11ee54d18..275116b1099 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelope.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelope.java @@ -76,4 +76,22 @@ public SentryEnvelope( return new SentryEnvelope( event.getEventId(), sdkVersion, SentryEnvelopeItem.fromEvent(serializer, event)); } + + public static @NotNull SentryEnvelope from( + final @Nullable SentryId eventId, + final @NotNull String traceFilePath, + final @NotNull String traceFileName, + final @Nullable SdkVersion sdkVersion, + final long maxTraceSize, + final boolean isStartupTrace) + throws IOException { + Objects.requireNonNull(traceFilePath, "Trace file path is required."); + Objects.requireNonNull(traceFileName, "Trace file name is required."); + + return new SentryEnvelope( + eventId, + sdkVersion, + SentryEnvelopeItem.fromTraceFile( + traceFilePath, traceFileName, maxTraceSize, isStartupTrace)); + } } diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index d16c6881355..d43b96d6245 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -179,51 +179,8 @@ public static SentryEnvelopeItem fromAttachment( } return attachment.getBytes(); } else if (attachment.getPathname() != null) { - - try { - File file = new File(attachment.getPathname()); - - if (!file.isFile()) { - throw new SentryEnvelopeException( - String.format( - "Reading the attachment %s failed, because the file located at the path is not a file.", - attachment.getPathname())); - } - - if (!file.canRead()) { - throw new SentryEnvelopeException( - String.format( - "Reading the attachment %s failed, because can't read the file.", - attachment.getPathname())); - } - - if (file.length() > maxAttachmentSize) { - throw new SentryEnvelopeException( - String.format( - "Dropping attachment, because the size of the it located at " - + "'%s' with %d bytes is bigger than the maximum " - + "allowed attachment size of %d bytes.", - attachment.getPathname(), file.length(), maxAttachmentSize)); - } - - try (FileInputStream fileInputStream = - new FileInputStream(attachment.getPathname()); - BufferedInputStream inputStream = new BufferedInputStream(fileInputStream); - ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { - byte[] bytes = new byte[1024]; - int length; - int offset = 0; - while ((length = inputStream.read(bytes)) != -1) { - outputStream.write(bytes, offset, length); - } - return outputStream.toByteArray(); - } - } catch (IOException | SecurityException exception) { - throw new SentryEnvelopeException( - String.format("Reading the attachment %s failed.", attachment.getPathname())); - } + return readEnvelopePayloadFromFile(attachment.getPathname(), maxAttachmentSize); } - throw new SentryEnvelopeException( String.format( "Couldn't attach the attachment %s.\n" @@ -243,6 +200,76 @@ public static SentryEnvelopeItem fromAttachment( return new SentryEnvelopeItem(itemHeader, () -> cachedItem.getBytes()); } + public static SentryEnvelopeItem fromTraceFile( + final @NotNull String traceFilePath, + final @NotNull String traceFileName, + final long maxTraceSize, + final boolean isStartupTrace) { + + final CachedItem cachedItem = + new CachedItem( + () -> { + if (!traceFilePath.isEmpty()) { + return readEnvelopePayloadFromFile(traceFilePath, maxTraceSize); + } + throw new SentryEnvelopeException( + String.format( + "Couldn't attach the trace %s.\nPlease check the trace was written correctly", + traceFilePath)); + }); + + SentryEnvelopeItemHeader itemHeader = + new SentryEnvelopeItemHeader( + isStartupTrace ? SentryItemType.SessionTrace : SentryItemType.InteractionTrace, + () -> cachedItem.getBytes().length, + "application/json", + traceFileName); + + // Don't use method reference. This can cause issues on Android + return new SentryEnvelopeItem(itemHeader, () -> cachedItem.getBytes()); + } + + private static byte[] readEnvelopePayloadFromFile(String pathname, long maxFileLength) + throws SentryEnvelopeException { + try { + File file = new File(pathname); + + if (!file.isFile()) { + throw new SentryEnvelopeException( + String.format( + "Reading the item %s failed, because the file located at the path is not a file.", + pathname)); + } + + if (!file.canRead()) { + throw new SentryEnvelopeException( + String.format("Reading the item %s failed, because can't read the file.", pathname)); + } + + if (file.length() > maxFileLength) { + throw new SentryEnvelopeException( + String.format( + "Dropping item, because its size located at '%s' with %d bytes is bigger " + + "than the maximum allowed size of %d bytes.", + pathname, file.length(), maxFileLength)); + } + + try (FileInputStream fileInputStream = new FileInputStream(pathname); + BufferedInputStream inputStream = new BufferedInputStream(fileInputStream); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { + byte[] bytes = new byte[1024]; + int length; + int offset = 0; + while ((length = inputStream.read(bytes)) != -1) { + outputStream.write(bytes, offset, length); + } + return outputStream.toByteArray(); + } + } catch (IOException | SecurityException exception) { + throw new SentryEnvelopeException(String.format("Reading the item %s failed.", pathname)); + } + } + private static class CachedItem { private @Nullable byte[] bytes; private final @Nullable Callable dataFactory; diff --git a/sentry/src/main/java/io/sentry/SentryItemType.java b/sentry/src/main/java/io/sentry/SentryItemType.java index 3ae083588e0..a7b474b6c13 100644 --- a/sentry/src/main/java/io/sentry/SentryItemType.java +++ b/sentry/src/main/java/io/sentry/SentryItemType.java @@ -10,6 +10,8 @@ public enum SentryItemType { UserFeedback("user_report"), // Sentry backend still uses user_report Attachment("attachment"), Transaction("transaction"), + SessionTrace("profiling-sessions"), + InteractionTrace("profiling-traces"), Unknown("__unknown__"); // DataCategory.Unknown private final String itemType; diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 5c83ca633a1..f2915de9da9 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -284,6 +284,21 @@ public class SentryOptions { /** Controls if the `tracestate` header is attached to envelopes and HTTP client integrations. */ private boolean traceSampling; + /** The cache dir. path for caching profiling traces */ + private @Nullable String profilingTracesDirPath; + + /** Control if profiling is enabled or not for transactions */ + private boolean profilingEnabled = false; + + /** Max trace file size in bytes. */ + private long maxTraceFileSize = 5 * 1024 * 1024; + + /** Interval for profiling traces in milliseconds. Defaults to 300 times per second */ + private int profilingTracesIntervalMs = 1_000 / 300; + + /** Listener interface to perform operations when a transaction is started or ended */ + private @NotNull ITransactionListener transactionListener = NoOpTransactionListener.getInstance(); + /** * Contains a list of origins to which `sentry-trace` header should be sent in HTTP integrations. */ @@ -1471,6 +1486,96 @@ public void setTraceSampling(boolean traceSampling) { this.traceSampling = traceSampling; } + /** + * 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 maximum trace file size for each envelope item in bytes. + * + * @return the maximum attachment size in bytes. + */ + public long getMaxTraceFileSize() { + return maxTraceFileSize; + } + + /** + * Sets the max trace file size for each envelope item in bytes. Default is 5 Mb. + * + * @param maxTraceFileSize the max trace file size in bytes. + */ + public void setMaxTraceFileSize(long maxTraceFileSize) { + this.maxTraceFileSize = maxTraceFileSize; + } + + /** + * Returns the interval for profiling traces in milliseconds. + * + * @return the interval for profiling traces in milliseconds. + */ + public int getProfilingTracesIntervalMs() { + return profilingTracesIntervalMs; + } + + /** + * Sets the interval for profiling traces in milliseconds. + * + * @param profilingTracesIntervalMs - the interval for profiling traces in milliseconds. + */ + public void setProfilingTracesIntervalMs(final int profilingTracesIntervalMs) { + this.profilingTracesIntervalMs = profilingTracesIntervalMs; + } + + /** + * Returns the listener interface to perform operations when a transaction is started or ended. + * + * @return the listener interface to perform operations when a transaction is started or ended. + */ + public @NotNull ITransactionListener getTransactionListener() { + return transactionListener; + } + + /** + * Sets the listener interface to perform operations when a transaction is started or ended. + * + * @param transactionListener - the listener for operations when a transaction is started or ended + */ + public void setTransactionListener(final @NotNull ITransactionListener transactionListener) { + this.transactionListener = transactionListener; + } + + /** + * Returns if profiling is enabled for transactions. + * + * @return if profiling is enabled for transactions. + */ + public boolean isProfilingEnabled() { + return profilingEnabled; + } + + /** + * Sets whether profiling is enabled for transactions. + * + * @param profilingEnabled - whether profiling is enabled for transactions + */ + public void setProfilingEnabled(boolean profilingEnabled) { + this.profilingEnabled = profilingEnabled; + } + /** * Returns a list of origins to which `sentry-trace` header should be sent in HTTP integrations. * @@ -1631,6 +1736,13 @@ void merge(final @NotNull SentryOptions options) { if (options.getEnableDeduplication() != null) { setEnableDeduplication(options.getEnableDeduplication()); } + if (options.getProfilingTracesDirPath() != null) { + setProfilingTracesDirPath(options.getProfilingTracesDirPath()); + } + setTransactionListener(options.getTransactionListener()); + setProfilingTracesIntervalMs(options.getProfilingTracesIntervalMs()); + setMaxTraceFileSize(options.getMaxTraceFileSize()); + setProfilingEnabled(options.isProfilingEnabled()); final Map tags = new HashMap<>(options.getTags()); for (final Map.Entry tag : tags.entrySet()) { this.tags.put(tag.getKey(), tag.getValue()); diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index eab380a3af3..2d82d7ba513 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -240,6 +240,8 @@ public void finish(@Nullable SpanStatus status) { if (transactionFinishedCallback != null) { transactionFinishedCallback.execute(this); } + if (hub.getOptions().isProfilingEnabled()) + hub.getOptions().getTransactionListener().onTransactionEnd(this); hub.captureTransaction(transaction, this.traceState()); } } diff --git a/sentry/src/main/java/io/sentry/transport/ReusableCountLatch.java b/sentry/src/main/java/io/sentry/transport/ReusableCountLatch.java index bb79cfced28..1193d3c88ff 100644 --- a/sentry/src/main/java/io/sentry/transport/ReusableCountLatch.java +++ b/sentry/src/main/java/io/sentry/transport/ReusableCountLatch.java @@ -16,9 +16,9 @@ *

A {@code ReusableCountLatch} is initialized with a given count. The {@link * #waitTillZero} methods block until the current count reaches zero due to invocations of the * {@link #decrement} method, after which all waiting threads are released. If zero has been reached - * any subsequent invocations of {@link #waitTillZero} return immediately. The coun cen be increased - * calling the {@link #increment()} method and any subsequent thread calling the {@link - * #waitTillZero} method will be block again until another zero is reached. + * any subsequent invocations of {@link #waitTillZero} return immediately. The count can be + * increased calling the {@link #increment()} method and any subsequent thread calling the {@link + * #waitTillZero} method will be blocked again until another zero is reached. * *

{@code ReusableCountLatch} provides more versatility than {@link * java.util.concurrent.CountDownLatch CountDownLatch} as the count doesn't have to be known upfront @@ -27,7 +27,7 @@ * instead can count up to 2_147_483_647 (2^31-1). * *

Great use case for {@code ReusableCountLatch} is when you wait for tasks on other threads to - * finish, but these tasks could trigger more tasks and it is not know upfront how many will be + * finish, but these tasks could trigger more tasks and it is not known upfront how many will be * triggered in total. * * @author mtymes diff --git a/sentry/src/main/java/io/sentry/util/SentryExecutors.java b/sentry/src/main/java/io/sentry/util/SentryExecutors.java new file mode 100644 index 00000000000..d2800ef440d --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/SentryExecutors.java @@ -0,0 +1,13 @@ +package io.sentry.util; + +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public final class SentryExecutors { + + private SentryExecutors() {} + + public static final ScheduledExecutorService tracingExecutor = new ScheduledThreadPoolExecutor(1); +} diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index a9b55da24ef..76902ae1d60 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -55,7 +55,7 @@ class HubTest { @Test fun `when no dsn available, ctor throws illegal arg`() { val ex = assertFailsWith { Hub(SentryOptions()) } - assertEquals("Hub requires a DSN to be instantiated. Considering using the NoOpHub is no DSN is available.", ex.message) + assertEquals("Hub requires a DSN to be instantiated. Considering using the NoOpHub if no DSN is available.", ex.message) } @Ignore("Sentry static class is registering integrations") diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index f25fa1a61f8..e13112a005f 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -88,6 +88,7 @@ class OutboxSenderTest { fixture.envelopeReader = EnvelopeReader() whenever(fixture.options.maxSpans).thenReturn(1000) whenever(fixture.hub.options).thenReturn(fixture.options) + whenever(fixture.options.transactionListener).thenReturn(NoOpTransactionListener.getInstance()) val transactionContext = TransactionContext("fixture-name", "http") transactionContext.description = "fixture-request" diff --git a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt index 57a263f9ce8..1edcc0e4435 100644 --- a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt +++ b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt @@ -197,9 +197,9 @@ class SentryEnvelopeItemTest { } assertEquals( - "Dropping attachment, because the size of the it located at " + + "Dropping item, because its size located at " + "'${fixture.pathname}' with ${file.length()} bytes is bigger than the maximum " + - "allowed attachment size of ${fixture.maxAttachmentSize} bytes.", + "allowed size of ${fixture.maxAttachmentSize} bytes.", exception.message ) } From 69e07de35277f3c877eb1b1d628e289c2804600d Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 3 Feb 2022 13:29:20 +0100 Subject: [PATCH 02/14] fixed few typos AndroidTraceTransactionListener now takes Hub in the constructor (for testability) --- .../api/sentry-android-core.api | 9 ++--- .../core/AndroidOptionsInitializer.java | 2 +- .../core/AndroidTraceTransactionListener.java | 36 ++++++++++--------- sentry/api/sentry.api | 10 ++++-- sentry/src/main/java/io/sentry/Sentry.java | 11 ++++++ .../java/io/sentry/SentryEnvelopeItem.java | 2 +- .../main/java/io/sentry/SentryOptions.java | 14 ++++---- .../main/java/io/sentry}/util/FileUtils.java | 2 +- 8 files changed, 50 insertions(+), 36 deletions(-) rename {sentry-android-core/src/main/java/io/sentry/android/core => sentry/src/main/java/io/sentry}/util/FileUtils.java (97%) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 234248f1308..097c61f8e88 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -13,7 +13,8 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android } public final class io/sentry/android/core/AndroidTraceTransactionListener : io/sentry/ITransactionListener { - public fun (Lio/sentry/SentryOptions;)V + public fun ()V + public fun (Lio/sentry/IHub;)V public fun onTransactionEnd (Lio/sentry/ITransaction;)V public fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -186,12 +187,6 @@ public final class io/sentry/android/core/util/DeviceOrientations { public static fun getOrientation (I)Lio/sentry/protocol/Device$DeviceOrientation; } -public final class io/sentry/android/core/util/FileUtils { - public fun ()V - public static fun deleteRecursively (Ljava/io/File;)Z - public static fun resolve (Ljava/io/File;Ljava/lang/String;)Ljava/io/File; -} - public final class io/sentry/android/core/util/MainThreadChecker { public static fun isMainThread ()Z public static fun isMainThread (Lio/sentry/protocol/SentryThread;)Z diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 32f3f7607ac..2625f9910e6 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -116,7 +116,7 @@ static void init( options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); - options.setTransactionListener(new AndroidTraceTransactionListener(options)); + options.setTransactionListener(new AndroidTraceTransactionListener()); } private static void installDefaultIntegrations( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java index 788f143296f..3077c5ec7cb 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java @@ -4,14 +4,15 @@ import android.os.Build; import android.os.Debug; +import io.sentry.HubAdapter; +import io.sentry.IHub; import io.sentry.ITransaction; import io.sentry.ITransactionListener; -import io.sentry.Sentry; import io.sentry.SentryEnvelope; import io.sentry.SentryLevel; import io.sentry.SentryOptions; -import io.sentry.android.core.util.FileUtils; import io.sentry.protocol.SentryId; +import io.sentry.util.FileUtils; import io.sentry.util.Objects; import io.sentry.util.SentryExecutors; import java.io.File; @@ -33,6 +34,8 @@ public final class AndroidTraceTransactionListener implements ITransactionListen */ private static final int BUFFER_SIZE_BYTES = 3_000_000; + private final IHub hub; + private @Nullable File traceFile = null; private @Nullable File traceFilesDir = null; private boolean startedMethodTracing = false; @@ -40,21 +43,21 @@ public final class AndroidTraceTransactionListener implements ITransactionListen private @NotNull final SentryOptions options; - public AndroidTraceTransactionListener(final @NotNull SentryOptions options) { - this.options = Objects.requireNonNull(options, "SentryOptions is required"); - final String tracesFilesDirPath = this.options.getProfilingTracesDirPath(); + public AndroidTraceTransactionListener() { + this(HubAdapter.getInstance()); + } + + public AndroidTraceTransactionListener(IHub hub) { + this.hub = Objects.requireNonNull(hub, "hub is required"); + options = hub.getOptions(); + final String tracesFilesDirPath = options.getProfilingTracesDirPath(); if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) { - this.options + options .getLogger() .log(SentryLevel.ERROR, "No profiling traces dir path is defined in options."); return; } - traceFilesDir = new File(tracesFilesDirPath); - // 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. - FileUtils.deleteRecursively(traceFilesDir); - traceFilesDir.mkdirs(); } @Override @@ -81,7 +84,7 @@ public synchronized void onTransactionStart(ITransaction transaction) { return; } - long intervalMs = options.getProfilingTracesIntervalMs(); + long intervalMs = options.getProfilingTracesIntervalMillis(); if (intervalMs <= 0) { options .getLogger() @@ -107,6 +110,7 @@ public synchronized void onTransactionEnd(ITransaction transaction) { if (startedMethodTracing) { startedMethodTracing = false; + activeTransaction = null; Debug.stopMethodTracing(); @@ -133,16 +137,14 @@ public synchronized void onTransactionEnd(ITransaction transaction) { options.getSdkVersion(), options.getMaxTraceFileSize(), true); - Sentry.getCurrentHub().captureEnvelope(envelope); + hub.captureEnvelope(envelope); } catch (IOException e) { - options.getLogger().log(SentryLevel.ERROR, "Failed to capture session.", e); - return; + options.getLogger().log(SentryLevel.ERROR, "Failed to capture the trace.", e); + // We don't return out of this function here, so we can go on and clean the variables } } if (traceFile != null) traceFile.delete(); - - activeTransaction = null; traceFile = null; } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 35a46d985cb..5b00d80ec70 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -869,7 +869,7 @@ public class io/sentry/SentryOptions { public fun getMaxTraceFileSize ()J public fun getOutboxPath ()Ljava/lang/String; public fun getProfilingTracesDirPath ()Ljava/lang/String; - public fun getProfilingTracesIntervalMs ()I + public fun getProfilingTracesIntervalMillis ()I public fun getProguardUuid ()Ljava/lang/String; public fun getProxy ()Lio/sentry/SentryOptions$Proxy; public fun getReadTimeoutMillis ()I @@ -941,7 +941,7 @@ public class io/sentry/SentryOptions { public fun setMaxTraceFileSize (J)V public fun setProfilingEnabled (Z)V public fun setProfilingTracesDirPath (Ljava/lang/String;)V - public fun setProfilingTracesIntervalMs (I)V + public fun setProfilingTracesIntervalMillis (I)V public fun setProguardUuid (Ljava/lang/String;)V public fun setProxy (Lio/sentry/SentryOptions$Proxy;)V public fun setReadTimeoutMillis (I)V @@ -2066,6 +2066,12 @@ public final class io/sentry/util/ExceptionUtils { public static fun findRootCause (Ljava/lang/Throwable;)Ljava/lang/Throwable; } +public final class io/sentry/util/FileUtils { + public fun ()V + public static fun deleteRecursively (Ljava/io/File;)Z + public static fun resolve (Ljava/io/File;Ljava/lang/String;)Ljava/io/File; +} + public final class io/sentry/util/LogUtils { public fun ()V public static fun logIfNotFlushable (Lio/sentry/ILogger;Ljava/lang/Object;)V diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index e4f1cb64e75..39ec19ecbb3 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -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; @@ -230,6 +231,16 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) options.setEnvelopeDiskCache(EnvelopeCache.create(options)); } + if (options.isProfilingEnabled() + && options.getProfilingTracesDirPath() != null + && !options.getProfilingTracesDirPath().isEmpty()) { + // 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. + final File traceFilesDir = new File(options.getProfilingTracesDirPath()); + FileUtils.deleteRecursively(traceFilesDir); + traceFilesDir.mkdirs(); + } + return true; } diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index d43b96d6245..02f71be801b 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -214,7 +214,7 @@ public static SentryEnvelopeItem fromTraceFile( } throw new SentryEnvelopeException( String.format( - "Couldn't attach the trace %s.\nPlease check the trace was written correctly", + "Couldn't attach the trace %s.\nPlease check if the trace was written correctly", traceFilePath)); }); diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index f2915de9da9..f0d6a23d2be 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -294,7 +294,7 @@ public class SentryOptions { private long maxTraceFileSize = 5 * 1024 * 1024; /** Interval for profiling traces in milliseconds. Defaults to 300 times per second */ - private int profilingTracesIntervalMs = 1_000 / 300; + private int profilingTracesIntervalMillis = 1_000 / 300; /** Listener interface to perform operations when a transaction is started or ended */ private @NotNull ITransactionListener transactionListener = NoOpTransactionListener.getInstance(); @@ -1527,17 +1527,17 @@ public void setMaxTraceFileSize(long maxTraceFileSize) { * * @return the interval for profiling traces in milliseconds. */ - public int getProfilingTracesIntervalMs() { - return profilingTracesIntervalMs; + public int getProfilingTracesIntervalMillis() { + return profilingTracesIntervalMillis; } /** * Sets the interval for profiling traces in milliseconds. * - * @param profilingTracesIntervalMs - the interval for profiling traces in milliseconds. + * @param profilingTracesIntervalMillis - the interval for profiling traces in milliseconds. */ - public void setProfilingTracesIntervalMs(final int profilingTracesIntervalMs) { - this.profilingTracesIntervalMs = profilingTracesIntervalMs; + public void setProfilingTracesIntervalMillis(final int profilingTracesIntervalMillis) { + this.profilingTracesIntervalMillis = profilingTracesIntervalMillis; } /** @@ -1740,7 +1740,7 @@ void merge(final @NotNull SentryOptions options) { setProfilingTracesDirPath(options.getProfilingTracesDirPath()); } setTransactionListener(options.getTransactionListener()); - setProfilingTracesIntervalMs(options.getProfilingTracesIntervalMs()); + setProfilingTracesIntervalMillis(options.getProfilingTracesIntervalMillis()); setMaxTraceFileSize(options.getMaxTraceFileSize()); setProfilingEnabled(options.isProfilingEnabled()); final Map tags = new HashMap<>(options.getTags()); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/util/FileUtils.java b/sentry/src/main/java/io/sentry/util/FileUtils.java similarity index 97% rename from sentry-android-core/src/main/java/io/sentry/android/core/util/FileUtils.java rename to sentry/src/main/java/io/sentry/util/FileUtils.java index 19a8c456611..3c45659326c 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/util/FileUtils.java +++ b/sentry/src/main/java/io/sentry/util/FileUtils.java @@ -1,4 +1,4 @@ -package io.sentry.android.core.util; +package io.sentry.util; import java.io.File; import org.jetbrains.annotations.ApiStatus; From 41613c79b79420c804a9391c53c6b536d5cd527a Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 9 Feb 2022 20:13:03 +0100 Subject: [PATCH 03/14] SentryExecutorService now uses a ScheduledExecutorService to allow the schedule() method ITransactionListener.onTransactionFinish() now returns a ProfilingTraceData, used to add the item to the transaction's envelope added io.sentry.traces.profiling-enable to manifest options profiling is now enabled by default --- .../api/sentry-android-core.api | 2 +- .../core/AndroidTraceTransactionListener.java | 137 +++++++++--------- .../android/core/ManifestMetadataReader.java | 5 + .../core/ActivityLifecycleIntegrationTest.kt | 14 +- .../core/ManifestMetadataReaderTest.kt | 25 ++++ sentry/api/sentry.api | 43 +++--- sentry/src/main/java/io/sentry/Hub.java | 10 +- .../src/main/java/io/sentry/HubAdapter.java | 6 +- sentry/src/main/java/io/sentry/IHub.java | 21 ++- .../main/java/io/sentry/ISentryClient.java | 23 ++- .../io/sentry/ISentryExecutorService.java | 7 +- .../java/io/sentry/ITransactionListener.java | 7 +- sentry/src/main/java/io/sentry/NoOpHub.java | 3 +- .../main/java/io/sentry/NoOpSentryClient.java | 3 +- .../io/sentry/NoOpSentryExecutorService.java | 5 + .../io/sentry/NoOpTransactionListener.java | 9 +- .../java/io/sentry/ProfilingTraceData.java | 35 +++++ .../src/main/java/io/sentry/SentryClient.java | 28 +++- .../main/java/io/sentry/SentryEnvelope.java | 18 --- .../java/io/sentry/SentryEnvelopeHeader.java | 3 + .../java/io/sentry/SentryEnvelopeItem.java | 39 +++-- .../java/io/sentry/SentryExecutorService.java | 13 +- .../main/java/io/sentry/SentryItemType.java | 3 +- .../main/java/io/sentry/SentryOptions.java | 10 +- .../src/main/java/io/sentry/SentryTracer.java | 8 +- .../main/java/io/sentry/util/FileUtils.java | 16 -- .../java/io/sentry/util/SentryExecutors.java | 13 -- sentry/src/test/java/io/sentry/HubTest.kt | 2 +- .../io/sentry/SentryExecutorServiceTest.kt | 14 +- .../test/java/io/sentry/SentryTracerTest.kt | 19 ++- 30 files changed, 333 insertions(+), 208 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/ProfilingTraceData.java delete mode 100644 sentry/src/main/java/io/sentry/util/SentryExecutors.java diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 097c61f8e88..6027f8279d5 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -15,7 +15,7 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android public final class io/sentry/android/core/AndroidTraceTransactionListener : io/sentry/ITransactionListener { public fun ()V public fun (Lio/sentry/IHub;)V - public fun onTransactionEnd (Lio/sentry/ITransaction;)V + public fun onTransactionFinish (Lio/sentry/ITransaction;)Lio/sentry/ProfilingTraceData; public fun onTransactionStart (Lio/sentry/ITransaction;)V } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java index 3077c5ec7cb..184b0dfa5a3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java @@ -8,16 +8,12 @@ import io.sentry.IHub; import io.sentry.ITransaction; import io.sentry.ITransactionListener; -import io.sentry.SentryEnvelope; +import io.sentry.ProfilingTraceData; import io.sentry.SentryLevel; -import io.sentry.SentryOptions; -import io.sentry.protocol.SentryId; -import io.sentry.util.FileUtils; import io.sentry.util.Objects; -import io.sentry.util.SentryExecutors; import java.io.File; -import java.io.IOException; import java.util.UUID; +import java.util.concurrent.Future; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -34,25 +30,22 @@ public final class AndroidTraceTransactionListener implements ITransactionListen */ private static final int BUFFER_SIZE_BYTES = 3_000_000; - private final IHub hub; - + private final @NotNull IHub hub; private @Nullable File traceFile = null; private @Nullable File traceFilesDir = null; - private boolean startedMethodTracing = false; - private @Nullable ITransaction activeTransaction = null; - - private @NotNull final SentryOptions options; + private @Nullable Future scheduledFinish = null; + private volatile @Nullable ITransaction activeTransaction = null; + private volatile @Nullable ProfilingTraceData lastProfilingData = null; public AndroidTraceTransactionListener() { this(HubAdapter.getInstance()); } - public AndroidTraceTransactionListener(IHub hub) { + public AndroidTraceTransactionListener(final @NotNull IHub hub) { this.hub = Objects.requireNonNull(hub, "hub is required"); - options = hub.getOptions(); - final String tracesFilesDirPath = options.getProfilingTracesDirPath(); + final String tracesFilesDirPath = hub.getOptions().getProfilingTracesDirPath(); if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) { - options + hub.getOptions() .getLogger() .log(SentryLevel.ERROR, "No profiling traces dir path is defined in options."); return; @@ -62,89 +55,95 @@ public AndroidTraceTransactionListener(IHub hub) { @Override @SuppressWarnings("FutureReturnValueIgnored") - public synchronized void onTransactionStart(ITransaction transaction) { + public synchronized void onTransactionStart(@NotNull ITransaction transaction) { // Debug.startMethodTracingSampling() is only available since Lollipop if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) return; - // Let's be sure to end any running trace - if (activeTransaction != null) onTransactionEnd(activeTransaction); + // If a transaction is currently being profiled, we ignore this call + if (activeTransaction != null) { + return; + } - traceFile = FileUtils.resolve(traceFilesDir, "sentry-" + UUID.randomUUID() + ".trace"); + traceFile = + traceFilesDir == null ? null : new File(traceFilesDir, UUID.randomUUID() + ".trace"); if (traceFile == null) { - options.getLogger().log(SentryLevel.DEBUG, "Could not create a trace file"); + hub.getOptions().getLogger().log(SentryLevel.DEBUG, "Could not create a trace file"); return; } if (traceFile.exists()) { - options + hub.getOptions() .getLogger() .log(SentryLevel.DEBUG, "Trace file already exists: %s", traceFile.getPath()); return; } - long intervalMs = options.getProfilingTracesIntervalMillis(); - if (intervalMs <= 0) { - options + long intervalMillis = hub.getOptions().getProfilingTracesIntervalMillis(); + if (intervalMillis <= 0) { + hub.getOptions() .getLogger() - .log(SentryLevel.DEBUG, "Profiling trace interval is set to %d milliseconds", intervalMs); + .log( + SentryLevel.DEBUG, + "Profiling trace interval is set to %d milliseconds", + intervalMillis); return; } + activeTransaction = transaction; // We stop the trace after 30 seconds, since such a long trace is very probably a trace // that will never end due to an error - SentryExecutors.tracingExecutor.schedule( - () -> onTransactionEnd(transaction), 30_000, MILLISECONDS); + scheduledFinish = + hub.getOptions() + .getExecutorService() + .schedule(() -> lastProfilingData = onTransactionFinish(transaction), 30_000); - startedMethodTracing = true; - int intervalUs = (int) MILLISECONDS.toMicros(intervalMs); - activeTransaction = transaction; + int intervalUs = (int) MILLISECONDS.toMicros(intervalMillis); Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs); } @Override - public synchronized void onTransactionEnd(ITransaction transaction) { - // In case a previous timeout tries to end a newer transaction we simply ignore it - if (transaction != activeTransaction) return; - - if (startedMethodTracing) { - startedMethodTracing = false; - activeTransaction = null; - - Debug.stopMethodTracing(); - - if (traceFile == null || !traceFile.exists()) { - options - .getLogger() - .log( - SentryLevel.DEBUG, - "Trace file %s does not exists", - traceFile == null ? "null" : traceFile.getPath()); - return; + public synchronized @Nullable ProfilingTraceData onTransactionFinish( + @NotNull ITransaction transaction) { + + // Profiling finished, but we check if we cached last profiling data due to a timeout + if (activeTransaction == null) { + ProfilingTraceData profilingData = lastProfilingData; + // If the cached last profiling data refers to the transaction that started it we return it + // back, otherwise we would simply lose it + if (profilingData != null + && profilingData.getTraceId().equals(transaction.getSpanContext().getTraceId())) { + return profilingData; } + return null; + } - // todo should I use transaction.getEventId() instead of new SentryId()? - // Or should I add the transaction id as an header to the envelope? - // Or should I simply ignore the transaction entirely (wouldn't make any sense)? - // And how to check if a trace is from a startup? - try { - SentryEnvelope envelope = - SentryEnvelope.from( - new SentryId(), - traceFile.getPath(), - traceFile.getName(), - options.getSdkVersion(), - options.getMaxTraceFileSize(), - true); - hub.captureEnvelope(envelope); - } catch (IOException e) { - options.getLogger().log(SentryLevel.ERROR, "Failed to capture the trace.", e); - // We don't return out of this function here, so we can go on and clean the variables - } + if (activeTransaction != transaction) { + return null; + } + + Debug.stopMethodTracing(); + + lastProfilingData = null; + + if (scheduledFinish != null) { + scheduledFinish.cancel(true); + } + + if (traceFile == null || !traceFile.exists()) { + hub.getOptions() + .getLogger() + .log( + SentryLevel.DEBUG, + "Trace file %s does not exists", + traceFile == null ? "null" : traceFile.getPath()); + return null; } - if (traceFile != null) traceFile.delete(); - traceFile = null; + return new ProfilingTraceData( + traceFile, + transaction.getSpanContext().getTraceId(), + transaction.getSpanContext().getSpanId()); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index 4b987d57a70..e9141c6df1a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -53,6 +53,8 @@ final class ManifestMetadataReader { static final String TRACES_ACTIVITY_AUTO_FINISH_ENABLE = "io.sentry.traces.activity.auto-finish.enable"; + static final String TRACES_PROFILING_ENABLE = "io.sentry.traces.profiling-enable"; + @ApiStatus.Experimental static final String TRACE_SAMPLING = "io.sentry.traces.trace-sampling"; static final String TRACING_ORIGINS = "io.sentry.traces.tracing-origins"; @@ -209,6 +211,9 @@ static void applyMetadata( TRACES_ACTIVITY_AUTO_FINISH_ENABLE, options.isEnableActivityLifecycleTracingAutoFinish())); + options.setProfilingEnabled( + readBool(metadata, logger, TRACES_PROFILING_ENABLE, options.isProfilingEnabled())); + final List tracingOrigins = readList(metadata, logger, TRACING_ORIGINS); if (tracingOrigins != null) { for (final String tracingOrigin : tracingOrigins) { diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 6a5c88ef5aa..e8fdecb6d54 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -370,6 +370,7 @@ class ActivityLifecycleIntegrationTest { check { assertEquals(SpanStatus.OK, it.status) }, + anyOrNull(), anyOrNull() ) } @@ -391,6 +392,7 @@ class ActivityLifecycleIntegrationTest { check { assertEquals(SpanStatus.UNKNOWN_ERROR, it.status) }, + anyOrNull(), anyOrNull() ) } @@ -406,7 +408,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, fixture.bundle) sut.onActivityPostResumed(activity) - verify(fixture.hub, never()).captureTransaction(any(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), anyOrNull(), anyOrNull()) } @Test @@ -417,7 +419,7 @@ class ActivityLifecycleIntegrationTest { val activity = mock() sut.onActivityPostResumed(activity) - verify(fixture.hub, never()).captureTransaction(any(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), anyOrNull(), anyOrNull()) } @Test @@ -430,7 +432,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, fixture.bundle) sut.onActivityDestroyed(activity) - verify(fixture.hub).captureTransaction(any(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) } @Test @@ -499,7 +501,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(mock(), mock()) sut.onActivityCreated(mock(), fixture.bundle) - verify(fixture.hub).captureTransaction(any(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) } @Test @@ -512,7 +514,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, mock()) sut.onActivityResumed(activity) - verify(fixture.hub, never()).captureTransaction(any(), any()) + verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull()) } @Test @@ -539,7 +541,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, mock()) sut.onActivityResumed(activity) - verify(fixture.hub).captureTransaction(any(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index 718024c60e0..2cf570c9c68 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -689,6 +689,31 @@ class ManifestMetadataReaderTest { assertFalse(fixture.options.isTraceSampling) } + @Test + fun `applyMetadata reads enableTracesProfiling to options`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.TRACES_PROFILING_ENABLE to false) + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options) + + // Assert + assertFalse(fixture.options.isProfilingEnabled) + } + + @Test + fun `applyMetadata reads enableTracesProfiling to options and keeps default`() { + // Arrange + val context = fixture.getContext() + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options) + + // Assert + assertTrue(fixture.options.isProfilingEnabled) + } + @Test fun `applyMetadata reads tracingOrigins to options`() { // Arrange diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 5b00d80ec70..cfa4eb941f1 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -116,7 +116,7 @@ public final class io/sentry/Hub : io/sentry/IHub { public fun captureEvent (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; - public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; + public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Ljava/lang/Object;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public fun captureUserFeedback (Lio/sentry/UserFeedback;)V public fun clearBreadcrumbs ()V public fun clone ()Lio/sentry/IHub; @@ -156,7 +156,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public fun captureEvent (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; - public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; + public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Ljava/lang/Object;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public fun captureUserFeedback (Lio/sentry/UserFeedback;)V public fun clearBreadcrumbs ()V public fun clone ()Lio/sentry/IHub; @@ -214,7 +214,8 @@ public abstract interface class io/sentry/IHub { public fun captureMessage (Ljava/lang/String;)Lio/sentry/protocol/SentryId; public abstract fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;)Lio/sentry/protocol/SentryId; - public abstract fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; + public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; + public abstract fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Ljava/lang/Object;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public abstract fun captureUserFeedback (Lio/sentry/UserFeedback;)V public abstract fun clearBreadcrumbs ()V @@ -289,13 +290,20 @@ public abstract interface class io/sentry/ISentryClient { public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/Scope;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;)Lio/sentry/protocol/SentryId; - public abstract fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Lio/sentry/Scope;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; + public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Lio/sentry/Scope;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; + public abstract fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Lio/sentry/Scope;Ljava/lang/Object;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public abstract fun captureUserFeedback (Lio/sentry/UserFeedback;)V public abstract fun close ()V public abstract fun flush (J)V public abstract fun isEnabled ()Z } +public abstract interface class io/sentry/ISentryExecutorService { + public abstract fun close (J)V + public abstract fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; + public abstract fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; +} + public abstract interface class io/sentry/ISerializer { public abstract fun deserialize (Ljava/io/Reader;Ljava/lang/Class;)Ljava/lang/Object; public abstract fun deserializeEnvelope (Ljava/io/InputStream;)Lio/sentry/SentryEnvelope; @@ -342,7 +350,7 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan { } public abstract interface class io/sentry/ITransactionListener { - public abstract fun onTransactionEnd (Lio/sentry/ITransaction;)V + public abstract fun onTransactionFinish (Lio/sentry/ITransaction;)Lio/sentry/ProfilingTraceData; public abstract fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -380,7 +388,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public fun captureEvent (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; - public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; + public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Ljava/lang/Object;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public fun captureUserFeedback (Lio/sentry/UserFeedback;)V public fun clearBreadcrumbs ()V public fun clone ()Lio/sentry/IHub; @@ -486,7 +494,7 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { public final class io/sentry/NoOpTransactionListener : io/sentry/ITransactionListener { public static fun getInstance ()Lio/sentry/NoOpTransactionListener; - public fun onTransactionEnd (Lio/sentry/ITransaction;)V + public fun onTransactionFinish (Lio/sentry/ITransaction;)Lio/sentry/ProfilingTraceData; public fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -506,6 +514,13 @@ public final class io/sentry/OutboxSender : io/sentry/IEnvelopeSender { public fun processEnvelopeFile (Ljava/lang/String;Ljava/lang/Object;)V } +public final class io/sentry/ProfilingTraceData { + public fun (Ljava/io/File;Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;)V + public fun getSpanId ()Lio/sentry/SpanId; + public fun getTraceFile ()Ljava/io/File; + public fun getTraceId ()Lio/sentry/protocol/SentryId; +} + public final class io/sentry/RequestDetails { public fun (Ljava/lang/String;Ljava/util/Map;)V public fun getHeaders ()Ljava/util/Map; @@ -702,7 +717,7 @@ public final class io/sentry/SentryClient : io/sentry/ISentryClient { public fun captureEnvelope (Lio/sentry/SentryEnvelope;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Scope;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; public fun captureSession (Lio/sentry/Session;Ljava/lang/Object;)V - public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Lio/sentry/Scope;Ljava/lang/Object;)Lio/sentry/protocol/SentryId; + public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceState;Lio/sentry/Scope;Ljava/lang/Object;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public fun captureUserFeedback (Lio/sentry/UserFeedback;)V public fun close ()V public fun flush (J)V @@ -722,7 +737,6 @@ public final class io/sentry/SentryEnvelope { public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SdkVersion;Ljava/lang/Iterable;)V public static fun from (Lio/sentry/ISerializer;Lio/sentry/SentryBaseEvent;Lio/sentry/protocol/SdkVersion;)Lio/sentry/SentryEnvelope; public static fun from (Lio/sentry/ISerializer;Lio/sentry/Session;Lio/sentry/protocol/SdkVersion;)Lio/sentry/SentryEnvelope; - public static fun from (Lio/sentry/protocol/SentryId;Ljava/lang/String;Ljava/lang/String;Lio/sentry/protocol/SdkVersion;JZ)Lio/sentry/SentryEnvelope; public fun getHeader ()Lio/sentry/SentryEnvelopeHeader; public fun getItems ()Ljava/lang/Iterable; } @@ -748,8 +762,8 @@ public final class io/sentry/SentryEnvelopeHeaderAdapter : com/google/gson/TypeA public final class io/sentry/SentryEnvelopeItem { public static fun fromAttachment (Lio/sentry/Attachment;J)Lio/sentry/SentryEnvelopeItem; public static fun fromEvent (Lio/sentry/ISerializer;Lio/sentry/SentryBaseEvent;)Lio/sentry/SentryEnvelopeItem; + public static fun fromProfilingTrace (Lio/sentry/ProfilingTraceData;J)Lio/sentry/SentryEnvelopeItem; public static fun fromSession (Lio/sentry/ISerializer;Lio/sentry/Session;)Lio/sentry/SentryEnvelopeItem; - public static fun fromTraceFile (Ljava/lang/String;Ljava/lang/String;JZ)Lio/sentry/SentryEnvelopeItem; public static fun fromUserFeedback (Lio/sentry/ISerializer;Lio/sentry/UserFeedback;)Lio/sentry/SentryEnvelopeItem; public fun getData ()[B public fun getEvent (Lio/sentry/ISerializer;)Lio/sentry/SentryEvent; @@ -807,9 +821,8 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/ public final class io/sentry/SentryItemType : java/lang/Enum { public static final field Attachment Lio/sentry/SentryItemType; public static final field Event Lio/sentry/SentryItemType; - public static final field InteractionTrace Lio/sentry/SentryItemType; + public static final field ProfilingTrace Lio/sentry/SentryItemType; public static final field Session Lio/sentry/SentryItemType; - public static final field SessionTrace Lio/sentry/SentryItemType; public static final field Transaction Lio/sentry/SentryItemType; public static final field Unknown Lio/sentry/SentryItemType; public static final field UserFeedback Lio/sentry/SentryItemType; @@ -853,6 +866,7 @@ public class io/sentry/SentryOptions { public fun getEnvelopeReader ()Lio/sentry/IEnvelopeReader; public fun getEnvironment ()Ljava/lang/String; public fun getEventProcessors ()Ljava/util/List; + public fun getExecutorService ()Lio/sentry/ISentryExecutorService; public fun getFlushTimeoutMillis ()J public fun getHostnameVerifier ()Ljavax/net/ssl/HostnameVerifier; public fun getIgnoredExceptionsForType ()Ljava/util/Set; @@ -2069,7 +2083,6 @@ public final class io/sentry/util/ExceptionUtils { public final class io/sentry/util/FileUtils { public fun ()V public static fun deleteRecursively (Ljava/io/File;)Z - public static fun resolve (Ljava/io/File;Ljava/lang/String;)Ljava/io/File; } public final class io/sentry/util/LogUtils { @@ -2094,10 +2107,6 @@ public final class io/sentry/util/Platform { public static fun isJvm ()Z } -public final class io/sentry/util/SentryExecutors { - public static final field tracingExecutor Ljava/util/concurrent/ScheduledExecutorService; -} - public final class io/sentry/util/StringUtils { public static fun byteCountToString (J)Ljava/lang/String; public static fun capitalize (Ljava/lang/String;)Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 1cb7eeb273c..f876cddde3c 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -538,7 +538,8 @@ public void flush(long timeoutMillis) { public @NotNull SentryId captureTransaction( final @NotNull SentryTransaction transaction, final @Nullable TraceState traceState, - final @Nullable Object hint) { + final @Nullable Object hint, + final @Nullable ProfilingTraceData profilingTraceData) { Objects.requireNonNull(transaction, "transaction is required"); SentryId sentryId = SentryId.EMPTY_ID; @@ -569,7 +570,9 @@ public void flush(long timeoutMillis) { try { item = stack.peek(); sentryId = - item.getClient().captureTransaction(transaction, traceState, item.getScope(), hint); + item.getClient() + .captureTransaction( + transaction, traceState, item.getScope(), hint, profilingTraceData); } catch (Throwable e) { options .getLogger() @@ -659,10 +662,9 @@ public void flush(long timeoutMillis) { waitForChildren, transactionFinishedCallback); - // todo Should the listener be called only when a transaction is sampled? // The listener is called only if the transaction exists, as the transaction will needs to // stop it - if (options.isProfilingEnabled()) { + if (samplingDecision && options.isProfilingEnabled()) { final ITransactionListener transactionListener = options.getTransactionListener(); transactionListener.onTransactionStart(transaction); } diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index b0fd237fed4..004a89d83e5 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -160,8 +160,10 @@ public void flush(long timeoutMillis) { public @NotNull SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceState traceState, - @Nullable Object hint) { - return Sentry.getCurrentHub().captureTransaction(transaction, traceState, hint); + @Nullable Object hint, + @Nullable ProfilingTraceData profilingTraceData) { + return Sentry.getCurrentHub() + .captureTransaction(transaction, traceState, hint, profilingTraceData); } @Override diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index 78ba3751e13..a19f1d47a24 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -272,6 +272,7 @@ default void addBreadcrumb(@NotNull String message, @NotNull String category) { * @param transaction the transaction * @param traceState the trace state * @param hint the hint + * @param profilingTraceData the profiling trace data * @return transaction's id */ @ApiStatus.Internal @@ -279,7 +280,25 @@ default void addBreadcrumb(@NotNull String message, @NotNull String category) { SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceState traceState, - @Nullable Object hint); + @Nullable Object hint, + final @Nullable ProfilingTraceData profilingTraceData); + + /** + * Captures the transaction and enqueues it for sending to Sentry server. + * + * @param transaction the transaction + * @param traceState the trace state + * @param hint the hint + * @return transaction's id + */ + @ApiStatus.Internal + @NotNull + default SentryId captureTransaction( + @NotNull SentryTransaction transaction, + @Nullable TraceState traceState, + @Nullable Object hint) { + return captureTransaction(transaction, traceState, hint, null); + } @ApiStatus.Internal @NotNull diff --git a/sentry/src/main/java/io/sentry/ISentryClient.java b/sentry/src/main/java/io/sentry/ISentryClient.java index b8273d3f8e8..8c8d6dfda46 100644 --- a/sentry/src/main/java/io/sentry/ISentryClient.java +++ b/sentry/src/main/java/io/sentry/ISentryClient.java @@ -217,11 +217,32 @@ default SentryId captureTransaction( */ @NotNull @ApiStatus.Experimental + default SentryId captureTransaction( + @NotNull SentryTransaction transaction, + @Nullable TraceState traceState, + @Nullable Scope scope, + @Nullable Object hint) { + return captureTransaction(transaction, traceState, scope, hint, null); + } + + /** + * Captures a transaction. + * + * @param transaction the {@link ITransaction} to send + * @param traceState the trace state + * @param scope An optional scope to be applied to the event. + * @param hint SDK specific but provides high level information about the origin of the event + * @param profilingTraceData An optional profiling trace data captured during the transaction + * @return The Id (SentryId object) of the event + */ + @NotNull + @ApiStatus.Experimental SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceState traceState, @Nullable Scope scope, - @Nullable Object hint); + @Nullable Object hint, + @Nullable ProfilingTraceData profilingTraceData); /** * Captures a transaction without scope nor hint. diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index dad1f1d1b86..aeaa9a3bc82 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -1,10 +1,12 @@ package io.sentry; import java.util.concurrent.Future; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; /** Sentry Executor Service that sends cached events and envelopes on App. start. */ -interface ISentryExecutorService { +@ApiStatus.Internal +public interface ISentryExecutorService { /** * Submits a Runnable to the ThreadExecutor @@ -15,6 +17,9 @@ interface ISentryExecutorService { @NotNull Future submit(final @NotNull Runnable runnable); + @NotNull + Future schedule(final @NotNull Runnable runnable, final long delayMillis); + /** * Closes the ThreadExecutor and awaits for the timeout * diff --git a/sentry/src/main/java/io/sentry/ITransactionListener.java b/sentry/src/main/java/io/sentry/ITransactionListener.java index 1623280d43a..e4c3ef34d04 100644 --- a/sentry/src/main/java/io/sentry/ITransactionListener.java +++ b/sentry/src/main/java/io/sentry/ITransactionListener.java @@ -1,11 +1,14 @@ package io.sentry; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** Used for performing operations when a transaction is started or ended. */ @ApiStatus.Internal public interface ITransactionListener { - void onTransactionStart(ITransaction transaction); + void onTransactionStart(@NotNull ITransaction transaction); - void onTransactionEnd(ITransaction transaction); + @Nullable + ProfilingTraceData onTransactionFinish(@NotNull ITransaction transaction); } diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index 728e6f5b632..5ca237712ae 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -120,7 +120,8 @@ public void flush(long timeoutMillis) {} public @NotNull SentryId captureTransaction( final @NotNull SentryTransaction transaction, final @Nullable TraceState traceState, - final @Nullable Object hint) { + final @Nullable Object hint, + final @Nullable ProfilingTraceData profilingTraceData) { return SentryId.EMPTY_ID; } diff --git a/sentry/src/main/java/io/sentry/NoOpSentryClient.java b/sentry/src/main/java/io/sentry/NoOpSentryClient.java index aace30692b4..5b90e7b8754 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryClient.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryClient.java @@ -48,7 +48,8 @@ public SentryId captureEnvelope(@NotNull SentryEnvelope envelope, @Nullable Obje @NotNull SentryTransaction transaction, @Nullable TraceState traceState, @Nullable Scope scope, - @Nullable Object hint) { + @Nullable Object hint, + @Nullable ProfilingTraceData profilingTraceData) { return SentryId.EMPTY_ID; } } diff --git a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java index 1f2231de3ea..dfb323ec4e5 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java @@ -18,6 +18,11 @@ private NoOpSentryExecutorService() {} return new FutureTask<>(() -> null); } + @Override + public @NotNull Future schedule(@NotNull Runnable runnable, long delayMillis) { + return new FutureTask<>(() -> null); + } + @Override public void close(long timeoutMillis) {} } diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionListener.java b/sentry/src/main/java/io/sentry/NoOpTransactionListener.java index 38145525ed8..466004151ad 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionListener.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionListener.java @@ -1,5 +1,8 @@ package io.sentry; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + public final class NoOpTransactionListener implements ITransactionListener { private static final NoOpTransactionListener instance = new NoOpTransactionListener(); @@ -11,8 +14,10 @@ public static NoOpTransactionListener getInstance() { } @Override - public void onTransactionStart(ITransaction transaction) {} + public void onTransactionStart(@NotNull ITransaction transaction) {} @Override - public void onTransactionEnd(ITransaction transaction) {} + public @Nullable ProfilingTraceData onTransactionFinish(@NotNull ITransaction transaction) { + return null; + } } diff --git a/sentry/src/main/java/io/sentry/ProfilingTraceData.java b/sentry/src/main/java/io/sentry/ProfilingTraceData.java new file mode 100644 index 00000000000..02b2db058d5 --- /dev/null +++ b/sentry/src/main/java/io/sentry/ProfilingTraceData.java @@ -0,0 +1,35 @@ +package io.sentry; + +import io.sentry.protocol.SentryId; +import java.io.File; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +@ApiStatus.Internal +public final class ProfilingTraceData { + private final @NotNull File traceFile; + private final @NotNull SentryId traceId; + private final @NotNull SpanId spanId; + + public ProfilingTraceData( + @NotNull File traceFile, @NotNull SentryId traceId, @NotNull SpanId spanId) { + this.traceFile = traceFile; + this.traceId = traceId; + this.spanId = spanId; + } + + @NotNull + public File getTraceFile() { + return traceFile; + } + + @NotNull + public SentryId getTraceId() { + return traceId; + } + + @NotNull + public SpanId getSpanId() { + return spanId; + } +} diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index b7fb87f4906..51d35968a94 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.exception.SentryEnvelopeException; import io.sentry.hints.DiskFlushNotification; import io.sentry.protocol.Contexts; import io.sentry.protocol.SentryId; @@ -133,12 +134,12 @@ private boolean shouldApplyScopeData( ? scope.getTransaction().traceState() : null; final SentryEnvelope envelope = - buildEnvelope(event, getAttachmentsFromScope(scope), session, traceState); + buildEnvelope(event, getAttachmentsFromScope(scope), session, traceState, null); if (envelope != null) { transport.send(envelope, hint); } - } catch (IOException e) { + } catch (IOException | SentryEnvelopeException e) { options.getLogger().log(SentryLevel.WARNING, e, "Capturing event %s failed.", sentryId); // if there was an error capturing the event, we return an emptyId @@ -160,8 +161,9 @@ private boolean shouldApplyScopeData( final @Nullable SentryBaseEvent event, final @Nullable List attachments, final @Nullable Session session, - final @Nullable TraceState traceState) - throws IOException { + final @Nullable TraceState traceState, + final @Nullable ProfilingTraceData profilingTraceData) + throws IOException, SentryEnvelopeException { SentryId sentryId = null; final List envelopeItems = new ArrayList<>(); @@ -179,6 +181,12 @@ private boolean shouldApplyScopeData( envelopeItems.add(sessionItem); } + if (profilingTraceData != null) { + final SentryEnvelopeItem profilingTraceItem = + SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, options.getMaxTraceFileSize()); + envelopeItems.add(profilingTraceItem); + } + if (attachments != null) { for (final Attachment attachment : attachments) { final SentryEnvelopeItem attachmentItem = @@ -401,7 +409,8 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec @NotNull SentryTransaction transaction, @Nullable TraceState traceState, final @Nullable Scope scope, - final @Nullable Object hint) { + final @Nullable Object hint, + final @Nullable ProfilingTraceData profilingTraceData) { Objects.requireNonNull(transaction, "Transaction is required."); options @@ -437,13 +446,18 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec try { final SentryEnvelope envelope = buildEnvelope( - transaction, filterForTransaction(getAttachmentsFromScope(scope)), null, traceState); + transaction, + filterForTransaction(getAttachmentsFromScope(scope)), + null, + traceState, + profilingTraceData); + if (envelope != null) { transport.send(envelope, hint); } else { sentryId = SentryId.EMPTY_ID; } - } catch (IOException e) { + } catch (IOException | SentryEnvelopeException e) { options.getLogger().log(SentryLevel.WARNING, e, "Capturing transaction %s failed.", sentryId); // if there was an error capturing the event, we return an emptyId sentryId = SentryId.EMPTY_ID; diff --git a/sentry/src/main/java/io/sentry/SentryEnvelope.java b/sentry/src/main/java/io/sentry/SentryEnvelope.java index 275116b1099..5a11ee54d18 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelope.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelope.java @@ -76,22 +76,4 @@ public SentryEnvelope( return new SentryEnvelope( event.getEventId(), sdkVersion, SentryEnvelopeItem.fromEvent(serializer, event)); } - - public static @NotNull SentryEnvelope from( - final @Nullable SentryId eventId, - final @NotNull String traceFilePath, - final @NotNull String traceFileName, - final @Nullable SdkVersion sdkVersion, - final long maxTraceSize, - final boolean isStartupTrace) - throws IOException { - Objects.requireNonNull(traceFilePath, "Trace file path is required."); - Objects.requireNonNull(traceFileName, "Trace file name is required."); - - return new SentryEnvelope( - eventId, - sdkVersion, - SentryEnvelopeItem.fromTraceFile( - traceFilePath, traceFileName, maxTraceSize, isStartupTrace)); - } } diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeHeader.java b/sentry/src/main/java/io/sentry/SentryEnvelopeHeader.java index eb596505e47..15accf9b607 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeHeader.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeHeader.java @@ -15,6 +15,9 @@ public final class SentryEnvelopeHeader { private final @Nullable TraceState trace; + // todo Should I add a TraceProfilingData object here and in SentryEnvelopeHeaderAdapter to allow + // OutboxSender? + public SentryEnvelopeHeader( final @Nullable SentryId eventId, final @Nullable SdkVersion sdkVersion) { this(eventId, sdkVersion, null); diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 02f71be801b..16ce18ce97e 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -200,33 +200,30 @@ public static SentryEnvelopeItem fromAttachment( return new SentryEnvelopeItem(itemHeader, () -> cachedItem.getBytes()); } - public static SentryEnvelopeItem fromTraceFile( - final @NotNull String traceFilePath, - final @NotNull String traceFileName, - final long maxTraceSize, - final boolean isStartupTrace) { + public static @Nullable SentryEnvelopeItem fromProfilingTrace( + final @NotNull ProfilingTraceData profilingTraceData, final long maxTraceFileSize) + throws SentryEnvelopeException { - final CachedItem cachedItem = - new CachedItem( - () -> { - if (!traceFilePath.isEmpty()) { - return readEnvelopePayloadFromFile(traceFilePath, maxTraceSize); - } - throw new SentryEnvelopeException( - String.format( - "Couldn't attach the trace %s.\nPlease check if the trace was written correctly", - traceFilePath)); - }); + File traceFile = profilingTraceData.getTraceFile(); + if (traceFile == null || !traceFile.exists()) return null; + + final byte[] traceBytes = readEnvelopePayloadFromFile(traceFile.getPath(), maxTraceFileSize); SentryEnvelopeItemHeader itemHeader = new SentryEnvelopeItemHeader( - isStartupTrace ? SentryItemType.SessionTrace : SentryItemType.InteractionTrace, - () -> cachedItem.getBytes().length, - "application/json", - traceFileName); + SentryItemType.ProfilingTrace, + () -> traceBytes.length, + "octet-stream", + traceFile.getName()); + + // todo can i simply add 2 fields in SentryEnvelopeItemHeader (traceId and spanId)? + // profilingTraceData.getTraceId().toString(), + // profilingTraceData.getSpanId().toString()); + + traceFile.delete(); // Don't use method reference. This can cause issues on Android - return new SentryEnvelopeItem(itemHeader, () -> cachedItem.getBytes()); + return new SentryEnvelopeItem(itemHeader, traceBytes); } private static byte[] readEnvelopePayloadFromFile(String pathname, long maxFileLength) diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index dd2faa29ab2..d388dfd7261 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -1,23 +1,23 @@ package io.sentry; -import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.TestOnly; final class SentryExecutorService implements ISentryExecutorService { - private final @NotNull ExecutorService executorService; + private final @NotNull ScheduledExecutorService executorService; @TestOnly - SentryExecutorService(final @NotNull ExecutorService executorService) { + SentryExecutorService(final @NotNull ScheduledExecutorService executorService) { this.executorService = executorService; } SentryExecutorService() { - this(Executors.newSingleThreadExecutor()); + this(Executors.newSingleThreadScheduledExecutor()); } @Override @@ -25,6 +25,11 @@ final class SentryExecutorService implements ISentryExecutorService { return executorService.submit(runnable); } + @Override + public @NotNull Future schedule(final @NotNull Runnable runnable, final long delayMillis) { + return executorService.schedule(runnable, delayMillis, TimeUnit.MILLISECONDS); + } + @Override public void close(final long timeoutMillis) { synchronized (executorService) { diff --git a/sentry/src/main/java/io/sentry/SentryItemType.java b/sentry/src/main/java/io/sentry/SentryItemType.java index a7b474b6c13..0456a8328fa 100644 --- a/sentry/src/main/java/io/sentry/SentryItemType.java +++ b/sentry/src/main/java/io/sentry/SentryItemType.java @@ -10,8 +10,7 @@ public enum SentryItemType { UserFeedback("user_report"), // Sentry backend still uses user_report Attachment("attachment"), Transaction("transaction"), - SessionTrace("profiling-sessions"), - InteractionTrace("profiling-traces"), + ProfilingTrace("profiling_android_trace"), Unknown("__unknown__"); // DataCategory.Unknown private final String itemType; diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index f0d6a23d2be..1400f258235 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -288,7 +288,7 @@ public class SentryOptions { private @Nullable String profilingTracesDirPath; /** Control if profiling is enabled or not for transactions */ - private boolean profilingEnabled = false; + private boolean profilingEnabled = true; /** Max trace file size in bytes. */ private long maxTraceFileSize = 5 * 1024 * 1024; @@ -1090,8 +1090,9 @@ public void setEnableUncaughtExceptionHandler( * * @return the SentryExecutorService */ + @ApiStatus.Internal @NotNull - ISentryExecutorService getExecutorService() { + public ISentryExecutorService getExecutorService() { return executorService; } @@ -1554,8 +1555,9 @@ public void setProfilingTracesIntervalMillis(final int profilingTracesIntervalMi * * @param transactionListener - the listener for operations when a transaction is started or ended */ - public void setTransactionListener(final @NotNull ITransactionListener transactionListener) { - this.transactionListener = transactionListener; + public void setTransactionListener(final @Nullable ITransactionListener transactionListener) { + this.transactionListener = + transactionListener != null ? transactionListener : NoOpTransactionListener.getInstance(); } /** diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 2d82d7ba513..4a6d3dc7eea 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -207,6 +207,10 @@ public void finish() { public void finish(@Nullable SpanStatus status) { this.finishStatus = FinishStatus.finishing(status); if (!root.isFinished() && (!waitForChildren || hasAllChildrenFinished())) { + ProfilingTraceData profilingTraceData = null; + if (hub.getOptions().isProfilingEnabled()) { + profilingTraceData = hub.getOptions().getTransactionListener().onTransactionFinish(this); + } root.finish(finishStatus.spanStatus); // finish unfinished children @@ -240,9 +244,7 @@ public void finish(@Nullable SpanStatus status) { if (transactionFinishedCallback != null) { transactionFinishedCallback.execute(this); } - if (hub.getOptions().isProfilingEnabled()) - hub.getOptions().getTransactionListener().onTransactionEnd(this); - hub.captureTransaction(transaction, this.traceState()); + hub.captureTransaction(transaction, this.traceState(), profilingTraceData); } } diff --git a/sentry/src/main/java/io/sentry/util/FileUtils.java b/sentry/src/main/java/io/sentry/util/FileUtils.java index 3c45659326c..ec4feeb3277 100644 --- a/sentry/src/main/java/io/sentry/util/FileUtils.java +++ b/sentry/src/main/java/io/sentry/util/FileUtils.java @@ -29,20 +29,4 @@ public static boolean deleteRecursively(@Nullable File file) { } return true; } - - /** - * If the relative path denotes an absolute path, it is returned back. Otherwise it is returned - * the file relative to f For instance, `File.resolve(new File("/foo/bar"), "gav")` is `new - * File("/foo/bar/gav")` While `File.resolve(new File("/foo/bar"), "/gav")` is `new File("/gav")`. - * - * @return concatenated this and [relative] paths, or just [relative] if it's absolute. - */ - public static @Nullable File resolve(@Nullable File f, @Nullable String relative) { - if (f == null || relative == null) return null; - File relativeFile = new File(relative); - // If relative path is absolute we return the relative file directly - if (relative.length() > 0 && relative.charAt(0) == File.separatorChar) return relativeFile; - // Otherwise we return the file relative to the parent f - return new File(f, relative); - } } diff --git a/sentry/src/main/java/io/sentry/util/SentryExecutors.java b/sentry/src/main/java/io/sentry/util/SentryExecutors.java deleted file mode 100644 index d2800ef440d..00000000000 --- a/sentry/src/main/java/io/sentry/util/SentryExecutors.java +++ /dev/null @@ -1,13 +0,0 @@ -package io.sentry.util; - -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import org.jetbrains.annotations.ApiStatus; - -@ApiStatus.Internal -public final class SentryExecutors { - - private SentryExecutors() {} - - public static final ScheduledExecutorService tracingExecutor = new ScheduledThreadPoolExecutor(1); -} diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index 76902ae1d60..2e332dc597d 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -1099,7 +1099,7 @@ class HubTest { val sentryTracer = SentryTracer(TransactionContext("name", "op", true), sut) sentryTracer.finish() val traceState = sentryTracer.traceState() - verify(mockClient).captureTransaction(any(), eq(traceState), any(), eq(null)) + verify(mockClient).captureTransaction(any(), eq(traceState), any(), eq(null), eq(null)) } @Test diff --git a/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt b/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt index 64d2ad7d376..6f65d505083 100644 --- a/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt +++ b/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt @@ -6,8 +6,8 @@ import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import org.awaitility.kotlin.await -import java.util.concurrent.ExecutorService import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.Test import kotlin.test.assertTrue @@ -16,7 +16,7 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards submit call to ExecutorService`() { - val executor = mock() + val executor = mock() val sentryExecutor = SentryExecutorService(executor) sentryExecutor.submit {} verify(executor).submit(any()) @@ -24,7 +24,7 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards close call to ExecutorService`() { - val executor = mock() + val executor = mock() val sentryExecutor = SentryExecutorService(executor) whenever(executor.isShutdown).thenReturn(false) whenever(executor.awaitTermination(any(), any())).thenReturn(true) @@ -34,7 +34,7 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards close and call shutdownNow if not enough time`() { - val executor = mock() + val executor = mock() val sentryExecutor = SentryExecutorService(executor) whenever(executor.isShutdown).thenReturn(false) whenever(executor.awaitTermination(any(), any())).thenReturn(false) @@ -44,7 +44,7 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards close and call shutdownNow if await throws`() { - val executor = mock() + val executor = mock() val sentryExecutor = SentryExecutorService(executor) whenever(executor.isShutdown).thenReturn(false) whenever(executor.awaitTermination(any(), any())).thenThrow(InterruptedException()) @@ -54,7 +54,7 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards close but do not shutdown if its already closed`() { - val executor = mock() + val executor = mock() val sentryExecutor = SentryExecutorService(executor) whenever(executor.isShutdown).thenReturn(true) sentryExecutor.close(15000) @@ -63,7 +63,7 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards close call to ExecutorService and close it`() { - val executor = Executors.newSingleThreadExecutor() + val executor = Executors.newSingleThreadScheduledExecutor() val sentryExecutor = SentryExecutorService(executor) sentryExecutor.close(15000) assertTrue(executor.isShutdown) diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index eca59215c7f..286ad15890d 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -121,6 +121,7 @@ class SentryTracerTest { check { assertEquals(it.transaction, tracer.name) }, + anyOrNull(), anyOrNull() ) } @@ -153,6 +154,7 @@ class SentryTracerTest { check { assertEquals(request, it.request) }, + anyOrNull(), anyOrNull() ) } @@ -179,6 +181,7 @@ class SentryTracerTest { assertEquals(tracer.spanContext.sampled, it.sampled) } }, + anyOrNull(), anyOrNull() ) } @@ -203,6 +206,7 @@ class SentryTracerTest { assertEquals(tracer.spanContext.sampled, it.sampled) } }, + anyOrNull(), anyOrNull() ) } @@ -220,6 +224,7 @@ class SentryTracerTest { assertEquals(emptyMap(), it.tags) } }, + anyOrNull(), anyOrNull() ) } @@ -236,6 +241,7 @@ class SentryTracerTest { assertEquals(1, it.spans.size) assertEquals("op1", it.spans.first().op) }, + anyOrNull(), anyOrNull() ) } @@ -367,6 +373,7 @@ class SentryTracerTest { assertEquals(SpanStatus.OK, it.status) } }, + anyOrNull(), anyOrNull() ) @@ -409,7 +416,7 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true) transaction.startChild("op") transaction.finish() - verify(fixture.hub, never()).captureTransaction(any(), any()) + verify(fixture.hub, never()).captureTransaction(any(), any(), any()) } @Test @@ -418,7 +425,7 @@ class SentryTracerTest { val child = transaction.startChild("op") child.finish() transaction.finish() - verify(fixture.hub).captureTransaction(any(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) } @Test @@ -439,7 +446,7 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true) val child = transaction.startChild("op") child.finish() - verify(fixture.hub, never()).captureTransaction(any(), any()) + verify(fixture.hub, never()).captureTransaction(any(), any(), any()) } @Test @@ -447,12 +454,13 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true) val child = transaction.startChild("op") transaction.finish(SpanStatus.INVALID_ARGUMENT) - verify(fixture.hub, never()).captureTransaction(any(), any()) + verify(fixture.hub, never()).captureTransaction(any(), any(), any()) child.finish() verify(fixture.hub, times(1)).captureTransaction( check { assertEquals(SpanStatus.INVALID_ARGUMENT, it.status) }, + anyOrNull(), anyOrNull() ) } @@ -471,6 +479,7 @@ class SentryTracerTest { assertEquals(SpanStatus.DEADLINE_EXCEEDED, it.spans[0].status) assertEquals(SpanStatus.DEADLINE_EXCEEDED, it.spans[1].status) }, + anyOrNull(), anyOrNull() ) } @@ -539,6 +548,7 @@ class SentryTracerTest { check { assertEquals("val", it.getExtra("key")) }, + anyOrNull(), anyOrNull() ) } @@ -556,6 +566,7 @@ class SentryTracerTest { assertEquals("val", it["key"]) } }, + anyOrNull(), anyOrNull() ) } From f9dc9517a9247d5dbe97714f2db8e4e3296d50c9 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 21 Feb 2022 16:12:20 +0100 Subject: [PATCH 04/14] fixed profiling item type ("profile") moved profilingTracesDirPath and profilingTracesIntervalMillis to SentryAndroidOptions updated ProfilingTraceData to be the payload of the profiling envelope item --- .../api/sentry-android-core.api | 7 +- .../core/AndroidOptionsInitializer.java | 12 +- .../core/AndroidTraceTransactionListener.java | 102 ++++++----- .../android/core/SentryAndroidOptions.java | 43 +++++ sentry/api/sentry.api | 32 +++- sentry/src/main/java/io/sentry/Hub.java | 2 +- .../main/java/io/sentry/ISentryClient.java | 6 +- .../java/io/sentry/ProfilingTraceData.java | 163 ++++++++++++++++-- sentry/src/main/java/io/sentry/Sentry.java | 11 -- .../src/main/java/io/sentry/SentryClient.java | 3 +- .../java/io/sentry/SentryEnvelopeHeader.java | 3 - .../java/io/sentry/SentryEnvelopeItem.java | 62 ++++--- .../main/java/io/sentry/SentryItemType.java | 2 +- .../main/java/io/sentry/SentryOptions.java | 46 ----- 14 files changed, 338 insertions(+), 156 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 6027f8279d5..a7908a20dcd 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -13,8 +13,7 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android } public final class io/sentry/android/core/AndroidTraceTransactionListener : io/sentry/ITransactionListener { - public fun ()V - public fun (Lio/sentry/IHub;)V + public fun (Lio/sentry/android/core/SentryAndroidOptions;)V public fun onTransactionFinish (Lio/sentry/ITransaction;)Lio/sentry/ProfilingTraceData; public fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -104,6 +103,8 @@ 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 public fun isEnableActivityLifecycleBreadcrumbs ()Z @@ -122,6 +123,8 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setEnableAppLifecycleBreadcrumbs (Z)V public fun setEnableAutoActivityLifecycleTracing (Z)V public fun setEnableSystemEventBreadcrumbs (Z)V + public fun setProfilingTracesDirPath (Ljava/lang/String;)V + public fun setProfilingTracesIntervalMillis (I)V } public final class io/sentry/android/core/SentryInitProvider : android/content/ContentProvider { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 2625f9910e6..f4963ff24cd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -13,6 +13,7 @@ import io.sentry.SendFireAndForgetOutboxSender; import io.sentry.SentryLevel; import io.sentry.SentryOptions; +import io.sentry.util.FileUtils; import io.sentry.util.Objects; import java.io.BufferedInputStream; import java.io.File; @@ -116,7 +117,7 @@ static void init( options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); - options.setTransactionListener(new AndroidTraceTransactionListener()); + options.setTransactionListener(new AndroidTraceTransactionListener(options)); } private static void installDefaultIntegrations( @@ -248,11 +249,18 @@ private static void readDefaultOptionValues( * @param options the SentryOptions */ private static void initializeCacheDirs( - final @NotNull Context context, final @NotNull SentryOptions options) { + 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()) { + // 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. + FileUtils.deleteRecursively(profilingTracesDir); + profilingTracesDir.mkdirs(); + } } private static boolean isNdkAvailable(final @NotNull IBuildInfoProvider buildInfoProvider) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java index 184b0dfa5a3..ffb1293424a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java @@ -4,8 +4,6 @@ import android.os.Build; import android.os.Debug; -import io.sentry.HubAdapter; -import io.sentry.IHub; import io.sentry.ITransaction; import io.sentry.ITransactionListener; import io.sentry.ProfilingTraceData; @@ -30,76 +28,79 @@ public final class AndroidTraceTransactionListener implements ITransactionListen */ private static final int BUFFER_SIZE_BYTES = 3_000_000; - private final @NotNull IHub hub; + private int intervalUs; private @Nullable File traceFile = null; private @Nullable File traceFilesDir = null; private @Nullable Future scheduledFinish = null; private volatile @Nullable ITransaction activeTransaction = null; private volatile @Nullable ProfilingTraceData lastProfilingData = null; + private final @NotNull SentryAndroidOptions options; - public AndroidTraceTransactionListener() { - this(HubAdapter.getInstance()); - } - - public AndroidTraceTransactionListener(final @NotNull IHub hub) { - this.hub = Objects.requireNonNull(hub, "hub is required"); - final String tracesFilesDirPath = hub.getOptions().getProfilingTracesDirPath(); + public AndroidTraceTransactionListener(final @NotNull SentryAndroidOptions sentryAndroidOptions) { + this.options = Objects.requireNonNull(sentryAndroidOptions, "SentryAndroidOptions is required"); + final String tracesFilesDirPath = options.getProfilingTracesDirPath(); if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) { - hub.getOptions() + options + .getLogger() + .log( + SentryLevel.WARNING, + "Disabling profiling because no profiling traces dir path is defined in options."); + return; + } + long intervalMillis = options.getProfilingTracesIntervalMillis(); + if (intervalMillis <= 0) { + options .getLogger() - .log(SentryLevel.ERROR, "No profiling traces dir path is defined in options."); + .log( + SentryLevel.WARNING, + "Disabling profiling because trace interval is set to %d milliseconds", + intervalMillis); return; } + intervalUs = (int) MILLISECONDS.toMicros(intervalMillis); traceFilesDir = new File(tracesFilesDirPath); } @Override - @SuppressWarnings("FutureReturnValueIgnored") public synchronized void onTransactionStart(@NotNull ITransaction transaction) { // Debug.startMethodTracingSampling() is only available since Lollipop if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) return; - // If a transaction is currently being profiled, we ignore this call - if (activeTransaction != null) { + // traceFilesDir is null or intervalUs is 0 only if there was a problem in the constructor, but + // we already logged that + if (traceFilesDir == null || intervalUs == 0) { return; } - traceFile = - traceFilesDir == null ? null : new File(traceFilesDir, UUID.randomUUID() + ".trace"); - - if (traceFile == null) { - hub.getOptions().getLogger().log(SentryLevel.DEBUG, "Could not create a trace file"); + // If a transaction is currently being profiled, we ignore this call + if (activeTransaction != null) { + options + .getLogger() + .log( + SentryLevel.WARNING, + "Profiling is already active and was started by transaction %s", + activeTransaction.getSpanContext().getTraceId().toString()); return; } + traceFile = new File(traceFilesDir, UUID.randomUUID() + ".trace"); + if (traceFile.exists()) { - hub.getOptions() + options .getLogger() .log(SentryLevel.DEBUG, "Trace file already exists: %s", traceFile.getPath()); return; } - - long intervalMillis = hub.getOptions().getProfilingTracesIntervalMillis(); - if (intervalMillis <= 0) { - hub.getOptions() - .getLogger() - .log( - SentryLevel.DEBUG, - "Profiling trace interval is set to %d milliseconds", - intervalMillis); - return; - } activeTransaction = transaction; // We stop the trace after 30 seconds, since such a long trace is very probably a trace // that will never end due to an error scheduledFinish = - hub.getOptions() + options .getExecutorService() .schedule(() -> lastProfilingData = onTransactionFinish(transaction), 30_000); - int intervalUs = (int) MILLISECONDS.toMicros(intervalMillis); Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs); } @@ -107,32 +108,43 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { public synchronized @Nullable ProfilingTraceData onTransactionFinish( @NotNull ITransaction transaction) { + final ITransaction finalActiveTransaction = activeTransaction; // Profiling finished, but we check if we cached last profiling data due to a timeout - if (activeTransaction == null) { + if (finalActiveTransaction == null) { ProfilingTraceData profilingData = lastProfilingData; // If the cached last profiling data refers to the transaction that started it we return it // back, otherwise we would simply lose it if (profilingData != null - && profilingData.getTraceId().equals(transaction.getSpanContext().getTraceId())) { + && profilingData + .getTraceId() + .equals(transaction.getSpanContext().getTraceId().toString())) { return profilingData; } return null; } - if (activeTransaction != transaction) { + if (finalActiveTransaction != transaction) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Transaction %s finished, but profiling was started by transaction %s. Skipping", + transaction.getSpanContext().getTraceId().toString(), + finalActiveTransaction.getSpanContext().getTraceId().toString()); return null; } Debug.stopMethodTracing(); lastProfilingData = null; + activeTransaction = null; if (scheduledFinish != null) { scheduledFinish.cancel(true); } if (traceFile == null || !traceFile.exists()) { - hub.getOptions() + options .getLogger() .log( SentryLevel.DEBUG, @@ -141,9 +153,19 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { return null; } + // todo how to retrieve these fields? (version name and code) + // buildNumber = Build.VERSION.INCREMENTAL? - osName = "android"? return new ProfilingTraceData( traceFile, - transaction.getSpanContext().getTraceId(), - transaction.getSpanContext().getSpanId()); + transaction, + Build.VERSION.SDK_INT, + Build.MANUFACTURER, + Build.MODEL, + "buildNumber", // ??? + "android", // ??? + Build.VERSION.RELEASE, + "versionName", // ??? + "versionCode", // ??? + options.getEnvironment()); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index f1862f6301f..f7ab1f10fe4 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -7,6 +7,7 @@ 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 { @@ -81,6 +82,12 @@ 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; + /** Interface that loads the debug images list */ private @NotNull IDebugImagesLoader debugImagesLoader = NoOpDebugImagesLoader.getInstance(); @@ -201,6 +208,42 @@ public void enableAllAutoBreadcrumbs(boolean enable) { enableAppLifecycleBreadcrumbs = 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. + * + * @return the interval for profiling traces in milliseconds. + */ + public int getProfilingTracesIntervalMillis() { + return profilingTracesIntervalMillis; + } + + /** + * Sets the interval for profiling traces in milliseconds. + * + * @param profilingTracesIntervalMillis - the interval for profiling traces in milliseconds. + */ + public void setProfilingTracesIntervalMillis(final int profilingTracesIntervalMillis) { + this.profilingTracesIntervalMillis = profilingTracesIntervalMillis; + } + /** * Returns the Debug image loader * diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index cfa4eb941f1..1f7e9ecbc46 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -515,10 +515,28 @@ public final class io/sentry/OutboxSender : io/sentry/IEnvelopeSender { } public final class io/sentry/ProfilingTraceData { - public fun (Ljava/io/File;Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;)V - public fun getSpanId ()Lio/sentry/SpanId; + public fun (Ljava/io/File;Lio/sentry/ITransaction;ILjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V + public fun getAndroid_api_level ()I + public fun getDevice_locale ()Ljava/lang/String; + public fun getDevice_manufacturer ()Ljava/lang/String; + public fun getDevice_model ()Ljava/lang/String; + public fun getDevice_os_build_number ()Ljava/lang/String; + public fun getDevice_os_name ()Ljava/lang/String; + public fun getDevice_os_version ()Ljava/lang/String; + public fun getEnvironment ()Ljava/lang/String; + public fun getError_code ()Ljava/lang/String; + public fun getError_description ()Ljava/lang/String; + public fun getPlatform ()Ljava/lang/String; + public fun getStacktrace ()Ljava/lang/String; + public fun getStacktrace_id ()Ljava/lang/String; public fun getTraceFile ()Ljava/io/File; - public fun getTraceId ()Lio/sentry/protocol/SentryId; + public fun getTraceId ()Ljava/lang/String; + public fun getTrace_id ()Ljava/lang/String; + public fun getTransaction_id ()Ljava/lang/String; + public fun getTransaction_name ()Ljava/lang/String; + public fun getVersion_code ()Ljava/lang/String; + public fun getVersion_name ()Ljava/lang/String; + public fun setStacktrace (Ljava/lang/String;)V } public final class io/sentry/RequestDetails { @@ -762,7 +780,7 @@ public final class io/sentry/SentryEnvelopeHeaderAdapter : com/google/gson/TypeA public final class io/sentry/SentryEnvelopeItem { public static fun fromAttachment (Lio/sentry/Attachment;J)Lio/sentry/SentryEnvelopeItem; public static fun fromEvent (Lio/sentry/ISerializer;Lio/sentry/SentryBaseEvent;)Lio/sentry/SentryEnvelopeItem; - public static fun fromProfilingTrace (Lio/sentry/ProfilingTraceData;J)Lio/sentry/SentryEnvelopeItem; + public static fun fromProfilingTrace (Lio/sentry/ProfilingTraceData;JLio/sentry/ISerializer;)Lio/sentry/SentryEnvelopeItem; public static fun fromSession (Lio/sentry/ISerializer;Lio/sentry/Session;)Lio/sentry/SentryEnvelopeItem; public static fun fromUserFeedback (Lio/sentry/ISerializer;Lio/sentry/UserFeedback;)Lio/sentry/SentryEnvelopeItem; public fun getData ()[B @@ -821,7 +839,7 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/ public final class io/sentry/SentryItemType : java/lang/Enum { public static final field Attachment Lio/sentry/SentryItemType; public static final field Event Lio/sentry/SentryItemType; - public static final field ProfilingTrace Lio/sentry/SentryItemType; + public static final field Profile Lio/sentry/SentryItemType; public static final field Session Lio/sentry/SentryItemType; public static final field Transaction Lio/sentry/SentryItemType; public static final field Unknown Lio/sentry/SentryItemType; @@ -882,8 +900,6 @@ 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 getProfilingTracesIntervalMillis ()I public fun getProguardUuid ()Ljava/lang/String; public fun getProxy ()Lio/sentry/SentryOptions$Proxy; public fun getReadTimeoutMillis ()I @@ -954,8 +970,6 @@ public class io/sentry/SentryOptions { public fun setMaxSpans (I)V public fun setMaxTraceFileSize (J)V public fun setProfilingEnabled (Z)V - public fun setProfilingTracesDirPath (Ljava/lang/String;)V - public fun setProfilingTracesIntervalMillis (I)V public fun setProguardUuid (Ljava/lang/String;)V public fun setProxy (Lio/sentry/SentryOptions$Proxy;)V public fun setReadTimeoutMillis (I)V diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index f876cddde3c..acd2859b813 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -662,7 +662,7 @@ public void flush(long timeoutMillis) { waitForChildren, transactionFinishedCallback); - // The listener is called only if the transaction exists, as the transaction will needs to + // The listener is called only if the transaction exists, as the transaction is needed to // stop it if (samplingDecision && options.isProfilingEnabled()) { final ITransactionListener transactionListener = options.getTransactionListener(); diff --git a/sentry/src/main/java/io/sentry/ISentryClient.java b/sentry/src/main/java/io/sentry/ISentryClient.java index 8c8d6dfda46..a69a808f3df 100644 --- a/sentry/src/main/java/io/sentry/ISentryClient.java +++ b/sentry/src/main/java/io/sentry/ISentryClient.java @@ -216,7 +216,7 @@ default SentryId captureTransaction( * @return The Id (SentryId object) of the event */ @NotNull - @ApiStatus.Experimental + @ApiStatus.Internal default SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceState traceState, @@ -236,7 +236,7 @@ default SentryId captureTransaction( * @return The Id (SentryId object) of the event */ @NotNull - @ApiStatus.Experimental + @ApiStatus.Internal SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceState traceState, @@ -251,7 +251,7 @@ SentryId captureTransaction( * @param traceState the trace state * @return The Id (SentryId object) of the event */ - @ApiStatus.Experimental + @ApiStatus.Internal default @NotNull SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceState traceState) { return captureTransaction(transaction, traceState, null, null); diff --git a/sentry/src/main/java/io/sentry/ProfilingTraceData.java b/sentry/src/main/java/io/sentry/ProfilingTraceData.java index 02b2db058d5..4a8bddbbb8d 100644 --- a/sentry/src/main/java/io/sentry/ProfilingTraceData.java +++ b/sentry/src/main/java/io/sentry/ProfilingTraceData.java @@ -1,35 +1,168 @@ package io.sentry; -import io.sentry.protocol.SentryId; import java.io.File; +import java.util.Locale; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; @ApiStatus.Internal public final class ProfilingTraceData { - private final @NotNull File traceFile; - private final @NotNull SentryId traceId; - private final @NotNull SpanId spanId; + + // This field is transient so that it's ignored by Gson + private final transient @NotNull File traceFile; + + // Device metadata + private final int android_api_level; + private final @NotNull String device_locale; + private final @NotNull String device_manufacturer; + private final @NotNull String device_model; + private final @NotNull String device_os_build_number; + private final @NotNull String device_os_name; + private final @NotNull String device_os_version; + private final @NotNull String platform; + + // Transaction info + private final @NotNull String error_code; + private final @NotNull String error_description; + private final @NotNull String transaction_name; + + // App info + private final @NotNull String version_name; + private final @NotNull String version_code; + + // Stacktrace context + private final @NotNull String transaction_id; + private final @NotNull String trace_id; + private final @NotNull String stacktrace_id; + private final @NotNull String environment; + + // Stacktrace (file) + /** Stacktrace encoded with Base64 */ + private @Nullable String stacktrace = null; public ProfilingTraceData( - @NotNull File traceFile, @NotNull SentryId traceId, @NotNull SpanId spanId) { + @NotNull File traceFile, + @NotNull ITransaction transaction, + int sdkInt, + @NotNull String deviceManufacturer, + @NotNull String deviceModel, + @NotNull String deviceOsBuildNumber, + @NotNull String deviceOsName, + @NotNull String deviceOsVersion, + @NotNull String versionName, + @NotNull String versionCode, + @Nullable String environment) { this.traceFile = traceFile; - this.traceId = traceId; - this.spanId = spanId; + + // Device metadata + this.android_api_level = sdkInt; + this.device_locale = Locale.getDefault().toString(); + this.device_manufacturer = deviceManufacturer; + this.device_model = deviceModel; + this.device_os_build_number = deviceOsBuildNumber; + this.device_os_name = deviceOsName; + this.device_os_version = deviceOsVersion; + this.platform = "android"; + + // Transaction info + this.error_code = transaction.getOperation(); + this.error_description = + transaction.getDescription() != null ? transaction.getDescription() : ""; + this.transaction_name = transaction.getName(); + + // App info + this.version_name = versionName; + this.version_code = versionCode; + + // Stacktrace context + this.transaction_id = transaction.getEventId().toString(); + this.trace_id = transaction.getSpanContext().getTraceId().toString(); + this.stacktrace_id = transaction.getSpanContext().getSpanId().toString(); + this.environment = environment != null ? environment : ""; } - @NotNull - public File getTraceFile() { + public @NotNull File getTraceFile() { return traceFile; } - @NotNull - public SentryId getTraceId() { - return traceId; + public @NotNull String getTraceId() { + return trace_id; + } + + public int getAndroid_api_level() { + return android_api_level; + } + + public @NotNull String getDevice_locale() { + return device_locale; + } + + public @NotNull String getDevice_manufacturer() { + return device_manufacturer; + } + + public @NotNull String getDevice_model() { + return device_model; + } + + public @NotNull String getDevice_os_build_number() { + return device_os_build_number; + } + + public @NotNull String getDevice_os_name() { + return device_os_name; + } + + public @NotNull String getDevice_os_version() { + return device_os_version; + } + + public @NotNull String getPlatform() { + return platform; + } + + public @NotNull String getError_code() { + return error_code; + } + + public @NotNull String getError_description() { + return error_description; + } + + public @NotNull String getTransaction_name() { + return transaction_name; + } + + public @NotNull String getVersion_name() { + return version_name; + } + + public @NotNull String getVersion_code() { + return version_code; + } + + public @NotNull String getTransaction_id() { + return transaction_id; + } + + public @NotNull String getTrace_id() { + return trace_id; + } + + public @NotNull String getStacktrace_id() { + return stacktrace_id; + } + + public @NotNull String getEnvironment() { + return environment; + } + + public @Nullable String getStacktrace() { + return stacktrace; } - @NotNull - public SpanId getSpanId() { - return spanId; + public void setStacktrace(@Nullable String stacktrace) { + this.stacktrace = stacktrace; } } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 39ec19ecbb3..e4f1cb64e75 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -4,7 +4,6 @@ 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; @@ -231,16 +230,6 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) options.setEnvelopeDiskCache(EnvelopeCache.create(options)); } - if (options.isProfilingEnabled() - && options.getProfilingTracesDirPath() != null - && !options.getProfilingTracesDirPath().isEmpty()) { - // 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. - final File traceFilesDir = new File(options.getProfilingTracesDirPath()); - FileUtils.deleteRecursively(traceFilesDir); - traceFilesDir.mkdirs(); - } - return true; } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 51d35968a94..d1c437472d0 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -183,7 +183,8 @@ private boolean shouldApplyScopeData( if (profilingTraceData != null) { final SentryEnvelopeItem profilingTraceItem = - SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, options.getMaxTraceFileSize()); + SentryEnvelopeItem.fromProfilingTrace( + profilingTraceData, options.getMaxTraceFileSize(), options.getSerializer()); envelopeItems.add(profilingTraceItem); } diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeHeader.java b/sentry/src/main/java/io/sentry/SentryEnvelopeHeader.java index 15accf9b607..eb596505e47 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeHeader.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeHeader.java @@ -15,9 +15,6 @@ public final class SentryEnvelopeHeader { private final @Nullable TraceState trace; - // todo Should I add a TraceProfilingData object here and in SentryEnvelopeHeaderAdapter to allow - // OutboxSender? - public SentryEnvelopeHeader( final @Nullable SentryId eventId, final @Nullable SdkVersion sdkVersion) { this(eventId, sdkVersion, null); diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 16ce18ce97e..80d16d266c5 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -1,8 +1,12 @@ package io.sentry; +import static io.sentry.vendor.Base64.NO_PADDING; +import static io.sentry.vendor.Base64.NO_WRAP; + import io.sentry.exception.SentryEnvelopeException; import io.sentry.protocol.SentryTransaction; import io.sentry.util.Objects; +import io.sentry.vendor.Base64; import java.io.BufferedInputStream; import java.io.BufferedReader; import java.io.BufferedWriter; @@ -179,7 +183,7 @@ public static SentryEnvelopeItem fromAttachment( } return attachment.getBytes(); } else if (attachment.getPathname() != null) { - return readEnvelopePayloadFromFile(attachment.getPathname(), maxAttachmentSize); + return readBytesFromFile(attachment.getPathname(), maxAttachmentSize); } throw new SentryEnvelopeException( String.format( @@ -201,32 +205,45 @@ public static SentryEnvelopeItem fromAttachment( } public static @Nullable SentryEnvelopeItem fromProfilingTrace( - final @NotNull ProfilingTraceData profilingTraceData, final long maxTraceFileSize) + final @NotNull ProfilingTraceData profilingTraceData, + final long maxTraceFileSize, + ISerializer serializer) throws SentryEnvelopeException { File traceFile = profilingTraceData.getTraceFile(); - if (traceFile == null || !traceFile.exists()) return null; - - final byte[] traceBytes = readEnvelopePayloadFromFile(traceFile.getPath(), maxTraceFileSize); - - SentryEnvelopeItemHeader itemHeader = - new SentryEnvelopeItemHeader( - SentryItemType.ProfilingTrace, - () -> traceBytes.length, - "octet-stream", - traceFile.getName()); - - // todo can i simply add 2 fields in SentryEnvelopeItemHeader (traceId and spanId)? - // profilingTraceData.getTraceId().toString(), - // profilingTraceData.getSpanId().toString()); - - traceFile.delete(); + if (!traceFile.exists()) { + return null; + } - // Don't use method reference. This can cause issues on Android - return new SentryEnvelopeItem(itemHeader, traceBytes); + // The payload of the profiling item is a json that includes the trace file encoded with base64 + byte[] traceFileBytes = readBytesFromFile(traceFile.getPath(), maxTraceFileSize); + String base64Trace = Base64.encodeToString(traceFileBytes, NO_WRAP | NO_PADDING); + profilingTraceData.setStacktrace(base64Trace); + + try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); + final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + serializer.serialize(profilingTraceData, writer); + byte[] payloadBytes = stream.toByteArray(); + + SentryEnvelopeItemHeader itemHeader = + new SentryEnvelopeItemHeader( + SentryItemType.Profile, + () -> payloadBytes.length, + "application-json", + traceFile.getName()); + + // Don't use method reference. This can cause issues on Android + return new SentryEnvelopeItem(itemHeader, payloadBytes); + } catch (IOException e) { + throw new SentryEnvelopeException( + String.format("Failed to serialize profiling trace data\n%s", e.getMessage())); + } finally { + // In any case we delete the trace file + traceFile.delete(); + } } - private static byte[] readEnvelopePayloadFromFile(String pathname, long maxFileLength) + private static byte[] readBytesFromFile(String pathname, long maxFileLength) throws SentryEnvelopeException { try { File file = new File(pathname); @@ -263,7 +280,8 @@ private static byte[] readEnvelopePayloadFromFile(String pathname, long maxFileL return outputStream.toByteArray(); } } catch (IOException | SecurityException exception) { - throw new SentryEnvelopeException(String.format("Reading the item %s failed.", pathname)); + throw new SentryEnvelopeException( + String.format("Reading the item %s failed.\n%s", pathname, exception.getMessage())); } } diff --git a/sentry/src/main/java/io/sentry/SentryItemType.java b/sentry/src/main/java/io/sentry/SentryItemType.java index 0456a8328fa..91dd33b12d0 100644 --- a/sentry/src/main/java/io/sentry/SentryItemType.java +++ b/sentry/src/main/java/io/sentry/SentryItemType.java @@ -10,7 +10,7 @@ public enum SentryItemType { UserFeedback("user_report"), // Sentry backend still uses user_report Attachment("attachment"), Transaction("transaction"), - ProfilingTrace("profiling_android_trace"), + Profile("profile"), Unknown("__unknown__"); // DataCategory.Unknown private final String itemType; diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 1400f258235..00548700d84 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -284,18 +284,12 @@ public class SentryOptions { /** Controls if the `tracestate` header is attached to envelopes and HTTP client integrations. */ private boolean traceSampling; - /** The cache dir. path for caching profiling traces */ - private @Nullable String profilingTracesDirPath; - /** Control if profiling is enabled or not for transactions */ private boolean profilingEnabled = true; /** Max trace file size in bytes. */ private long maxTraceFileSize = 5 * 1024 * 1024; - /** Interval for profiling traces in milliseconds. Defaults to 300 times per second */ - private int profilingTracesIntervalMillis = 1_000 / 300; - /** Listener interface to perform operations when a transaction is started or ended */ private @NotNull ITransactionListener transactionListener = NoOpTransactionListener.getInstance(); @@ -1487,24 +1481,6 @@ public void setTraceSampling(boolean traceSampling) { this.traceSampling = traceSampling; } - /** - * 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 maximum trace file size for each envelope item in bytes. * @@ -1523,24 +1499,6 @@ public void setMaxTraceFileSize(long maxTraceFileSize) { this.maxTraceFileSize = maxTraceFileSize; } - /** - * Returns the interval for profiling traces in milliseconds. - * - * @return the interval for profiling traces in milliseconds. - */ - public int getProfilingTracesIntervalMillis() { - return profilingTracesIntervalMillis; - } - - /** - * Sets the interval for profiling traces in milliseconds. - * - * @param profilingTracesIntervalMillis - the interval for profiling traces in milliseconds. - */ - public void setProfilingTracesIntervalMillis(final int profilingTracesIntervalMillis) { - this.profilingTracesIntervalMillis = profilingTracesIntervalMillis; - } - /** * Returns the listener interface to perform operations when a transaction is started or ended. * @@ -1738,11 +1696,7 @@ void merge(final @NotNull SentryOptions options) { if (options.getEnableDeduplication() != null) { setEnableDeduplication(options.getEnableDeduplication()); } - if (options.getProfilingTracesDirPath() != null) { - setProfilingTracesDirPath(options.getProfilingTracesDirPath()); - } setTransactionListener(options.getTransactionListener()); - setProfilingTracesIntervalMillis(options.getProfilingTracesIntervalMillis()); setMaxTraceFileSize(options.getMaxTraceFileSize()); setProfilingEnabled(options.isProfilingEnabled()); final Map tags = new HashMap<>(options.getTags()); From 9671886e51575a32bbe740b748ff11b4634ab325 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 3 Mar 2022 18:08:07 +0100 Subject: [PATCH 05/14] renamed ITransactionListener to ITransactionProfiler added some logs profiling is now disabled by default updated ProfilingTraceData --- .../api/sentry-android-core.api | 6 -- .../core/AndroidOptionsInitializer.java | 2 +- ...r.java => AndroidTransactionProfiler.java} | 61 +++++++++++++------ .../android/core/ManifestMetadataReader.java | 2 +- .../core/ActivityLifecycleIntegrationTest.kt | 8 ++- .../core/ManifestMetadataReaderTest.kt | 6 +- sentry/api/sentry.api | 15 +++-- sentry/src/main/java/io/sentry/Hub.java | 2 +- ...istener.java => ITransactionProfiler.java} | 2 +- ...ener.java => NoOpTransactionProfiler.java} | 8 +-- .../java/io/sentry/ProfilingTraceData.java | 45 ++++++-------- .../java/io/sentry/SentryEnvelopeItem.java | 2 +- .../main/java/io/sentry/SentryOptions.java | 10 +-- .../src/main/java/io/sentry/SentryTracer.java | 2 +- .../test/java/io/sentry/OutboxSenderTest.kt | 2 +- .../test/java/io/sentry/SentryTracerTest.kt | 13 +++- 16 files changed, 104 insertions(+), 82 deletions(-) rename sentry-android-core/src/main/java/io/sentry/android/core/{AndroidTraceTransactionListener.java => AndroidTransactionProfiler.java} (72%) rename sentry/src/main/java/io/sentry/{ITransactionListener.java => ITransactionProfiler.java} (90%) rename sentry/src/main/java/io/sentry/{NoOpTransactionListener.java => NoOpTransactionProfiler.java} (57%) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index a7908a20dcd..006feb5a0f1 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -12,12 +12,6 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/AndroidTraceTransactionListener : io/sentry/ITransactionListener { - public fun (Lio/sentry/android/core/SentryAndroidOptions;)V - public fun onTransactionFinish (Lio/sentry/ITransaction;)Lio/sentry/ProfilingTraceData; - public fun onTransactionStart (Lio/sentry/ITransaction;)V -} - public final class io/sentry/android/core/AnrIntegration : io/sentry/Integration, java/io/Closeable { public fun (Landroid/content/Context;)V public fun close ()V diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index f4963ff24cd..d7211ad8237 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -117,7 +117,7 @@ static void init( options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); - options.setTransactionListener(new AndroidTraceTransactionListener(options)); + options.setTransactionListener(new AndroidTransactionProfiler(options)); } private static void installDefaultIntegrations( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java similarity index 72% rename from sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java rename to sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index ffb1293424a..f083f2024f4 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTraceTransactionListener.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -5,7 +5,7 @@ import android.os.Build; import android.os.Debug; import io.sentry.ITransaction; -import io.sentry.ITransactionListener; +import io.sentry.ITransactionProfiler; import io.sentry.ProfilingTraceData; import io.sentry.SentryLevel; import io.sentry.util.Objects; @@ -15,7 +15,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -public final class AndroidTraceTransactionListener implements ITransactionListener { +final class AndroidTransactionProfiler implements ITransactionProfiler { /** * This appears to correspond to the buffer size of the data part of the file, excluding the key @@ -28,15 +28,17 @@ public final class AndroidTraceTransactionListener implements ITransactionListen */ private static final int BUFFER_SIZE_BYTES = 3_000_000; + private static final int PROFILING_TIMEOUT_MILLIS = 30_000; + private int intervalUs; private @Nullable File traceFile = null; private @Nullable File traceFilesDir = null; private @Nullable Future scheduledFinish = null; private volatile @Nullable ITransaction activeTransaction = null; - private volatile @Nullable ProfilingTraceData lastProfilingData = null; + private volatile @Nullable ProfilingTraceData timedOutProfilingData = null; private final @NotNull SentryAndroidOptions options; - public AndroidTraceTransactionListener(final @NotNull SentryAndroidOptions sentryAndroidOptions) { + public AndroidTransactionProfiler(final @NotNull SentryAndroidOptions sentryAndroidOptions) { this.options = Objects.requireNonNull(sentryAndroidOptions, "SentryAndroidOptions is required"); final String tracesFilesDirPath = options.getProfilingTracesDirPath(); if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) { @@ -99,7 +101,9 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { scheduledFinish = options .getExecutorService() - .schedule(() -> lastProfilingData = onTransactionFinish(transaction), 30_000); + .schedule( + () -> timedOutProfilingData = onTransactionFinish(transaction), + PROFILING_TIMEOUT_MILLIS); Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs); } @@ -109,17 +113,38 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { @NotNull ITransaction transaction) { final ITransaction finalActiveTransaction = activeTransaction; + final ProfilingTraceData profilingData = timedOutProfilingData; + // Profiling finished, but we check if we cached last profiling data due to a timeout if (finalActiveTransaction == null) { - ProfilingTraceData profilingData = lastProfilingData; - // If the cached last profiling data refers to the transaction that started it we return it - // back, otherwise we would simply lose it - if (profilingData != null - && profilingData - .getTraceId() - .equals(transaction.getSpanContext().getTraceId().toString())) { - return profilingData; + // If the cached timed out profiling data refers to the transaction that started it we return + // it back, otherwise we would simply lose it + if (profilingData != null) { + // The timed out transaction is finishing + if (profilingData + .getTraceId() + .equals(transaction.getSpanContext().getTraceId().toString())) { + timedOutProfilingData = null; + return profilingData; + } else { + // Another transaction is finishing before the timed out one + options + .getLogger() + .log( + SentryLevel.ERROR, + "Profiling data with id %s exists but doesn't match the closing transaction %s", + profilingData.getTraceId(), + transaction.getSpanContext().getTraceId().toString()); + return null; + } } + // A transaction is finishing, but profiling didn't start. Maybe it was started by another one + options + .getLogger() + .log( + SentryLevel.INFO, + "Transaction %s finished, but profiling never started for it. Skipping", + transaction.getSpanContext().getTraceId().toString()); return null; } @@ -136,34 +161,32 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { Debug.stopMethodTracing(); - lastProfilingData = null; activeTransaction = null; if (scheduledFinish != null) { scheduledFinish.cancel(true); + scheduledFinish = null; } if (traceFile == null || !traceFile.exists()) { options .getLogger() .log( - SentryLevel.DEBUG, + SentryLevel.ERROR, "Trace file %s does not exists", traceFile == null ? "null" : traceFile.getPath()); return null; } - // todo how to retrieve these fields? (version name and code) - // buildNumber = Build.VERSION.INCREMENTAL? - osName = "android"? + // todo how to retrieve version name and code? return new ProfilingTraceData( traceFile, transaction, Build.VERSION.SDK_INT, Build.MANUFACTURER, Build.MODEL, - "buildNumber", // ??? - "android", // ??? Build.VERSION.RELEASE, + options.getProguardUuid(), "versionName", // ??? "versionCode", // ??? options.getEnvironment()); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index e9141c6df1a..309d4c44eda 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -53,7 +53,7 @@ final class ManifestMetadataReader { static final String TRACES_ACTIVITY_AUTO_FINISH_ENABLE = "io.sentry.traces.activity.auto-finish.enable"; - static final String TRACES_PROFILING_ENABLE = "io.sentry.traces.profiling-enable"; + static final String TRACES_PROFILING_ENABLE = "io.sentry.traces.profiling.enable"; @ApiStatus.Experimental static final String TRACE_SAMPLING = "io.sentry.traces.trace-sampling"; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index e8fdecb6d54..20b039fdb3d 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -371,6 +371,7 @@ class ActivityLifecycleIntegrationTest { assertEquals(SpanStatus.OK, it.status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -393,6 +394,7 @@ class ActivityLifecycleIntegrationTest { assertEquals(SpanStatus.UNKNOWN_ERROR, it.status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -432,7 +434,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, fixture.bundle) sut.onActivityDestroyed(activity) - verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test @@ -501,7 +503,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(mock(), mock()) sut.onActivityCreated(mock(), fixture.bundle) - verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test @@ -541,7 +543,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, mock()) sut.onActivityResumed(activity) - verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index 2cf570c9c68..543a7b7d632 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -692,14 +692,14 @@ class ManifestMetadataReaderTest { @Test fun `applyMetadata reads enableTracesProfiling to options`() { // Arrange - val bundle = bundleOf(ManifestMetadataReader.TRACES_PROFILING_ENABLE to false) + val bundle = bundleOf(ManifestMetadataReader.TRACES_PROFILING_ENABLE to true) val context = fixture.getContext(metaData = bundle) // Act ManifestMetadataReader.applyMetadata(context, fixture.options) // Assert - assertFalse(fixture.options.isProfilingEnabled) + assertTrue(fixture.options.isProfilingEnabled) } @Test @@ -711,7 +711,7 @@ class ManifestMetadataReaderTest { ManifestMetadataReader.applyMetadata(context, fixture.options) // Assert - assertTrue(fixture.options.isProfilingEnabled) + assertFalse(fixture.options.isProfilingEnabled) } @Test diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 1f7e9ecbc46..fb4c2c4b35f 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -349,7 +349,7 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan { public abstract fun setRequest (Lio/sentry/protocol/Request;)V } -public abstract interface class io/sentry/ITransactionListener { +public abstract interface class io/sentry/ITransactionProfiler { public abstract fun onTransactionFinish (Lio/sentry/ITransaction;)Lio/sentry/ProfilingTraceData; public abstract fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -492,8 +492,8 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { public fun traceState ()Lio/sentry/TraceState; } -public final class io/sentry/NoOpTransactionListener : io/sentry/ITransactionListener { - public static fun getInstance ()Lio/sentry/NoOpTransactionListener; +public final class io/sentry/NoOpTransactionProfiler : io/sentry/ITransactionProfiler { + public static fun getInstance ()Lio/sentry/NoOpTransactionProfiler; public fun onTransactionFinish (Lio/sentry/ITransaction;)Lio/sentry/ProfilingTraceData; public fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -515,8 +515,9 @@ public final class io/sentry/OutboxSender : io/sentry/IEnvelopeSender { } public final class io/sentry/ProfilingTraceData { - public fun (Ljava/io/File;Lio/sentry/ITransaction;ILjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V + public fun (Ljava/io/File;Lio/sentry/ITransaction;ILjava/lang/String;Ljava/lang/String;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_locale ()Ljava/lang/String; public fun getDevice_manufacturer ()Ljava/lang/String; public fun getDevice_model ()Ljava/lang/String; @@ -524,8 +525,6 @@ public final class io/sentry/ProfilingTraceData { public fun getDevice_os_name ()Ljava/lang/String; public fun getDevice_os_version ()Ljava/lang/String; public fun getEnvironment ()Ljava/lang/String; - public fun getError_code ()Ljava/lang/String; - public fun getError_description ()Ljava/lang/String; public fun getPlatform ()Ljava/lang/String; public fun getStacktrace ()Ljava/lang/String; public fun getStacktrace_id ()Ljava/lang/String; @@ -916,7 +915,7 @@ public class io/sentry/SentryOptions { public fun getTracesSampleRate ()Ljava/lang/Double; public fun getTracesSampler ()Lio/sentry/SentryOptions$TracesSamplerCallback; public fun getTracingOrigins ()Ljava/util/List; - public fun getTransactionListener ()Lio/sentry/ITransactionListener; + public fun getTransactionListener ()Lio/sentry/ITransactionProfiler; public fun getTransportFactory ()Lio/sentry/ITransportFactory; public fun getTransportGate ()Lio/sentry/transport/ITransportGate; public fun isAttachServerName ()Z @@ -987,7 +986,7 @@ public class io/sentry/SentryOptions { public fun setTraceSampling (Z)V public fun setTracesSampleRate (Ljava/lang/Double;)V public fun setTracesSampler (Lio/sentry/SentryOptions$TracesSamplerCallback;)V - public fun setTransactionListener (Lio/sentry/ITransactionListener;)V + public fun setTransactionListener (Lio/sentry/ITransactionProfiler;)V public fun setTransportFactory (Lio/sentry/ITransportFactory;)V public fun setTransportGate (Lio/sentry/transport/ITransportGate;)V } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index acd2859b813..1d513ed19cd 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -665,7 +665,7 @@ public void flush(long timeoutMillis) { // The listener is called only if the transaction exists, as the transaction is needed to // stop it if (samplingDecision && options.isProfilingEnabled()) { - final ITransactionListener transactionListener = options.getTransactionListener(); + final ITransactionProfiler transactionListener = options.getTransactionListener(); transactionListener.onTransactionStart(transaction); } } diff --git a/sentry/src/main/java/io/sentry/ITransactionListener.java b/sentry/src/main/java/io/sentry/ITransactionProfiler.java similarity index 90% rename from sentry/src/main/java/io/sentry/ITransactionListener.java rename to sentry/src/main/java/io/sentry/ITransactionProfiler.java index e4c3ef34d04..6e6e9cf582c 100644 --- a/sentry/src/main/java/io/sentry/ITransactionListener.java +++ b/sentry/src/main/java/io/sentry/ITransactionProfiler.java @@ -6,7 +6,7 @@ /** Used for performing operations when a transaction is started or ended. */ @ApiStatus.Internal -public interface ITransactionListener { +public interface ITransactionProfiler { void onTransactionStart(@NotNull ITransaction transaction); @Nullable diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionListener.java b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java similarity index 57% rename from sentry/src/main/java/io/sentry/NoOpTransactionListener.java rename to sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java index 466004151ad..2580ded221d 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionListener.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java @@ -3,13 +3,13 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -public final class NoOpTransactionListener implements ITransactionListener { +public final class NoOpTransactionProfiler implements ITransactionProfiler { - private static final NoOpTransactionListener instance = new NoOpTransactionListener(); + private static final NoOpTransactionProfiler instance = new NoOpTransactionProfiler(); - private NoOpTransactionListener() {} + private NoOpTransactionProfiler() {} - public static NoOpTransactionListener getInstance() { + public static NoOpTransactionProfiler getInstance() { return instance; } diff --git a/sentry/src/main/java/io/sentry/ProfilingTraceData.java b/sentry/src/main/java/io/sentry/ProfilingTraceData.java index 4a8bddbbb8d..a444234fa67 100644 --- a/sentry/src/main/java/io/sentry/ProfilingTraceData.java +++ b/sentry/src/main/java/io/sentry/ProfilingTraceData.java @@ -2,6 +2,7 @@ import java.io.File; import java.util.Locale; +import java.util.UUID; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -21,10 +22,9 @@ public final class ProfilingTraceData { private final @NotNull String device_os_name; private final @NotNull String device_os_version; private final @NotNull String platform; + private final @NotNull String build_id; // Transaction info - private final @NotNull String error_code; - private final @NotNull String error_description; private final @NotNull String transaction_name; // App info @@ -45,40 +45,37 @@ public ProfilingTraceData( @NotNull File traceFile, @NotNull ITransaction transaction, int sdkInt, - @NotNull String deviceManufacturer, - @NotNull String deviceModel, - @NotNull String deviceOsBuildNumber, - @NotNull String deviceOsName, - @NotNull String deviceOsVersion, - @NotNull String versionName, - @NotNull String versionCode, + @Nullable String deviceManufacturer, + @Nullable String deviceModel, + @Nullable String deviceOsVersion, + @Nullable String buildId, + @Nullable String versionName, + @Nullable String versionCode, @Nullable String environment) { this.traceFile = traceFile; // Device metadata this.android_api_level = sdkInt; this.device_locale = Locale.getDefault().toString(); - this.device_manufacturer = deviceManufacturer; - this.device_model = deviceModel; - this.device_os_build_number = deviceOsBuildNumber; - this.device_os_name = deviceOsName; - this.device_os_version = deviceOsVersion; + this.device_manufacturer = deviceManufacturer != null ? deviceManufacturer : ""; + this.device_model = deviceModel != null ? deviceModel : ""; + this.device_os_build_number = ""; + this.device_os_name = "android"; + this.device_os_version = deviceOsVersion != null ? deviceOsVersion : ""; this.platform = "android"; + this.build_id = buildId != null ? buildId : ""; // Transaction info - this.error_code = transaction.getOperation(); - this.error_description = - transaction.getDescription() != null ? transaction.getDescription() : ""; this.transaction_name = transaction.getName(); // App info - this.version_name = versionName; - this.version_code = versionCode; + this.version_name = versionName != null ? versionName : ""; + this.version_code = versionCode != null ? versionCode : ""; // Stacktrace context this.transaction_id = transaction.getEventId().toString(); this.trace_id = transaction.getSpanContext().getTraceId().toString(); - this.stacktrace_id = transaction.getSpanContext().getSpanId().toString(); + this.stacktrace_id = UUID.randomUUID().toString(); this.environment = environment != null ? environment : ""; } @@ -122,12 +119,8 @@ public int getAndroid_api_level() { return platform; } - public @NotNull String getError_code() { - return error_code; - } - - public @NotNull String getError_description() { - return error_description; + public @NotNull String getBuild_id() { + return build_id; } public @NotNull String getTransaction_name() { diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 80d16d266c5..5b576249ae1 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -207,7 +207,7 @@ public static SentryEnvelopeItem fromAttachment( public static @Nullable SentryEnvelopeItem fromProfilingTrace( final @NotNull ProfilingTraceData profilingTraceData, final long maxTraceFileSize, - ISerializer serializer) + final @NotNull ISerializer serializer) throws SentryEnvelopeException { File traceFile = profilingTraceData.getTraceFile(); diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 00548700d84..12978c1df0d 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -285,13 +285,13 @@ public class SentryOptions { private boolean traceSampling; /** Control if profiling is enabled or not for transactions */ - private boolean profilingEnabled = true; + private boolean profilingEnabled = false; /** Max trace file size in bytes. */ private long maxTraceFileSize = 5 * 1024 * 1024; /** Listener interface to perform operations when a transaction is started or ended */ - private @NotNull ITransactionListener transactionListener = NoOpTransactionListener.getInstance(); + private @NotNull ITransactionProfiler transactionListener = NoOpTransactionProfiler.getInstance(); /** * Contains a list of origins to which `sentry-trace` header should be sent in HTTP integrations. @@ -1504,7 +1504,7 @@ public void setMaxTraceFileSize(long maxTraceFileSize) { * * @return the listener interface to perform operations when a transaction is started or ended. */ - public @NotNull ITransactionListener getTransactionListener() { + public @NotNull ITransactionProfiler getTransactionListener() { return transactionListener; } @@ -1513,9 +1513,9 @@ public void setMaxTraceFileSize(long maxTraceFileSize) { * * @param transactionListener - the listener for operations when a transaction is started or ended */ - public void setTransactionListener(final @Nullable ITransactionListener transactionListener) { + public void setTransactionListener(final @Nullable ITransactionProfiler transactionListener) { this.transactionListener = - transactionListener != null ? transactionListener : NoOpTransactionListener.getInstance(); + transactionListener != null ? transactionListener : NoOpTransactionProfiler.getInstance(); } /** diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 4a6d3dc7eea..5d1b676b962 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -244,7 +244,7 @@ public void finish(@Nullable SpanStatus status) { if (transactionFinishedCallback != null) { transactionFinishedCallback.execute(this); } - hub.captureTransaction(transaction, this.traceState(), profilingTraceData); + hub.captureTransaction(transaction, this.traceState(), null, profilingTraceData); } } diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index e13112a005f..0d9b04ccb8e 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -88,7 +88,7 @@ class OutboxSenderTest { fixture.envelopeReader = EnvelopeReader() whenever(fixture.options.maxSpans).thenReturn(1000) whenever(fixture.hub.options).thenReturn(fixture.options) - whenever(fixture.options.transactionListener).thenReturn(NoOpTransactionListener.getInstance()) + whenever(fixture.options.transactionListener).thenReturn(NoOpTransactionProfiler.getInstance()) val transactionContext = TransactionContext("fixture-name", "http") transactionContext.description = "fixture-request" diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 286ad15890d..1ef4ba5be0f 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -122,6 +122,7 @@ class SentryTracerTest { assertEquals(it.transaction, tracer.name) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -155,6 +156,7 @@ class SentryTracerTest { assertEquals(request, it.request) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -182,6 +184,7 @@ class SentryTracerTest { } }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -207,6 +210,7 @@ class SentryTracerTest { } }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -225,6 +229,7 @@ class SentryTracerTest { } }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -242,6 +247,7 @@ class SentryTracerTest { assertEquals("op1", it.spans.first().op) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -374,6 +380,7 @@ class SentryTracerTest { } }, anyOrNull(), + anyOrNull(), anyOrNull() ) @@ -425,7 +432,7 @@ class SentryTracerTest { val child = transaction.startChild("op") child.finish() transaction.finish() - verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test @@ -461,6 +468,7 @@ class SentryTracerTest { assertEquals(SpanStatus.INVALID_ARGUMENT, it.status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -480,6 +488,7 @@ class SentryTracerTest { assertEquals(SpanStatus.DEADLINE_EXCEEDED, it.spans[1].status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -549,6 +558,7 @@ class SentryTracerTest { assertEquals("val", it.getExtra("key")) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -567,6 +577,7 @@ class SentryTracerTest { } }, anyOrNull(), + anyOrNull(), anyOrNull() ) } From 065221c96833cb81aa89a58c3e2c6beb097d4105 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 4 Mar 2022 15:43:46 +0100 Subject: [PATCH 06/14] AndroidTransactionProfiler now has a PackageInfo in constructor to retrieve versionName and versionCode --- .../android/core/AndroidOptionsInitializer.java | 3 ++- .../android/core/AndroidTransactionProfiler.java | 12 ++++++++---- .../java/io/sentry/android/core/ContextUtils.java | 11 +++++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index d7211ad8237..bd8cf411d45 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -117,7 +117,8 @@ static void init( options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); - options.setTransactionListener(new AndroidTransactionProfiler(options)); + options.setTransactionListener( + new AndroidTransactionProfiler(options, ContextUtils.getPackageInfo(context, logger))); } private static void installDefaultIntegrations( 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 f083f2024f4..364c36f8116 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 @@ -2,6 +2,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; +import android.content.pm.PackageInfo; import android.os.Build; import android.os.Debug; import io.sentry.ITransaction; @@ -37,9 +38,13 @@ final class AndroidTransactionProfiler implements ITransactionProfiler { private volatile @Nullable ITransaction activeTransaction = null; private volatile @Nullable ProfilingTraceData timedOutProfilingData = null; private final @NotNull SentryAndroidOptions options; + private final @Nullable PackageInfo packageInfo; - public AndroidTransactionProfiler(final @NotNull SentryAndroidOptions sentryAndroidOptions) { + public AndroidTransactionProfiler( + final @NotNull SentryAndroidOptions sentryAndroidOptions, + final @Nullable PackageInfo packageInfo) { this.options = Objects.requireNonNull(sentryAndroidOptions, "SentryAndroidOptions is required"); + this.packageInfo = packageInfo; final String tracesFilesDirPath = options.getProfilingTracesDirPath(); if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) { options @@ -178,7 +183,6 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { return null; } - // todo how to retrieve version name and code? return new ProfilingTraceData( traceFile, transaction, @@ -187,8 +191,8 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { Build.MODEL, Build.VERSION.RELEASE, options.getProguardUuid(), - "versionName", // ??? - "versionCode", // ??? + packageInfo != null ? ContextUtils.getVersionName(packageInfo) : "", + packageInfo != null ? ContextUtils.getVersionCode(packageInfo) : "", options.getEnvironment()); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java index e577576f25c..bd961282f81 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java @@ -41,6 +41,17 @@ static String getVersionCode(final @NotNull PackageInfo packageInfo) { return getVersionCodeDep(packageInfo); } + /** + * Returns the App's version name based on the PackageInfo + * + * @param packageInfo the PackageInfo class + * @return the versionName + */ + @NotNull + static String getVersionName(final @NotNull PackageInfo packageInfo) { + return packageInfo.versionName; + } + @SuppressWarnings("deprecation") private static @NotNull String getVersionCodeDep(final @NotNull PackageInfo packageInfo) { return Integer.toString(packageInfo.versionCode); From e2f805d1191551052b722f3011cc903d681f35de Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 4 Mar 2022 17:39:35 +0100 Subject: [PATCH 07/14] updated tests transaction tests --- .../io/sentry/apollo/SentryApolloInterceptorTest.kt | 10 ++++++++++ .../sentry/spring/tracing/SentryTracingFilterTest.kt | 12 ++++++++++++ .../spring/tracing/SentryTransactionAdviceTest.kt | 10 ++++++++++ 3 files changed, 32 insertions(+) diff --git a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt index 645f9bba307..e830f02e319 100644 --- a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt +++ b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt @@ -84,6 +84,8 @@ class SentryApolloInterceptorTest { assertTransactionDetails(it) assertEquals(SpanStatus.OK, it.spans.first().status) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -97,6 +99,8 @@ class SentryApolloInterceptorTest { assertTransactionDetails(it) assertEquals(SpanStatus.PERMISSION_DENIED, it.spans.first().status) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -110,6 +114,8 @@ class SentryApolloInterceptorTest { assertTransactionDetails(it) assertEquals(SpanStatus.INTERNAL_ERROR, it.spans.first().status) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -144,6 +150,8 @@ class SentryApolloInterceptorTest { val httpClientSpan = it.spans.first() assertEquals("overwritten description", httpClientSpan.description) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -158,6 +166,8 @@ class SentryApolloInterceptorTest { check { assertEquals(1, it.spans.size) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt index 1dd1dc32587..6748ca5d352 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt @@ -83,6 +83,8 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK) assertThat(it.contexts.trace!!.operation).isEqualTo("http.server") }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -97,6 +99,8 @@ class SentryTracingFilterTest { check { assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -111,6 +115,8 @@ class SentryTracingFilterTest { check { assertThat(it.contexts.trace!!.status).isNull() }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -125,6 +131,8 @@ class SentryTracingFilterTest { check { assertThat(it.contexts.trace!!.parentSpanId).isNull() }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -140,6 +148,8 @@ class SentryTracingFilterTest { check { assertThat(it.contexts.trace!!.parentSpanId).isEqualTo(parentSpanId) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -171,6 +181,8 @@ class SentryTracingFilterTest { check { assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTransactionAdviceTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTransactionAdviceTest.kt index b8f1364a703..3d797ac640b 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTransactionAdviceTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTransactionAdviceTest.kt @@ -63,6 +63,8 @@ class SentryTransactionAdviceTest { assertThat(it.contexts.trace!!.operation).isEqualTo("bean") assertThat(it.status).isEqualTo(SpanStatus.OK) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -74,6 +76,8 @@ class SentryTransactionAdviceTest { check { assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR) }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -86,6 +90,8 @@ class SentryTransactionAdviceTest { assertThat(it.transaction).isEqualTo("SampleService.methodWithoutTransactionNameSet") assertThat(it.contexts.trace!!.operation).isEqualTo("op") }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -107,6 +113,8 @@ class SentryTransactionAdviceTest { assertThat(it.transaction).isEqualTo("ClassAnnotatedSampleService.hello") assertThat(it.contexts.trace!!.operation).isEqualTo("op") }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -119,6 +127,8 @@ class SentryTransactionAdviceTest { assertThat(it.transaction).isEqualTo("ClassAnnotatedWithOperationSampleService.hello") assertThat(it.contexts.trace!!.operation).isEqualTo("my-op") }, + anyOrNull(), + anyOrNull(), anyOrNull() ) } From 2fc46f49020f9050de0bc84e8aa9e37c75cad10b Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 7 Mar 2022 11:31:05 +0100 Subject: [PATCH 08/14] updated changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 433cbb2645b..2690b1d5777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +* Feat: Add Android profiling traces #1897 + ## 5.6.2 ### Various fixes & improvements From 1515671dc1ba0d33b698cdcffba88a2d9c047ac1 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 7 Mar 2022 15:29:11 +0100 Subject: [PATCH 09/14] changed references of transactionListener to transactionProfiler --- .../android/core/AndroidOptionsInitializer.java | 2 +- .../io/sentry/android/core/ContextUtils.java | 2 +- sentry/api/sentry.api | 4 ++-- sentry/src/main/java/io/sentry/Hub.java | 2 +- .../src/main/java/io/sentry/SentryOptions.java | 16 ++++++++-------- sentry/src/main/java/io/sentry/SentryTracer.java | 2 +- .../src/test/java/io/sentry/OutboxSenderTest.kt | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 84421be6df7..e10cbb0373e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -118,7 +118,7 @@ static void init( options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); - options.setTransactionListener( + options.setTransactionProfiler( new AndroidTransactionProfiler(options, ContextUtils.getPackageInfo(context, logger))); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java index bd961282f81..7e8b0c18b89 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java @@ -47,7 +47,7 @@ static String getVersionCode(final @NotNull PackageInfo packageInfo) { * @param packageInfo the PackageInfo class * @return the versionName */ - @NotNull + @Nullable static String getVersionName(final @NotNull PackageInfo packageInfo) { return packageInfo.versionName; } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 169a3bca971..b1653522594 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -918,7 +918,7 @@ public class io/sentry/SentryOptions { public fun getTracesSampleRate ()Ljava/lang/Double; public fun getTracesSampler ()Lio/sentry/SentryOptions$TracesSamplerCallback; public fun getTracingOrigins ()Ljava/util/List; - public fun getTransactionListener ()Lio/sentry/ITransactionProfiler; + public fun getTransactionProfiler ()Lio/sentry/ITransactionProfiler; public fun getTransportFactory ()Lio/sentry/ITransportFactory; public fun getTransportGate ()Lio/sentry/transport/ITransportGate; public fun isAttachServerName ()Z @@ -991,7 +991,7 @@ public class io/sentry/SentryOptions { public fun setTraceSampling (Z)V public fun setTracesSampleRate (Ljava/lang/Double;)V public fun setTracesSampler (Lio/sentry/SentryOptions$TracesSamplerCallback;)V - public fun setTransactionListener (Lio/sentry/ITransactionProfiler;)V + public fun setTransactionProfiler (Lio/sentry/ITransactionProfiler;)V public fun setTransportFactory (Lio/sentry/ITransportFactory;)V public fun setTransportGate (Lio/sentry/transport/ITransportGate;)V } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 1d513ed19cd..dca38fbcc9d 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -665,7 +665,7 @@ public void flush(long timeoutMillis) { // The listener is called only if the transaction exists, as the transaction is needed to // stop it if (samplingDecision && options.isProfilingEnabled()) { - final ITransactionProfiler transactionListener = options.getTransactionListener(); + final ITransactionProfiler transactionListener = options.getTransactionProfiler(); transactionListener.onTransactionStart(transaction); } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 548a18be232..7fcb3ee99ad 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -297,7 +297,7 @@ public class SentryOptions { private long maxTraceFileSize = 5 * 1024 * 1024; /** Listener interface to perform operations when a transaction is started or ended */ - private @NotNull ITransactionProfiler transactionListener = NoOpTransactionProfiler.getInstance(); + private @NotNull ITransactionProfiler transactionProfiler = NoOpTransactionProfiler.getInstance(); /** * Contains a list of origins to which `sentry-trace` header should be sent in HTTP integrations. @@ -1539,18 +1539,18 @@ public void setMaxTraceFileSize(long maxTraceFileSize) { * * @return the listener interface to perform operations when a transaction is started or ended. */ - public @NotNull ITransactionProfiler getTransactionListener() { - return transactionListener; + public @NotNull ITransactionProfiler getTransactionProfiler() { + return transactionProfiler; } /** * Sets the listener interface to perform operations when a transaction is started or ended. * - * @param transactionListener - the listener for operations when a transaction is started or ended + * @param transactionProfiler - the listener for operations when a transaction is started or ended */ - public void setTransactionListener(final @Nullable ITransactionProfiler transactionListener) { - this.transactionListener = - transactionListener != null ? transactionListener : NoOpTransactionProfiler.getInstance(); + public void setTransactionProfiler(final @Nullable ITransactionProfiler transactionProfiler) { + this.transactionProfiler = + transactionProfiler != null ? transactionProfiler : NoOpTransactionProfiler.getInstance(); } /** @@ -1734,7 +1734,7 @@ void merge(final @NotNull SentryOptions options) { if (options.getEnableDeduplication() != null) { setEnableDeduplication(options.getEnableDeduplication()); } - setTransactionListener(options.getTransactionListener()); + setTransactionProfiler(options.getTransactionProfiler()); setMaxTraceFileSize(options.getMaxTraceFileSize()); setProfilingEnabled(options.isProfilingEnabled()); final Map tags = new HashMap<>(options.getTags()); diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 5d1b676b962..8be73cc4c9a 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -209,7 +209,7 @@ public void finish(@Nullable SpanStatus status) { if (!root.isFinished() && (!waitForChildren || hasAllChildrenFinished())) { ProfilingTraceData profilingTraceData = null; if (hub.getOptions().isProfilingEnabled()) { - profilingTraceData = hub.getOptions().getTransactionListener().onTransactionFinish(this); + profilingTraceData = hub.getOptions().getTransactionProfiler().onTransactionFinish(this); } root.finish(finishStatus.spanStatus); diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index 0d9b04ccb8e..0444ed0b90c 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -88,7 +88,7 @@ class OutboxSenderTest { fixture.envelopeReader = EnvelopeReader() whenever(fixture.options.maxSpans).thenReturn(1000) whenever(fixture.hub.options).thenReturn(fixture.options) - whenever(fixture.options.transactionListener).thenReturn(NoOpTransactionProfiler.getInstance()) + whenever(fixture.options.transactionProfiler).thenReturn(NoOpTransactionProfiler.getInstance()) val transactionContext = TransactionContext("fixture-name", "http") transactionContext.description = "fixture-request" From ffa2c21f5d69faed8eaa2f7f508d0258a3aace58 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 9 Mar 2022 17:32:00 +0100 Subject: [PATCH 10/14] profiling traces directory's content is now deleted in the background, instead of deleting the whole dir and recreating it in the main thread AndroidTransactionProfiler now takes a buildInfoProvider in ctor profile envelope item now uses a CachedItem to read the trace file in the background --- .../core/AndroidOptionsInitializer.java | 21 +++++-- .../core/AndroidTransactionProfiler.java | 29 ++++++--- .../android/core/BuildInfoProvider.java | 15 +++++ .../android/core/IBuildInfoProvider.java | 24 ++++++++ .../java/io/sentry/SentryEnvelopeItem.java | 61 +++++++++++-------- .../main/java/io/sentry/util/FileUtils.java | 2 +- 6 files changed, 113 insertions(+), 39 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index e10cbb0373e..8706723e3f2 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -119,7 +119,8 @@ static void init( options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); options.setTransactionProfiler( - new AndroidTransactionProfiler(options, ContextUtils.getPackageInfo(context, logger))); + new AndroidTransactionProfiler( + options, ContextUtils.getPackageInfo(context, logger), buildInfoProvider)); } private static void installDefaultIntegrations( @@ -251,6 +252,7 @@ private static void readDefaultOptionValues( * @param context the Application context * @param options the SentryOptions */ + @SuppressWarnings("FutureReturnValueIgnored") private static void initializeCacheDirs( final @NotNull Context context, final @NotNull SentryAndroidOptions options) { final File cacheDir = new File(context.getCacheDir(), "sentry"); @@ -259,10 +261,21 @@ private static void initializeCacheDirs( options.setProfilingTracesDirPath(profilingTracesDir.getAbsolutePath()); if (options.isProfilingEnabled()) { - // 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. - FileUtils.deleteRecursively(profilingTracesDir); 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); + } + }); } } 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 364c36f8116..e0e1c3ef5e1 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 @@ -2,6 +2,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; +import android.annotation.SuppressLint; import android.content.pm.PackageInfo; import android.os.Build; import android.os.Debug; @@ -38,12 +39,16 @@ final class AndroidTransactionProfiler implements ITransactionProfiler { private volatile @Nullable ITransaction activeTransaction = null; private volatile @Nullable ProfilingTraceData timedOutProfilingData = null; private final @NotNull SentryAndroidOptions options; + private final @NotNull IBuildInfoProvider buildInfoProvider; private final @Nullable PackageInfo packageInfo; public AndroidTransactionProfiler( final @NotNull SentryAndroidOptions sentryAndroidOptions, - final @Nullable PackageInfo packageInfo) { + final @Nullable PackageInfo packageInfo, + final @NotNull IBuildInfoProvider buildInfoProvider) { this.options = Objects.requireNonNull(sentryAndroidOptions, "SentryAndroidOptions is required"); + this.buildInfoProvider = + Objects.requireNonNull(buildInfoProvider, "The BuildInfoProvider is required."); this.packageInfo = packageInfo; final String tracesFilesDirPath = options.getProfilingTracesDirPath(); if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) { @@ -68,11 +73,12 @@ public AndroidTransactionProfiler( traceFilesDir = new File(tracesFilesDirPath); } + @SuppressLint("NewApi") @Override public synchronized void onTransactionStart(@NotNull ITransaction transaction) { // Debug.startMethodTracingSampling() is only available since Lollipop - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) return; + 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 // we already logged that @@ -183,16 +189,23 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { return null; } + String versionName = ""; + String versionCode = ""; + if (packageInfo != null) { + versionName = ContextUtils.getVersionName(packageInfo); + versionCode = ContextUtils.getVersionCode(packageInfo); + } + return new ProfilingTraceData( traceFile, transaction, - Build.VERSION.SDK_INT, - Build.MANUFACTURER, - Build.MODEL, - Build.VERSION.RELEASE, + buildInfoProvider.getSdkInfoVersion(), + buildInfoProvider.getManufacturer(), + buildInfoProvider.getModel(), + buildInfoProvider.getVersionRelease(), options.getProguardUuid(), - packageInfo != null ? ContextUtils.getVersionName(packageInfo) : "", - packageInfo != null ? ContextUtils.getVersionCode(packageInfo) : "", + versionName, + versionCode, options.getEnvironment()); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/BuildInfoProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/BuildInfoProvider.java index 2b8fdc811f6..b4ac6cc087a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/BuildInfoProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/BuildInfoProvider.java @@ -22,4 +22,19 @@ public int getSdkInfoVersion() { public @Nullable String getBuildTags() { return Build.TAGS; } + + @Override + public @Nullable String getManufacturer() { + return Build.MANUFACTURER; + } + + @Override + public @Nullable String getModel() { + return Build.MODEL; + } + + @Override + public @Nullable String getVersionRelease() { + return Build.VERSION.RELEASE; + } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/IBuildInfoProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/IBuildInfoProvider.java index e23c4db4b89..7cfcf81ec40 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/IBuildInfoProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/IBuildInfoProvider.java @@ -19,4 +19,28 @@ public interface IBuildInfoProvider { */ @Nullable String getBuildTags(); + + /** + * Returns the manufacturer of the device + * + * @return the Manufacturer + */ + @Nullable + String getManufacturer(); + + /** + * Returns the model of the device + * + * @return the Build tags + */ + @Nullable + String getModel(); + + /** + * Returns the release version of the device os + * + * @return the Release version + */ + @Nullable + String getVersionRelease(); } diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 5b576249ae1..66aa2b4fb43 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -215,32 +215,41 @@ public static SentryEnvelopeItem fromAttachment( return null; } - // The payload of the profiling item is a json that includes the trace file encoded with base64 - byte[] traceFileBytes = readBytesFromFile(traceFile.getPath(), maxTraceFileSize); - String base64Trace = Base64.encodeToString(traceFileBytes, NO_WRAP | NO_PADDING); - profilingTraceData.setStacktrace(base64Trace); - - try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { - serializer.serialize(profilingTraceData, writer); - byte[] payloadBytes = stream.toByteArray(); - - SentryEnvelopeItemHeader itemHeader = - new SentryEnvelopeItemHeader( - SentryItemType.Profile, - () -> payloadBytes.length, - "application-json", - traceFile.getName()); - - // Don't use method reference. This can cause issues on Android - return new SentryEnvelopeItem(itemHeader, payloadBytes); - } catch (IOException e) { - throw new SentryEnvelopeException( - String.format("Failed to serialize profiling trace data\n%s", e.getMessage())); - } finally { - // In any case we delete the trace file - traceFile.delete(); - } + // Using CachedItem, so we read the trace file in the background + final CachedItem cachedItem = + new CachedItem( + () -> { + if (!traceFile.exists()) { + return null; + } + // 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.setStacktrace(base64Trace); + + try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); + final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + serializer.serialize(profilingTraceData, writer); + return stream.toByteArray(); + } catch (IOException e) { + throw new SentryEnvelopeException( + String.format("Failed to serialize profiling trace data\n%s", e.getMessage())); + } finally { + // In any case we delete the trace file + traceFile.delete(); + } + }); + + SentryEnvelopeItemHeader itemHeader = + new SentryEnvelopeItemHeader( + SentryItemType.Profile, + () -> cachedItem.getBytes().length, + "application-json", + traceFile.getName()); + + // Don't use method reference. This can cause issues on Android + return new SentryEnvelopeItem(itemHeader, () -> cachedItem.getBytes()); } private static byte[] readBytesFromFile(String pathname, long maxFileLength) diff --git a/sentry/src/main/java/io/sentry/util/FileUtils.java b/sentry/src/main/java/io/sentry/util/FileUtils.java index ec4feeb3277..33af2e7b41c 100644 --- a/sentry/src/main/java/io/sentry/util/FileUtils.java +++ b/sentry/src/main/java/io/sentry/util/FileUtils.java @@ -27,6 +27,6 @@ public static boolean deleteRecursively(@Nullable File file) { for (File f : children) { if (!deleteRecursively(f)) return false; } - return true; + return file.delete(); } } From 7830a2556e5f51e1711f5061948440a9e8124edd Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 9 Mar 2022 18:27:29 +0100 Subject: [PATCH 11/14] profiling traces directory's content is now deleted in the background, instead of deleting the whole dir and recreating it in the main thread AndroidTransactionProfiler now takes a buildInfoProvider in ctor profile envelope item now uses a CachedItem to read the trace file in the background --- sentry-android-core/api/sentry-android-core.api | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 0918f1b50d6..6f9aa87b2eb 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -60,7 +60,10 @@ public final class io/sentry/android/core/BuildConfig { public final class io/sentry/android/core/BuildInfoProvider : io/sentry/android/core/IBuildInfoProvider { public fun ()V public fun getBuildTags ()Ljava/lang/String; + public fun getManufacturer ()Ljava/lang/String; + public fun getModel ()Ljava/lang/String; public fun getSdkInfoVersion ()I + public fun getVersionRelease ()Ljava/lang/String; } public abstract class io/sentry/android/core/EnvelopeFileObserverIntegration : io/sentry/Integration, java/io/Closeable { @@ -72,7 +75,10 @@ public abstract class io/sentry/android/core/EnvelopeFileObserverIntegration : i public abstract interface class io/sentry/android/core/IBuildInfoProvider { public abstract fun getBuildTags ()Ljava/lang/String; + public abstract fun getManufacturer ()Ljava/lang/String; + public abstract fun getModel ()Ljava/lang/String; public abstract fun getSdkInfoVersion ()I + public abstract fun getVersionRelease ()Ljava/lang/String; } public abstract interface class io/sentry/android/core/IDebugImagesLoader { From 2a0175628969f28cce969f6f5a573d7daa9d38cc Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 10 Mar 2022 22:11:26 +0100 Subject: [PATCH 12/14] added fields to ProfilingTraceData: -device_is_emulator -device_cpu_frequencies -device_physical_memory_bytes -duration_ns added context to AndroidTransactionProfiler instead of packageInfo to retrieve memory info added isEmulator() to BuildInfoProvider --- .../api/sentry-android-core.api | 4 +- .../core/AndroidOptionsInitializer.java | 5 +- .../core/AndroidTransactionProfiler.java | 49 ++++++++++++++++- .../android/core/BuildInfoProvider.java | 41 ++++++++++++++ .../core/DefaultAndroidEventProcessor.java | 33 +----------- .../android/core/IBuildInfoProvider.java | 8 +++ .../core/internal/util/CpuInfoUtils.java | 54 +++++++++++++++++++ sentry/api/sentry.api | 7 ++- .../java/io/sentry/ProfilingTraceData.java | 33 +++++++++++- .../main/java/io/sentry/util/FileUtils.java | 27 ++++++++++ 10 files changed, 221 insertions(+), 40 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/internal/util/CpuInfoUtils.java diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 6f9aa87b2eb..5799c87a357 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -58,12 +58,13 @@ public final class io/sentry/android/core/BuildConfig { } public final class io/sentry/android/core/BuildInfoProvider : io/sentry/android/core/IBuildInfoProvider { - public fun ()V + public fun (Lio/sentry/ILogger;)V public fun getBuildTags ()Ljava/lang/String; public fun getManufacturer ()Ljava/lang/String; public fun getModel ()Ljava/lang/String; public fun getSdkInfoVersion ()I public fun getVersionRelease ()Ljava/lang/String; + public fun isEmulator ()Ljava/lang/Boolean; } public abstract class io/sentry/android/core/EnvelopeFileObserverIntegration : io/sentry/Integration, java/io/Closeable { @@ -79,6 +80,7 @@ public abstract interface class io/sentry/android/core/IBuildInfoProvider { public abstract fun getModel ()Ljava/lang/String; public abstract fun getSdkInfoVersion ()I public abstract fun getVersionRelease ()Ljava/lang/String; + public abstract fun isEmulator ()Ljava/lang/Boolean; } public abstract interface class io/sentry/android/core/IDebugImagesLoader { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 8706723e3f2..3a6b3637af5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -58,7 +58,7 @@ static void init( final @NotNull SentryAndroidOptions options, @NotNull Context context, final @NotNull ILogger logger) { - init(options, context, logger, new BuildInfoProvider()); + init(options, context, logger, new BuildInfoProvider(logger)); } /** @@ -119,8 +119,7 @@ static void init( options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); options.setTransactionProfiler( - new AndroidTransactionProfiler( - options, ContextUtils.getPackageInfo(context, logger), buildInfoProvider)); + new AndroidTransactionProfiler(context, options, buildInfoProvider)); } private static void installDefaultIntegrations( 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 e0e1c3ef5e1..bcb77554ec5 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 @@ -1,15 +1,20 @@ package io.sentry.android.core; +import static android.content.Context.ACTIVITY_SERVICE; import static java.util.concurrent.TimeUnit.MILLISECONDS; import android.annotation.SuppressLint; +import android.app.ActivityManager; +import android.content.Context; import android.content.pm.PackageInfo; import android.os.Build; import android.os.Debug; +import android.os.SystemClock; import io.sentry.ITransaction; import io.sentry.ITransactionProfiler; import io.sentry.ProfilingTraceData; import io.sentry.SentryLevel; +import io.sentry.android.core.internal.util.CpuInfoUtils; import io.sentry.util.Objects; import java.io.File; import java.util.UUID; @@ -38,18 +43,21 @@ final class AndroidTransactionProfiler implements ITransactionProfiler { private @Nullable Future scheduledFinish = null; private volatile @Nullable ITransaction activeTransaction = null; private volatile @Nullable ProfilingTraceData timedOutProfilingData = null; + private final @NotNull Context context; private final @NotNull SentryAndroidOptions options; private final @NotNull IBuildInfoProvider buildInfoProvider; private final @Nullable PackageInfo packageInfo; + private long transactionStartNanos = 0; public AndroidTransactionProfiler( + final @NotNull Context context, final @NotNull SentryAndroidOptions sentryAndroidOptions, - final @Nullable PackageInfo packageInfo, final @NotNull IBuildInfoProvider buildInfoProvider) { + this.context = Objects.requireNonNull(context, "The application context is required"); this.options = Objects.requireNonNull(sentryAndroidOptions, "SentryAndroidOptions is required"); this.buildInfoProvider = Objects.requireNonNull(buildInfoProvider, "The BuildInfoProvider is required."); - this.packageInfo = packageInfo; + this.packageInfo = ContextUtils.getPackageInfo(context, options.getLogger()); final String tracesFilesDirPath = options.getProfilingTracesDirPath(); if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) { options @@ -116,13 +124,19 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { () -> timedOutProfilingData = onTransactionFinish(transaction), PROFILING_TIMEOUT_MILLIS); + transactionStartNanos = SystemClock.elapsedRealtimeNanos(); Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs); } + @SuppressLint("NewApi") @Override public synchronized @Nullable ProfilingTraceData onTransactionFinish( @NotNull ITransaction transaction) { + // onTransactionStart() is only available since Lollipop + // and SystemClock.elapsedRealtimeNanos() since Jelly Bean + if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) return null; + final ITransaction finalActiveTransaction = activeTransaction; final ProfilingTraceData profilingData = timedOutProfilingData; @@ -171,6 +185,7 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { } Debug.stopMethodTracing(); + long transactionDurationNanos = SystemClock.elapsedRealtimeNanos() - transactionStartNanos; activeTransaction = null; @@ -191,21 +206,51 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { String versionName = ""; String versionCode = ""; + String totalMem = "0"; + ActivityManager.MemoryInfo memInfo = getMemInfo(); if (packageInfo != null) { versionName = ContextUtils.getVersionName(packageInfo); versionCode = ContextUtils.getVersionCode(packageInfo); } + if (memInfo != null) { + totalMem = Long.toString(memInfo.totalMem); + } return new ProfilingTraceData( traceFile, transaction, + Long.toString(transactionDurationNanos), buildInfoProvider.getSdkInfoVersion(), buildInfoProvider.getManufacturer(), buildInfoProvider.getModel(), buildInfoProvider.getVersionRelease(), + buildInfoProvider.isEmulator(), + CpuInfoUtils.readMaxFrequencies(), + totalMem, options.getProguardUuid(), versionName, versionCode, options.getEnvironment()); } + + /** + * Get MemoryInfo object representing the memory state of the application. + * + * @return MemoryInfo object representing the memory state of the application + */ + private @Nullable ActivityManager.MemoryInfo getMemInfo() { + try { + ActivityManager actManager = (ActivityManager) context.getSystemService(ACTIVITY_SERVICE); + ActivityManager.MemoryInfo memInfo = new ActivityManager.MemoryInfo(); + if (actManager != null) { + actManager.getMemoryInfo(memInfo); + return memInfo; + } + options.getLogger().log(SentryLevel.INFO, "Error getting MemoryInfo."); + return null; + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, "Error getting MemoryInfo.", e); + return null; + } + } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/BuildInfoProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/BuildInfoProvider.java index b4ac6cc087a..4ed441db999 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/BuildInfoProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/BuildInfoProvider.java @@ -1,13 +1,22 @@ package io.sentry.android.core; import android.os.Build; +import io.sentry.ILogger; +import io.sentry.SentryLevel; +import io.sentry.util.Objects; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; /** The Android Impl. of IBuildInfoProvider which returns the Build class info. */ @ApiStatus.Internal public final class BuildInfoProvider implements IBuildInfoProvider { + final @NotNull ILogger logger; + + public BuildInfoProvider(final @NotNull ILogger logger) { + this.logger = Objects.requireNonNull(logger, "The ILogger object is required."); + } /** * Returns the Build.VERSION.SDK_INT * @@ -37,4 +46,36 @@ public int getSdkInfoVersion() { public @Nullable String getVersionRelease() { return Build.VERSION.RELEASE; } + + /** + * Check whether the application is running in an emulator. + * https://github.com/flutter/plugins/blob/master/packages/device_info/android/src/main/java/io/flutter/plugins/deviceinfo/DeviceInfoPlugin.java#L105 + * + * @return true if the application is running in an emulator, false otherwise + */ + @Override + public @Nullable Boolean isEmulator() { + try { + return (Build.BRAND.startsWith("generic") && Build.DEVICE.startsWith("generic")) + || Build.FINGERPRINT.startsWith("generic") + || Build.FINGERPRINT.startsWith("unknown") + || Build.HARDWARE.contains("goldfish") + || Build.HARDWARE.contains("ranchu") + || Build.MODEL.contains("google_sdk") + || Build.MODEL.contains("Emulator") + || Build.MODEL.contains("Android SDK built for x86") + || Build.MANUFACTURER.contains("Genymotion") + || Build.PRODUCT.contains("sdk_google") + || Build.PRODUCT.contains("google_sdk") + || Build.PRODUCT.contains("sdk") + || Build.PRODUCT.contains("sdk_x86") + || Build.PRODUCT.contains("vbox86p") + || Build.PRODUCT.contains("emulator") + || Build.PRODUCT.contains("simulator"); + } catch (Throwable e) { + logger.log( + SentryLevel.ERROR, "Error checking whether application is running in an emulator.", e); + return null; + } + } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 3b5539536bf..df1b6ef00dd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -105,7 +105,7 @@ public DefaultAndroidEventProcessor( } // its not IO, but it has been cached in the old version as well - map.put(EMULATOR, isEmulator()); + map.put(EMULATOR, buildInfoProvider.isEmulator()); final Map sideLoadedInfo = getSideLoadedInfo(); if (sideLoadedInfo != null) { @@ -523,37 +523,6 @@ private TimeZone getTimeZone() { return deviceOrientation; } - /** - * Check whether the application is running in an emulator. - * https://github.com/flutter/plugins/blob/master/packages/device_info/android/src/main/java/io/flutter/plugins/deviceinfo/DeviceInfoPlugin.java#L105 - * - * @return true if the application is running in an emulator, false otherwise - */ - private @Nullable Boolean isEmulator() { - try { - return (Build.BRAND.startsWith("generic") && Build.DEVICE.startsWith("generic")) - || Build.FINGERPRINT.startsWith("generic") - || Build.FINGERPRINT.startsWith("unknown") - || Build.HARDWARE.contains("goldfish") - || Build.HARDWARE.contains("ranchu") - || Build.MODEL.contains("google_sdk") - || Build.MODEL.contains("Emulator") - || Build.MODEL.contains("Android SDK built for x86") - || Build.MANUFACTURER.contains("Genymotion") - || Build.PRODUCT.contains("sdk_google") - || Build.PRODUCT.contains("google_sdk") - || Build.PRODUCT.contains("sdk") - || Build.PRODUCT.contains("sdk_x86") - || Build.PRODUCT.contains("vbox86p") - || Build.PRODUCT.contains("emulator") - || Build.PRODUCT.contains("simulator"); - } catch (Throwable e) { - logger.log( - SentryLevel.ERROR, "Error checking whether application is running in an emulator.", e); - return null; - } - } - /** * Get the total amount of internal storage, in bytes. * diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/IBuildInfoProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/IBuildInfoProvider.java index 7cfcf81ec40..b5390e09e1d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/IBuildInfoProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/IBuildInfoProvider.java @@ -43,4 +43,12 @@ public interface IBuildInfoProvider { */ @Nullable String getVersionRelease(); + + /** + * Check whether the application is running in an emulator. + * + * @return true if the application is running in an emulator, false otherwise + */ + @Nullable + Boolean isEmulator(); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/CpuInfoUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/CpuInfoUtils.java new file mode 100644 index 00000000000..190cddc4c1e --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/CpuInfoUtils.java @@ -0,0 +1,54 @@ +package io.sentry.android.core.internal.util; + +import io.sentry.util.FileUtils; +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +@ApiStatus.Internal +public final class CpuInfoUtils { + + private static final @NotNull String SYSTEM_CPU_PATH = "/sys/devices/system/cpu/"; + private static final @NotNull String CPUINFO_MAX_FREQ_PATH = "cpufreq/cpuinfo_max_freq"; + + /** Cached max frequencies to avoid reading files multiple times */ + private static @NotNull List cpuMaxFrequenciesMhz = new ArrayList<>(); + + /** + * Read the max frequency of each core of the cpu and returns it in Mhz + * + * @return A list with the frequency of each core of the cpu in Mhz + */ + public static @NotNull List readMaxFrequencies() { + if (!cpuMaxFrequenciesMhz.isEmpty()) { + return cpuMaxFrequenciesMhz; + } + File[] cpuDirs = new File(SYSTEM_CPU_PATH).listFiles(); + if (cpuDirs == null) { + return new ArrayList<>(); + } + + for (File cpuDir : cpuDirs) { + if (!cpuDir.getName().matches("cpu[0-9]+")) continue; + File cpuMaxFreqFile = new File(cpuDir, CPUINFO_MAX_FREQ_PATH); + + if (!cpuMaxFreqFile.exists() || !cpuMaxFreqFile.canRead()) continue; + + long khz = 0; + try { + String content = FileUtils.readText(cpuMaxFreqFile); + if (content == null) continue; + khz = Long.parseLong(content.trim()); + } catch (NumberFormatException e) { + continue; + } catch (IOException e) { + continue; + } + cpuMaxFrequenciesMhz.add(Long.toString(khz / 1000)); + } + return cpuMaxFrequenciesMhz; + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index b1653522594..dcde7a8654c 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -517,15 +517,18 @@ public final class io/sentry/OutboxSender : io/sentry/IEnvelopeSender { } public final class io/sentry/ProfilingTraceData { - public fun (Ljava/io/File;Lio/sentry/ITransaction;ILjava/lang/String;Ljava/lang/String;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/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 getAndroid_api_level ()I public fun getBuild_id ()Ljava/lang/String; + public fun getDevice_cpu_frequencies ()Ljava/util/List; public fun getDevice_locale ()Ljava/lang/String; public fun getDevice_manufacturer ()Ljava/lang/String; public fun getDevice_model ()Ljava/lang/String; public fun getDevice_os_build_number ()Ljava/lang/String; public fun getDevice_os_name ()Ljava/lang/String; public fun getDevice_os_version ()Ljava/lang/String; + public fun getDevice_physical_memory_bytes ()Ljava/lang/String; + public fun getDuration_ns ()Ljava/lang/String; public fun getEnvironment ()Ljava/lang/String; public fun getPlatform ()Ljava/lang/String; public fun getStacktrace ()Ljava/lang/String; @@ -537,6 +540,7 @@ public final class io/sentry/ProfilingTraceData { public fun getTransaction_name ()Ljava/lang/String; public fun getVersion_code ()Ljava/lang/String; public fun getVersion_name ()Ljava/lang/String; + public fun isDevice_is_emulator ()Z public fun setStacktrace (Ljava/lang/String;)V } @@ -2101,6 +2105,7 @@ public final class io/sentry/util/ExceptionUtils { public final class io/sentry/util/FileUtils { public fun ()V public static fun deleteRecursively (Ljava/io/File;)Z + public static fun readText (Ljava/io/File;)Ljava/lang/String; } public final class io/sentry/util/LogUtils { diff --git a/sentry/src/main/java/io/sentry/ProfilingTraceData.java b/sentry/src/main/java/io/sentry/ProfilingTraceData.java index a444234fa67..4967e997845 100644 --- a/sentry/src/main/java/io/sentry/ProfilingTraceData.java +++ b/sentry/src/main/java/io/sentry/ProfilingTraceData.java @@ -1,6 +1,7 @@ package io.sentry; import java.io.File; +import java.util.List; import java.util.Locale; import java.util.UUID; import org.jetbrains.annotations.ApiStatus; @@ -21,11 +22,16 @@ public final class ProfilingTraceData { private final @NotNull String device_os_build_number; 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 final @NotNull String device_physical_memory_bytes; private final @NotNull String platform; private final @NotNull String build_id; // Transaction info private final @NotNull String transaction_name; + // duration_ns is a String to avoid issues with numbers and json + private final @NotNull String duration_ns; // App info private final @NotNull String version_name; @@ -44,10 +50,14 @@ public final class ProfilingTraceData { public ProfilingTraceData( @NotNull File traceFile, @NotNull ITransaction transaction, + @NotNull String durationNanos, int sdkInt, @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, @@ -59,14 +69,19 @@ public ProfilingTraceData( this.device_locale = Locale.getDefault().toString(); this.device_manufacturer = deviceManufacturer != null ? deviceManufacturer : ""; 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 = ""; this.device_os_name = "android"; - this.device_os_version = deviceOsVersion != null ? deviceOsVersion : ""; this.platform = "android"; this.build_id = buildId != null ? buildId : ""; // Transaction info this.transaction_name = transaction.getName(); + this.duration_ns = durationNanos; // App info this.version_name = versionName != null ? versionName : ""; @@ -115,6 +130,10 @@ public int getAndroid_api_level() { return device_os_version; } + public boolean isDevice_is_emulator() { + return device_is_emulator; + } + public @NotNull String getPlatform() { return platform; } @@ -155,6 +174,18 @@ public int getAndroid_api_level() { return stacktrace; } + public @NotNull String getDuration_ns() { + return duration_ns; + } + + public @NotNull List getDevice_cpu_frequencies() { + return device_cpu_frequencies; + } + + public @NotNull String getDevice_physical_memory_bytes() { + return device_physical_memory_bytes; + } + public void setStacktrace(@Nullable String stacktrace) { this.stacktrace = stacktrace; } diff --git a/sentry/src/main/java/io/sentry/util/FileUtils.java b/sentry/src/main/java/io/sentry/util/FileUtils.java index 33af2e7b41c..b5ac3f8a1ac 100644 --- a/sentry/src/main/java/io/sentry/util/FileUtils.java +++ b/sentry/src/main/java/io/sentry/util/FileUtils.java @@ -1,6 +1,9 @@ package io.sentry.util; +import java.io.BufferedReader; import java.io.File; +import java.io.FileReader; +import java.io.IOException; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Nullable; @@ -29,4 +32,28 @@ public static boolean deleteRecursively(@Nullable File file) { } return file.delete(); } + + /** + * Reads the content of a File into a String. If the file does not exist or is not a file, null is + * returned. Do not use with large files, as the String is kept in memory! + * + * @param file file to read + * @return a String containing all the content of the file, or null if it doesn't exists + * @throws IOException In case of error reading the file + */ + @SuppressWarnings("DefaultCharset") + public static @Nullable String readText(@Nullable File file) throws IOException { + if (file == null || !file.exists() || !file.isFile()) { + return null; + } + StringBuilder contentBuilder = new StringBuilder(); + try (BufferedReader br = new BufferedReader(new FileReader(file))) { + + String line; + while ((line = br.readLine()) != null) { + contentBuilder.append(line).append("\n"); + } + } + return contentBuilder.toString(); + } } From 1c535818d20365a0813ecf930fce80d39111ca99 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 11 Mar 2022 17:02:31 +0100 Subject: [PATCH 13/14] last naming update to fields of ProfilingTraceData: -stacktrace -> sampled_profile -stacktrace_id -> profile_id --- sentry/api/sentry.api | 6 +++--- .../java/io/sentry/ProfilingTraceData.java | 20 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index dcde7a8654c..064ed9f6d62 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -531,8 +531,8 @@ public final class io/sentry/ProfilingTraceData { public fun getDuration_ns ()Ljava/lang/String; public fun getEnvironment ()Ljava/lang/String; public fun getPlatform ()Ljava/lang/String; - public fun getStacktrace ()Ljava/lang/String; - public fun getStacktrace_id ()Ljava/lang/String; + public fun getProfile_id ()Ljava/lang/String; + public fun getSampled_profile ()Ljava/lang/String; public fun getTraceFile ()Ljava/io/File; public fun getTraceId ()Ljava/lang/String; public fun getTrace_id ()Ljava/lang/String; @@ -541,7 +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 setStacktrace (Ljava/lang/String;)V + public fun setSampled_profile (Ljava/lang/String;)V } public final class io/sentry/RequestDetails { diff --git a/sentry/src/main/java/io/sentry/ProfilingTraceData.java b/sentry/src/main/java/io/sentry/ProfilingTraceData.java index 4967e997845..9e5eff5dd28 100644 --- a/sentry/src/main/java/io/sentry/ProfilingTraceData.java +++ b/sentry/src/main/java/io/sentry/ProfilingTraceData.java @@ -40,12 +40,12 @@ public final class ProfilingTraceData { // Stacktrace context private final @NotNull String transaction_id; private final @NotNull String trace_id; - private final @NotNull String stacktrace_id; + private final @NotNull String profile_id; private final @NotNull String environment; // Stacktrace (file) - /** Stacktrace encoded with Base64 */ - private @Nullable String stacktrace = null; + /** Profile trace encoded with Base64 */ + private @Nullable String sampled_profile = null; public ProfilingTraceData( @NotNull File traceFile, @@ -90,7 +90,7 @@ public ProfilingTraceData( // Stacktrace context this.transaction_id = transaction.getEventId().toString(); this.trace_id = transaction.getSpanContext().getTraceId().toString(); - this.stacktrace_id = UUID.randomUUID().toString(); + this.profile_id = UUID.randomUUID().toString(); this.environment = environment != null ? environment : ""; } @@ -162,16 +162,16 @@ public boolean isDevice_is_emulator() { return trace_id; } - public @NotNull String getStacktrace_id() { - return stacktrace_id; + public @NotNull String getProfile_id() { + return profile_id; } public @NotNull String getEnvironment() { return environment; } - public @Nullable String getStacktrace() { - return stacktrace; + public @Nullable String getSampled_profile() { + return sampled_profile; } public @NotNull String getDuration_ns() { @@ -186,7 +186,7 @@ public boolean isDevice_is_emulator() { return device_physical_memory_bytes; } - public void setStacktrace(@Nullable String stacktrace) { - this.stacktrace = stacktrace; + public void setSampled_profile(@Nullable String sampledProfile) { + this.sampled_profile = sampledProfile; } } From 7d08aca2434b7df95d41722bc63cf67a5cf1a1bf Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 11 Mar 2022 17:11:01 +0100 Subject: [PATCH 14/14] last naming update to fields of ProfilingTraceData: -stacktrace -> sampled_profile -stacktrace_id -> profile_id --- sentry/src/main/java/io/sentry/SentryEnvelopeItem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 66aa2b4fb43..b110f6491c4 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -226,7 +226,7 @@ public static SentryEnvelopeItem fromAttachment( // base64 byte[] traceFileBytes = readBytesFromFile(traceFile.getPath(), maxTraceFileSize); String base64Trace = Base64.encodeToString(traceFileBytes, NO_WRAP | NO_PADDING); - profilingTraceData.setStacktrace(base64Trace); + profilingTraceData.setSampled_profile(base64Trace); try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) {