diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java index 82ebe2e6a7a..654cd17fe44 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java @@ -301,7 +301,7 @@ private void stop(final boolean restartProfiler) { endData.measurementsMap, endData.traceFile, startProfileChunkTimestamp, - ProfileChunk.Platform.ANDROID)); + ProfileChunk.PLATFORM_ANDROID)); } } diff --git a/sentry-async-profiler/api/sentry-async-profiler.api b/sentry-async-profiler/api/sentry-async-profiler.api index 0bf859efb1e..af2962cbd61 100644 --- a/sentry-async-profiler/api/sentry-async-profiler.api +++ b/sentry-async-profiler/api/sentry-async-profiler.api @@ -4,10 +4,19 @@ public final class io/sentry/asyncprofiler/BuildConfig { } public final class io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverter : io/sentry/asyncprofiler/vendor/asyncprofiler/convert/JfrConverter { - public fun (Lio/sentry/asyncprofiler/vendor/asyncprofiler/jfr/JfrReader;Lio/sentry/asyncprofiler/vendor/asyncprofiler/convert/Arguments;Lio/sentry/SentryStackTraceFactory;)V + public fun (Lio/sentry/asyncprofiler/vendor/asyncprofiler/jfr/JfrReader;Lio/sentry/asyncprofiler/vendor/asyncprofiler/convert/Arguments;Lio/sentry/SentryStackTraceFactory;Lio/sentry/ILogger;)V public static fun convertFromFileStatic (Ljava/nio/file/Path;)Lio/sentry/protocol/profiling/SentryProfile; } +public final class io/sentry/asyncprofiler/convert/NonAggregatingEventCollector : io/sentry/asyncprofiler/vendor/asyncprofiler/jfr/event/EventCollector { + public fun ()V + public fun afterChunk ()V + public fun beforeChunk ()V + public fun collect (Lio/sentry/asyncprofiler/vendor/asyncprofiler/jfr/event/Event;)V + public fun finish ()Z + public fun forEach (Lio/sentry/asyncprofiler/vendor/asyncprofiler/jfr/event/EventCollector$Visitor;)V +} + public final class io/sentry/asyncprofiler/profiling/JavaContinuousProfiler : io/sentry/IContinuousProfiler, io/sentry/transport/RateLimiter$IRateLimitObserver { public fun (Lio/sentry/ILogger;Ljava/lang/String;ILio/sentry/ISentryExecutorService;)V public fun close (Z)V diff --git a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverter.java b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverter.java index 2fc7bda4775..4489497e815 100644 --- a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverter.java +++ b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverter.java @@ -1,13 +1,16 @@ package io.sentry.asyncprofiler.convert; import io.sentry.DateUtils; +import io.sentry.ILogger; import io.sentry.Sentry; +import io.sentry.SentryLevel; import io.sentry.SentryStackTraceFactory; import io.sentry.asyncprofiler.vendor.asyncprofiler.convert.Arguments; import io.sentry.asyncprofiler.vendor.asyncprofiler.convert.JfrConverter; import io.sentry.asyncprofiler.vendor.asyncprofiler.jfr.JfrReader; import io.sentry.asyncprofiler.vendor.asyncprofiler.jfr.StackTrace; import io.sentry.asyncprofiler.vendor.asyncprofiler.jfr.event.Event; +import io.sentry.asyncprofiler.vendor.asyncprofiler.jfr.event.EventCollector; import io.sentry.protocol.SentryStackFrame; import io.sentry.protocol.profiling.SentryProfile; import io.sentry.protocol.profiling.SentrySample; @@ -15,20 +18,30 @@ import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; public final class JfrAsyncProfilerToSentryProfileConverter extends JfrConverter { - private static final long NANOS_PER_SECOND = 1_000_000_000L; + private static final double NANOS_PER_SECOND = 1_000_000_000.0; + private static final long UNKNOWN_THREAD_ID = -1; private final @NotNull SentryProfile sentryProfile = new SentryProfile(); private final @NotNull SentryStackTraceFactory stackTraceFactory; + private final @NotNull ILogger logger; + private final @NotNull Map frameDeduplicationMap = new HashMap<>(); + private final @NotNull Map, Integer> stackDeduplicationMap = new HashMap<>(); public JfrAsyncProfilerToSentryProfileConverter( - JfrReader jfr, Arguments args, @NotNull SentryStackTraceFactory stackTraceFactory) { + JfrReader jfr, + Arguments args, + @NotNull SentryStackTraceFactory stackTraceFactory, + @NotNull ILogger logger) { super(jfr, args); this.stackTraceFactory = stackTraceFactory; + this.logger = logger; } @Override @@ -36,12 +49,18 @@ protected void convertChunk() { collector.forEach(new ProfileEventVisitor(sentryProfile, stackTraceFactory, jfr, args)); } + @Override + protected EventCollector createCollector(Arguments args) { + return new NonAggregatingEventCollector(); + } + public static @NotNull SentryProfile convertFromFileStatic(@NotNull Path jfrFilePath) throws IOException { JfrAsyncProfilerToSentryProfileConverter converter; try (JfrReader jfrReader = new JfrReader(jfrFilePath.toString())) { Arguments args = new Arguments(); args.cpu = false; + args.wall = true; args.alloc = false; args.threads = true; args.lines = true; @@ -49,18 +68,21 @@ protected void convertChunk() { SentryStackTraceFactory stackTraceFactory = new SentryStackTraceFactory(Sentry.getGlobalScope().getOptions()); - converter = new JfrAsyncProfilerToSentryProfileConverter(jfrReader, args, stackTraceFactory); + ILogger logger = Sentry.getGlobalScope().getOptions().getLogger(); + converter = + new JfrAsyncProfilerToSentryProfileConverter(jfrReader, args, stackTraceFactory, logger); converter.convert(); } return converter.sentryProfile; } - private class ProfileEventVisitor extends AggregatedEventVisitor { + private class ProfileEventVisitor implements EventCollector.Visitor { private final @NotNull SentryProfile sentryProfile; private final @NotNull SentryStackTraceFactory stackTraceFactory; private final @NotNull JfrReader jfr; private final @NotNull Arguments args; + private final double ticksPerNanosecond; public ProfileEventVisitor( @NotNull SentryProfile sentryProfile, @@ -71,25 +93,37 @@ public ProfileEventVisitor( this.stackTraceFactory = stackTraceFactory; this.jfr = jfr; this.args = args; + ticksPerNanosecond = jfr.ticksPerSec / NANOS_PER_SECOND; } @Override - public void visit(Event event, long value) { - StackTrace stackTrace = jfr.stackTraces.get(event.stackTraceId); - long threadId = resolveThreadId(event.tid); - - if (stackTrace != null) { - if (args.threads) { - processThreadMetadata(event, threadId); - } + public void visit(Event event, long samples, long value) { + try { + StackTrace stackTrace = jfr.stackTraces.get(event.stackTraceId); + long threadId = resolveThreadId(event.tid); - createSample(event, threadId); + if (stackTrace != null) { + if (args.threads) { + processThreadMetadata(event, threadId); + } - buildStackTraceAndFrames(stackTrace); + processSampleWithStack(event, threadId, stackTrace); + } + } catch (Exception e) { + logger.log(SentryLevel.WARNING, "Failed to process JFR event " + event, e); } } + private long resolveThreadId(int eventId) { + Long javaThreadId = jfr.javaThreads.get(eventId); + return javaThreadId != null ? javaThreadId : UNKNOWN_THREAD_ID; + } + private void processThreadMetadata(Event event, long threadId) { + if (threadId == UNKNOWN_THREAD_ID) { + return; + } + final String threadName = getPlainThreadName(event.tid); sentryProfile .getThreadMetadata() @@ -103,9 +137,41 @@ private void processThreadMetadata(Event event, long threadId) { }); } - private void buildStackTraceAndFrames(StackTrace stackTrace) { - List stack = new ArrayList<>(); - int currentFrame = sentryProfile.getFrames().size(); + private void processSampleWithStack(Event event, long threadId, StackTrace stackTrace) { + int stackIndex = addStackTrace(stackTrace); + + SentrySample sample = new SentrySample(); + sample.setTimestamp(calculateTimestamp(event)); + sample.setThreadId(String.valueOf(threadId)); + sample.setStackId(stackIndex); + + sentryProfile.getSamples().add(sample); + } + + private double calculateTimestamp(Event event) { + long nanosFromStart = (long) ((event.time - jfr.chunkStartTicks) / ticksPerNanosecond); + + long timeNs = jfr.chunkStartNanos + nanosFromStart; + + return DateUtils.nanosToSeconds(timeNs); + } + + private int addStackTrace(StackTrace stackTrace) { + List callStack = createFramesAndCallStack(stackTrace); + + Integer existingIndex = stackDeduplicationMap.get(callStack); + if (existingIndex != null) { + return existingIndex; + } + + int stackIndex = sentryProfile.getStacks().size(); + sentryProfile.getStacks().add(callStack); + stackDeduplicationMap.put(callStack, stackIndex); + return stackIndex; + } + + private List createFramesAndCallStack(StackTrace stackTrace) { + List callStack = new ArrayList<>(); long[] methods = stackTrace.methods; byte[] types = stackTrace.types; @@ -113,18 +179,30 @@ private void buildStackTraceAndFrames(StackTrace stackTrace) { for (int i = 0; i < methods.length; i++) { StackTraceElement element = getStackTraceElement(methods[i], types[i], locations[i]); - if (element.isNativeMethod()) { + if (element.isNativeMethod() || isNativeFrame(types[i])) { continue; } SentryStackFrame frame = createStackFrame(element); - sentryProfile.getFrames().add(frame); + int frameIndex = getOrAddFrame(frame); + callStack.add(frameIndex); + } + + return callStack; + } + + // Get existing frame index or add new frame and return its index + private int getOrAddFrame(SentryStackFrame frame) { + Integer existingIndex = frameDeduplicationMap.get(frame); - stack.add(currentFrame); - currentFrame++; + if (existingIndex != null) { + return existingIndex; } - sentryProfile.getStacks().add(stack); + int newIndex = sentryProfile.getFrames().size(); + sentryProfile.getFrames().add(frame); + frameDeduplicationMap.put(frame, newIndex); + return newIndex; } private SentryStackFrame createStackFrame(StackTraceElement element) { @@ -176,24 +254,6 @@ private boolean isRegularClassWithoutPackage(String className) { return !className.startsWith("["); } - private void createSample(Event event, long threadId) { - int stackId = sentryProfile.getStacks().size(); - SentrySample sample = new SentrySample(); - - // Calculate timestamp from JFR event time - long nsFromStart = - (event.time - jfr.chunkStartTicks) - * JfrAsyncProfilerToSentryProfileConverter.NANOS_PER_SECOND - / jfr.ticksPerSec; - long timeNs = jfr.chunkStartNanos + nsFromStart; - sample.setTimestamp(DateUtils.nanosToSeconds(timeNs)); - - sample.setThreadId(String.valueOf(threadId)); - sample.setStackId(stackId); - - sentryProfile.getSamples().add(sample); - } - private boolean shouldMarkAsSystemFrame(StackTraceElement element, String className) { return element.isNativeMethod() || className.isEmpty(); } @@ -201,11 +261,5 @@ private boolean shouldMarkAsSystemFrame(StackTraceElement element, String classN private @Nullable Integer extractLineNumber(StackTraceElement element) { return element.getLineNumber() != 0 ? element.getLineNumber() : null; } - - private long resolveThreadId(int eventThreadId) { - return jfr.threads.get(eventThreadId) != null - ? jfr.javaThreads.get(eventThreadId) - : eventThreadId; - } } } diff --git a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/convert/NonAggregatingEventCollector.java b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/convert/NonAggregatingEventCollector.java new file mode 100644 index 00000000000..ce9dcaebf36 --- /dev/null +++ b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/convert/NonAggregatingEventCollector.java @@ -0,0 +1,37 @@ +package io.sentry.asyncprofiler.convert; + +import io.sentry.asyncprofiler.vendor.asyncprofiler.jfr.event.Event; +import io.sentry.asyncprofiler.vendor.asyncprofiler.jfr.event.EventCollector; +import java.util.ArrayList; +import java.util.List; + +public final class NonAggregatingEventCollector implements EventCollector { + final List events = new ArrayList<>(); + + @Override + public void collect(Event e) { + events.add(e); + } + + @Override + public void beforeChunk() { + // No-op + } + + @Override + public void afterChunk() { + // No-op + } + + @Override + public boolean finish() { + return false; + } + + @Override + public void forEach(Visitor visitor) { + for (Event event : events) { + visitor.visit(event, event.samples(), event.value()); + } + } +} diff --git a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfiler.java b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfiler.java index e76adfa769f..6b45d568394 100644 --- a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfiler.java +++ b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfiler.java @@ -24,7 +24,6 @@ import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.SentryRandom; import java.io.File; -import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -52,15 +51,13 @@ public final class JavaContinuousProfiler private @Nullable Future stopFuture; private final @NotNull List payloadBuilders = new ArrayList<>(); private @NotNull SentryId profilerId = SentryId.EMPTY_ID; - private @NotNull SentryId chunkId = SentryId.EMPTY_ID; private final @NotNull AtomicBoolean isClosed = new AtomicBoolean(false); private @NotNull SentryDate startProfileChunkTimestamp = new SentryNanotimeDate(); private @NotNull String filename = ""; - private final @NotNull AsyncProfiler profiler; + private @NotNull AsyncProfiler profiler; private volatile boolean shouldSample = true; - private boolean shouldStop = false; private boolean isSampled = false; private int rootSpanCounter = 0; @@ -71,26 +68,47 @@ public JavaContinuousProfiler( final @NotNull ILogger logger, final @Nullable String profilingTracesDirPath, final int profilingTracesHz, - final @NotNull ISentryExecutorService executorService) { + final @NotNull ISentryExecutorService executorService) + throws Exception { this.logger = logger; this.profilingTracesDirPath = profilingTracesDirPath; this.profilingTracesHz = profilingTracesHz; this.executorService = executorService; + initializeProfiler(); + } + + private void initializeProfiler() throws Exception { this.profiler = AsyncProfiler.getInstance(); + // Check version to verify profiler is working + String version = profiler.execute("version"); + logger.log(SentryLevel.DEBUG, "AsyncProfiler initialized successfully. Version: " + version); } private boolean init() { - // We initialize it only once if (isInitialized) { return true; } isInitialized = true; + if (profilingTracesDirPath == null) { logger.log( SentryLevel.WARNING, "Disabling profiling because no profiling traces dir path is defined in options."); return false; } + + File profileDir = new File(profilingTracesDirPath); + + if (!profileDir.canWrite() || !profileDir.exists()) { + logger.log( + SentryLevel.WARNING, + "Disabling profiling because traces directory is not writable or does not exist: %s (writable=%b, exists=%b)", + profilingTracesDirPath, + profileDir.canWrite(), + profileDir.exists()); + return false; + } + if (profilingTracesHz <= 0) { logger.log( SentryLevel.WARNING, @@ -101,7 +119,6 @@ private boolean init() { return true; } - @SuppressWarnings("ReferenceEquality") @Override public void startProfiler( final @NotNull ProfileLifecycle profileLifecycle, @@ -141,6 +158,8 @@ public void startProfiler( logger.log(SentryLevel.DEBUG, "Started Profiler."); start(); } + } catch (Exception e) { + logger.log(SentryLevel.ERROR, "Error starting profiler: ", e); } } @@ -159,7 +178,6 @@ private void initScopes() { private void start() { initScopes(); - // Let's initialize trace folder and profiling interval if (!init()) { return; } @@ -179,20 +197,26 @@ private void start() { } else { startProfileChunkTimestamp = new SentryNanotimeDate(); } + filename = profilingTracesDirPath + File.separator + SentryUUID.generateSentryId() + ".jfr"; - String startData = null; + + File jfrFile = new File(filename); + try { final String profilingIntervalMicros = String.format("%dus", (int) SECONDS.toMicros(1) / profilingTracesHz); + // Example command: start,jfr,event=wall,interval=9900us,file=/path/to/trace.jfr final String command = - String.format("start,jfr,wall=%s,file=%s", profilingIntervalMicros, filename); - System.out.println(command); - startData = profiler.execute(command); + String.format( + "start,jfr,event=wall,interval=%s,file=%s", profilingIntervalMicros, filename); + + profiler.execute(command); + } catch (Exception e) { logger.log(SentryLevel.ERROR, "Failed to start profiling: ", e); - } - // check if profiling started - if (startData == null) { + filename = ""; + // Try to clean up the file if it was created + safelyRemoveFile(jfrFile); return; } @@ -202,10 +226,6 @@ private void start() { profilerId = new SentryId(); } - if (chunkId == SentryId.EMPTY_ID) { - chunkId = new SentryId(); - } - try { stopFuture = executorService.schedule(() -> stop(true), MAX_CHUNK_DURATION_MILLIS); } catch (RejectedExecutionException e) { @@ -213,7 +233,8 @@ private void start() { SentryLevel.ERROR, "Failed to schedule profiling chunk finish. Did you call Sentry.close()?", e); - shouldStop = true; + // If we can't schedule the auto-stop, stop immediately without restart + stop(false); } } @@ -232,10 +253,12 @@ public void stopProfiler(final @NotNull ProfileLifecycle profileLifecycle) { if (rootSpanCounter < 0) { rootSpanCounter = 0; } - shouldStop = true; + // Stop immediately without restart + stop(false); break; case MANUAL: - shouldStop = true; + // Stop immediately without restart + stop(false); break; } } @@ -249,57 +272,55 @@ private void stop(final boolean restartProfiler) { // check if profiler was created and it's running if (!isRunning) { // When the profiler is stopped due to an error (e.g. offline or rate limited), reset the - // ids + // id profilerId = SentryId.EMPTY_ID; - chunkId = SentryId.EMPTY_ID; return; } - String endData = null; + File jfrFile = new File(filename); + try { - endData = profiler.execute("stop,jfr"); - } catch (IOException e) { - throw new RuntimeException(e); + profiler.execute("stop,jfr"); + } catch (Exception e) { + logger.log(SentryLevel.ERROR, "Error stopping profiler, attempting cleanup: ", e); + // Clean up file if it exists + safelyRemoveFile(jfrFile); } - // check if profiler end successfully - if (endData == null) { - logger.log( - SentryLevel.ERROR, - "An error occurred while collecting a profile chunk, and it won't be sent."); - } else { - // The scopes can be null if the profiler is started before the SDK is initialized (app - // start profiling), meaning there's no scopes to send the chunks. In that case, we store - // the data in a list and send it when the next chunk is finished. + // The scopes can be null if the profiler is started before the SDK is initialized (app + // start profiling), meaning there's no scopes to send the chunks. In that case, we store + // the data in a list and send it when the next chunk is finished. + if (jfrFile.exists() && jfrFile.canRead() && jfrFile.length() > 0) { try (final @NotNull ISentryLifecycleToken ignored2 = payloadLock.acquire()) { - File jfrFile = new File(filename); - // TODO: should we add deleteOnExit() here to let the JVM clean up the file? - // as in `Sentry.java` `initJvmContinuousProfiling` each time we start the profiler we - // create a new - // temp directory/file that we can't cleanup on restart. Unless the user sets - // `profiling-traces-dir-path` manually - // jfrFile.deleteOnExit(); + jfrFile.deleteOnExit(); payloadBuilders.add( new ProfileChunk.Builder( profilerId, - chunkId, + new SentryId(), new HashMap<>(), jfrFile, startProfileChunkTimestamp, - ProfileChunk.Platform.JAVA)); + ProfileChunk.PLATFORM_JAVA)); } + } else { + logger.log( + SentryLevel.WARNING, + "JFR file is invalid or empty: exists=%b, readable=%b, size=%d", + jfrFile.exists(), + jfrFile.canRead(), + jfrFile.length()); + safelyRemoveFile(jfrFile); } + // Always clean up state, even if stop failed isRunning = false; - // A chunk is finished. Next chunk will have a different id. - chunkId = SentryId.EMPTY_ID; filename = ""; if (scopes != null) { sendChunks(scopes, scopes.getOptions()); } - if (restartProfiler && !shouldStop) { + if (restartProfiler) { logger.log(SentryLevel.DEBUG, "Profile chunk finished. Starting a new one."); start(); } else { @@ -307,6 +328,8 @@ private void stop(final boolean restartProfiler) { profilerId = SentryId.EMPTY_ID; logger.log(SentryLevel.DEBUG, "Profile chunk finished."); } + } catch (Exception e) { + logger.log(SentryLevel.ERROR, "Error stopping profiler: ", e); } } @@ -319,9 +342,8 @@ public void reevaluateSampling() { public void close(final boolean isTerminating) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { rootSpanCounter = 0; - shouldStop = true; + stop(false); if (isTerminating) { - stop(false); isClosed.set(true); } } @@ -359,9 +381,21 @@ private void sendChunks(final @NotNull IScopes scopes, final @NotNull SentryOpti } } + private void safelyRemoveFile(File file) { + try { + if (file.exists()) { + file.delete(); + } + } catch (Exception e) { + logger.log(SentryLevel.INFO, "Failed to remove jfr file %s.", file.getAbsolutePath(), e); + } + } + @Override public boolean isRunning() { - return isRunning; + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + return isRunning && !filename.isEmpty(); + } } @VisibleForTesting diff --git a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/provider/AsyncProfilerContinuousProfilerProvider.java b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/provider/AsyncProfilerContinuousProfilerProvider.java index 49d83cffb3d..226cfc09084 100644 --- a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/provider/AsyncProfilerContinuousProfilerProvider.java +++ b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/provider/AsyncProfilerContinuousProfilerProvider.java @@ -3,6 +3,8 @@ import io.sentry.IContinuousProfiler; import io.sentry.ILogger; import io.sentry.ISentryExecutorService; +import io.sentry.NoOpContinuousProfiler; +import io.sentry.SentryLevel; import io.sentry.asyncprofiler.profiling.JavaContinuousProfiler; import io.sentry.profiling.JavaContinuousProfilerProvider; import io.sentry.profiling.JavaProfileConverterProvider; @@ -22,7 +24,15 @@ public final class AsyncProfilerContinuousProfilerProvider String profilingTracesDirPath, int profilingTracesHz, ISentryExecutorService executorService) { - return new JavaContinuousProfiler( - logger, profilingTracesDirPath, profilingTracesHz, executorService); + try { + return new JavaContinuousProfiler( + logger, profilingTracesDirPath, profilingTracesHz, executorService); + } catch (Exception e) { + logger.log( + SentryLevel.WARNING, + "Failed to initialize AsyncProfiler. Profiling will be disabled.", + e); + return NoOpContinuousProfiler.getInstance(); + } } } diff --git a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/vendor/asyncprofiler/README.md b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/vendor/asyncprofiler/README.md new file mode 100644 index 00000000000..733a69f1c3b --- /dev/null +++ b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/vendor/asyncprofiler/README.md @@ -0,0 +1,4 @@ +# Vendored AsyncProfiler code for converting JFR Files +- Vendored-in from commit https://github.com/async-profiler/async-profiler/tree/fe1bc66d4b6181413847f6bbe5c0db805f3e9194 +- Only the code related to JFR conversion is included. +- The `AsyncProfiler` itself is included as a dependency in the Maven project. diff --git a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/vendor/asyncprofiler/jfr/JfrReader.java b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/vendor/asyncprofiler/jfr/JfrReader.java index abc9a0024b4..98c8aa01b22 100644 --- a/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/vendor/asyncprofiler/jfr/JfrReader.java +++ b/sentry-async-profiler/src/main/java/io/sentry/asyncprofiler/vendor/asyncprofiler/jfr/JfrReader.java @@ -60,6 +60,8 @@ public final class JfrReader implements Closeable { public final Dictionary types = new Dictionary<>(); public final Map typesByName = new HashMap<>(); public final Dictionary threads = new Dictionary<>(); + // Maps thread IDs to Java thread IDs + // Change compared to original async-profiler JFR reader public final Dictionary javaThreads = new Dictionary<>(); public final Dictionary classes = new Dictionary<>(); public final Dictionary strings = new Dictionary<>(); diff --git a/sentry-async-profiler/src/test/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverterTest.kt b/sentry-async-profiler/src/test/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverterTest.kt index 4442553f66c..2b9c8ae1104 100644 --- a/sentry-async-profiler/src/test/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverterTest.kt +++ b/sentry-async-profiler/src/test/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverterTest.kt @@ -1,20 +1,30 @@ package io.sentry.asyncprofiler.convert +import io.sentry.DateUtils import io.sentry.ILogger import io.sentry.IProfileConverter import io.sentry.IScope import io.sentry.IScopes import io.sentry.Sentry import io.sentry.SentryOptions +import io.sentry.SentryStackTraceFactory import io.sentry.TracesSampler import io.sentry.asyncprofiler.provider.AsyncProfilerProfileConverterProvider +import io.sentry.protocol.profiling.SentryProfile import io.sentry.test.DeferredExecutorService -import java.nio.file.Files -import kotlin.io.path.absolutePathString -import kotlin.io.path.deleteIfExists +import java.io.IOException +import java.time.LocalDateTime +import java.time.ZoneOffset +import java.time.temporal.ChronoUnit +import java.util.* +import kotlin.io.path.Path +import kotlin.math.absoluteValue import kotlin.test.AfterTest import kotlin.test.BeforeTest -import one.profiler.AsyncProfiler +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertTrue import org.junit.Test import org.mockito.Mockito import org.mockito.kotlin.any @@ -32,6 +42,7 @@ class JfrAsyncProfilerToSentryProfileConverterTest { val mockedSentry = Mockito.mockStatic(Sentry::class.java) val mockLogger = mock() val mockTracesSampler = mock() + val mockStackTraceFactory = mock() val scopes: IScopes = mock() val scope: IScope = mock() @@ -42,10 +53,18 @@ class JfrAsyncProfilerToSentryProfileConverterTest { profilesSampleRate = 1.0 isDebug = true setLogger(mockLogger) + // Set in-app packages for testing + addInAppInclude("io.sentry") + addInAppInclude("com.example") } init { whenever(mockTracesSampler.sampleSessionProfile(any())).thenReturn(true) + // Setup default in-app behavior for stack trace factory + whenever(mockStackTraceFactory.isInApp(any())).thenAnswer { invocation -> + val className = invocation.getArgument(0) + className.startsWith("io.sentry") || className.startsWith("com.example") + } } fun getSut(optionConfig: ((options: SentryOptions) -> Unit) = {}): IProfileConverter? { @@ -63,6 +82,9 @@ class JfrAsyncProfilerToSentryProfileConverterTest { fixture.mockedSentry.`when` { Sentry.getCurrentScopes() }.thenReturn(fixture.scopes) fixture.mockedSentry.`when` { Sentry.getGlobalScope() }.thenReturn(fixture.scope) + + // Ensure the global scope returns proper options for the static converter method + whenever(fixture.scope.options).thenReturn(fixture.options) } @AfterTest @@ -72,14 +94,209 @@ class JfrAsyncProfilerToSentryProfileConverterTest { } @Test - fun `convert async profiler to sentry`() { - val profiler = AsyncProfiler.getInstance() - val file = Files.createTempFile("sentry-async-profiler-test", ".jfr") - val command = String.format("start,jfr,wall=%s,file=%s", "9900us", file.absolutePathString()) - profiler.execute(command) - profiler.execute("stop,jfr") - - fixture.getSut()!!.convertFromFile(file.toAbsolutePath()) - file.deleteIfExists() + fun `check number of samples for specific frame`() { + val file = Path(loadFile("async_profiler_test_sample.jfr")) + + val sentryProfile = fixture.getSut()!!.convertFromFile(file) + val tracingFilterFrame = + sentryProfile.frames.filter { + it.function == "slowFunction" && it.module == "io.sentry.samples.console.Main" + } + + val tracingFilterFrameIndexes = tracingFilterFrame.map { sentryProfile.frames.indexOf(it) } + val tracingFilterStacks = + sentryProfile.stacks.filter { it.any { inner -> tracingFilterFrameIndexes.contains(inner) } } + val tracingFilterStackIds = tracingFilterStacks.map { sentryProfile.stacks.indexOf(it) } + val tracingFilterSamples = + sentryProfile.samples.filter { tracingFilterStackIds.contains(it.stackId) } + + // Sample size base on 101 samples/sec and 5 sec of profiling + // So expected around 500 samples (with some margin) + assertTrue( + tracingFilterSamples.count() >= 500 && tracingFilterSamples.count() <= 600, + "Expected sample count between 500 and 600, but was ${tracingFilterSamples.count()}", + ) + } + + @Test + fun `check number of samples for specific thread`() { + val file = Path(loadFile("async_profiler_test_sample.jfr")) + + val sentryProfile = fixture.getSut()!!.convertFromFile(file) + val mainThread = + sentryProfile.threadMetadata.entries.firstOrNull { it.value.name == "main" }?.key + + val samples = sentryProfile.samples.filter { it.threadId == mainThread } + + // Sample size base on 101 samples/sec and 5 sec of profiling + // So expected around 500 samples (with some margin) + assertTrue( + samples.count() >= 500 && samples.count() <= 600, + "Expected sample count between 500 and 600, but was ${samples.count()}", + ) + } + + @Test + fun `check no duplicate frames`() { + val file = Path(loadFile("async_profiler_test_sample.jfr")) + val sentryProfile = fixture.getSut()!!.convertFromFile(file) + + val frameSet = sentryProfile.frames.toSet() + + assertEquals(frameSet.size, sentryProfile.frames.size) + } + + @Test + fun `convertFromFile with valid JFR returns populated SentryProfile`() { + val file = Path(loadFile("async_profiler_test_sample.jfr")) + + val sentryProfile = fixture.getSut()!!.convertFromFile(file) + + assertNotNull(sentryProfile) + assertValidSentryProfile(sentryProfile) + } + + @Test + fun `convertFromFile parses timestamps correctly`() { + val file = Path(loadFile("async_profiler_test_sample.jfr")) + + val sentryProfile = fixture.getSut()!!.convertFromFile(file) + + val samples = sentryProfile.samples + assertTrue(samples.isNotEmpty()) + + val minTimestamp = samples.minOf { it.timestamp } + val maxTimestamp = samples.maxOf { it.timestamp } + val sampleTimeStamp = + DateUtils.nanosToDate((maxTimestamp * 1000 * 1000 * 1000).toLong()).toInstant() + + // The sample was recorded around "2025-09-05T08:14:50" in UTC timezone + val referenceTimestamp = LocalDateTime.parse("2025-09-05T08:14:50").toInstant(ZoneOffset.UTC) + val between = ChronoUnit.MILLIS.between(sampleTimeStamp, referenceTimestamp).absoluteValue + + assertTrue(between < 5000, "Sample timestamp should be within 5s of reference timestamp") + assertTrue(maxTimestamp >= minTimestamp, "Max timestamp should be >= min timestamp") + assertTrue( + maxTimestamp - minTimestamp <= 10, + "There should be a max difference of <10s between min and max timestamp", + ) + } + + @Test + fun `convertFromFile extracts thread metadata correctly`() { + val file = Path(loadFile("async_profiler_test_sample.jfr")) + + val sentryProfile = fixture.getSut()!!.convertFromFile(file) + + val threadMetadata = sentryProfile.threadMetadata + val samples = sentryProfile.samples + + assertTrue(threadMetadata.isNotEmpty()) + + // Verify thread IDs in samples match thread metadata keys + val threadIdsFromSamples = samples.map { it.threadId }.toSet() + threadIdsFromSamples.forEach { threadId -> + assertTrue( + threadMetadata.containsKey(threadId), + "Thread metadata should contain thread ID: $threadId", + ) + } + + // Verify thread metadata has proper values + threadMetadata.forEach { (_, metadata) -> + assertNotNull(metadata.name, "Thread name should not be null") + assertEquals(0, metadata.priority, "Thread priority should be default (0)") + } + } + + @Test + fun `converter processes frames with complete information`() { + val file = Path(loadFile("async_profiler_test_sample.jfr")) + + val sentryProfile = fixture.getSut()!!.convertFromFile(file) + + val frames = sentryProfile.frames + assertTrue(frames.isNotEmpty()) + + // Find frames with complete information + val completeFrames = + frames.filter { frame -> + frame.function != null && + frame.module != null && + frame.lineno != null && + frame.filename != null + } + + assertTrue(completeFrames.isNotEmpty(), "Should have frames with complete information") + } + + @Test + fun `converter marks in-app frames correctly`() { + val file = Path(loadFile("async_profiler_test_sample.jfr")) + + val sentryProfile = fixture.getSut()!!.convertFromFile(file) + + val frames = sentryProfile.frames + + // Verify system packages are marked as not in-app + val systemFrames = + frames.filter { frame -> + frame.module?.let { + it.startsWith("java.") || it.startsWith("sun.") || it.startsWith("jdk.") + } ?: false + } + + val inappSentryFrames = + frames.filter { frame -> frame.module?.startsWith("io.sentry.") ?: false } + + val emptyModuleFrames = frames.filter { it.module.isNullOrEmpty() } + + // Verify system classes are not marked as in-app + systemFrames.forEach { frame -> + assertFalse(frame.isInApp ?: false, "System classes should not be marked as in app") + } + + // Verify sentry classes are marked as in-app + inappSentryFrames.forEach { frame -> + assertTrue(frame.isInApp ?: false, "Sentry classes should be marked as in app") + } + + // Verify empty class names are marked as not in-app + emptyModuleFrames.forEach { frame -> + assertFalse(frame.isInApp ?: true, "Empty module frame should not be in-app") + } + } + + @Test + fun `converter filters native methods`() { + val file = Path(loadFile("async_profiler_test_sample.jfr")) + + val sentryProfile = fixture.getSut()!!.convertFromFile(file) + + // Native methods should be filtered out during frame creation + // Verify no frames have characteristics of native methods + sentryProfile.frames.forEach { frame -> + // Native methods would have been skipped, so no frame should indicate native + assertTrue( + frame.filename?.isNotEmpty() == true || frame?.module?.isNotEmpty() == true, + "Frame should have some non-native information", + ) + } + } + + @Test(expected = IOException::class) + fun `convertFromFile with non-existent file throws IOException`() { + val nonExistentFile = Path("/non/existent/file.jfr") + + JfrAsyncProfilerToSentryProfileConverter.convertFromFileStatic(nonExistentFile) + } + + private fun loadFile(path: String): String = javaClass.classLoader!!.getResource(path)!!.file + + private fun assertValidSentryProfile(profile: SentryProfile) { + assertNotNull(profile.samples, "Samples should not be null") + assertNotNull(profile.frames, "Frames should not be null") + assertNotNull(profile.stacks, "Stacks should not be null") + assertNotNull(profile.threadMetadata, "Thread metadata should not be null") } } diff --git a/sentry-async-profiler/src/test/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfilerTest.kt b/sentry-async-profiler/src/test/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfilerTest.kt index d89a0a2a1aa..86f5d51fee2 100644 --- a/sentry-async-profiler/src/test/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfilerTest.kt +++ b/sentry-async-profiler/src/test/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfilerTest.kt @@ -20,6 +20,7 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue +import org.mockito.ArgumentMatchers.startsWith import org.mockito.Mockito import org.mockito.kotlin.any import org.mockito.kotlin.eq @@ -116,9 +117,6 @@ class JavaContinuousProfilerTest { assertTrue(profiler.isRunning) // We are scheduling the profiler to stop at the end of the chunk, so it should still be running profiler.stopProfiler(ProfileLifecycle.MANUAL) - assertTrue(profiler.isRunning) - // We run the executor service to trigger the chunk finish, and the profiler shouldn't restart - fixture.executor.runAll() assertFalse(profiler.isRunning) } @@ -274,7 +272,17 @@ class JavaContinuousProfilerTest { fixture.executor.runAll() // We assert that no trace files are written assertTrue(File(fixture.options.profilingTracesDirPath!!).list()!!.isEmpty()) - verify(fixture.mockLogger).log(eq(SentryLevel.ERROR), eq("Failed to start profiling: "), any()) + val expectedPath = fixture.options.profilingTracesDirPath + verify(fixture.mockLogger) + .log( + eq(SentryLevel.WARNING), + eq( + "Disabling profiling because traces directory is not writable or does not exist: %s (writable=%b, exists=%b)" + ), + eq(expectedPath), + eq(false), + eq(true), + ) } @Test @@ -314,28 +322,42 @@ class JavaContinuousProfilerTest { assertTrue(profiler.isRunning) // We run the executor service to trigger the profiler restart (chunk finish) fixture.executor.runAll() + // At this point the chunk has been submitted to the executor, but yet to be sent verify(fixture.scopes, never()).captureProfileChunk(any()) profiler.stopProfiler(ProfileLifecycle.MANUAL) - // We stop the profiler, which should send a chunk + // We stop the profiler, which should send both the first and last chunk fixture.executor.runAll() - verify(fixture.scopes).captureProfileChunk(any()) + verify(fixture.scopes, times(2)).captureProfileChunk(any()) } @Test fun `close without terminating stops all profiles after chunk is finished`() { val profiler = fixture.getSut() - profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) profiler.startProfiler(ProfileLifecycle.TRACE, fixture.mockTracesSampler) assertTrue(profiler.isRunning) - // We are scheduling the profiler to stop at the end of the chunk, so it should still be running + // We are closing the profiler, which should stop all profiles after the chunk is finished profiler.close(false) - assertTrue(profiler.isRunning) + assertFalse(profiler.isRunning) // However, close() already resets the rootSpanCounter assertEquals(0, profiler.rootSpanCounter) + } - // We run the executor service to trigger the chunk finish, and the profiler shouldn't restart + @Test + fun `profiler can be stopped and restarted`() { + val profiler = fixture.getSut() + profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) + assertTrue(profiler.isRunning) + + profiler.stopProfiler(ProfileLifecycle.MANUAL) fixture.executor.runAll() assertFalse(profiler.isRunning) + + profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) + fixture.executor.runAll() + + assertTrue(profiler.isRunning) + verify(fixture.mockLogger, never()) + .log(eq(SentryLevel.WARNING), startsWith("JFR file is invalid or empty"), any(), any(), any()) } @Test diff --git a/sentry-async-profiler/src/test/resources/async_profiler_test_sample.jfr b/sentry-async-profiler/src/test/resources/async_profiler_test_sample.jfr new file mode 100644 index 00000000000..35b4c768eac Binary files /dev/null and b/sentry-async-profiler/src/test/resources/async_profiler_test_sample.jfr differ diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java index 9b260cfc46c..bb374f4a517 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java @@ -117,6 +117,9 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri sentryParentSpanId, baggage); sentrySpan.getSpanContext().setOrigin(SentrySpanExporter.TRACE_ORIGIN); + sentrySpan + .getSpanContext() + .setProfilerId(scopes.getOptions().getContinuousProfiler().getProfilerId()); spanStorage.storeSentrySpan(spanContext, sentrySpan); } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 0b8f428b1df..6814fc1de76 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1935,15 +1935,17 @@ public final class io/sentry/PerformanceCollectionData { } public final class io/sentry/ProfileChunk : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public static final field PLATFORM_ANDROID Ljava/lang/String; + public static final field PLATFORM_JAVA Ljava/lang/String; public fun ()V - public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SentryId;Ljava/io/File;Ljava/util/Map;Ljava/lang/Double;Lio/sentry/ProfileChunk$Platform;Lio/sentry/SentryOptions;)V + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SentryId;Ljava/io/File;Ljava/util/Map;Ljava/lang/Double;Ljava/lang/String;Lio/sentry/SentryOptions;)V public fun equals (Ljava/lang/Object;)Z public fun getChunkId ()Lio/sentry/protocol/SentryId; public fun getClientSdk ()Lio/sentry/protocol/SdkVersion; public fun getDebugMeta ()Lio/sentry/protocol/DebugMeta; public fun getEnvironment ()Ljava/lang/String; public fun getMeasurements ()Ljava/util/Map; - public fun getPlatform ()Lio/sentry/ProfileChunk$Platform; + public fun getPlatform ()Ljava/lang/String; public fun getProfilerId ()Lio/sentry/protocol/SentryId; public fun getRelease ()Ljava/lang/String; public fun getSampledProfile ()Ljava/lang/String; @@ -1961,7 +1963,7 @@ public final class io/sentry/ProfileChunk : io/sentry/JsonSerializable, io/sentr } public final class io/sentry/ProfileChunk$Builder { - public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SentryId;Ljava/util/Map;Ljava/io/File;Lio/sentry/SentryDate;Lio/sentry/ProfileChunk$Platform;)V + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SentryId;Ljava/util/Map;Ljava/io/File;Lio/sentry/SentryDate;Ljava/lang/String;)V public fun build (Lio/sentry/SentryOptions;)Lio/sentry/ProfileChunk; } @@ -1987,21 +1989,6 @@ public final class io/sentry/ProfileChunk$JsonKeys { public fun ()V } -public final class io/sentry/ProfileChunk$Platform : java/lang/Enum, io/sentry/JsonSerializable { - public static final field ANDROID Lio/sentry/ProfileChunk$Platform; - public static final field JAVA Lio/sentry/ProfileChunk$Platform; - public fun apiName ()Ljava/lang/String; - public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V - public static fun valueOf (Ljava/lang/String;)Lio/sentry/ProfileChunk$Platform; - public static fun values ()[Lio/sentry/ProfileChunk$Platform; -} - -public final class io/sentry/ProfileChunk$Platform$Deserializer : io/sentry/JsonDeserializer { - public fun ()V - public fun deserialize (Lio/sentry/ObjectReader;Lio/sentry/ILogger;)Lio/sentry/ProfileChunk$Platform; - public synthetic fun deserialize (Lio/sentry/ObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; -} - public final class io/sentry/ProfileContext : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public static final field TYPE Ljava/lang/String; public fun ()V @@ -3980,6 +3967,7 @@ public class io/sentry/SpanContext : io/sentry/JsonSerializable, io/sentry/JsonU public fun getOrigin ()Ljava/lang/String; public fun getParentSpanId ()Lio/sentry/SpanId; public fun getProfileSampled ()Ljava/lang/Boolean; + public fun getProfilerId ()Lio/sentry/protocol/SentryId; public fun getSampled ()Ljava/lang/Boolean; public fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getSpanId ()Lio/sentry/SpanId; @@ -3994,6 +3982,7 @@ public class io/sentry/SpanContext : io/sentry/JsonSerializable, io/sentry/JsonU public fun setInstrumenter (Lio/sentry/Instrumenter;)V public fun setOperation (Ljava/lang/String;)V public fun setOrigin (Ljava/lang/String;)V + public fun setProfilerId (Lio/sentry/protocol/SentryId;)V public fun setSampled (Ljava/lang/Boolean;)V public fun setSampled (Ljava/lang/Boolean;Ljava/lang/Boolean;)V public fun setSamplingDecision (Lio/sentry/TracesSamplingDecision;)V @@ -5870,6 +5859,7 @@ public final class io/sentry/protocol/SentrySpan$JsonKeys { public final class io/sentry/protocol/SentryStackFrame : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public fun ()V + public fun equals (Ljava/lang/Object;)Z public fun getAbsPath ()Ljava/lang/String; public fun getAddrMode ()Ljava/lang/String; public fun getColno ()Ljava/lang/Integer; @@ -5891,6 +5881,7 @@ public final class io/sentry/protocol/SentryStackFrame : io/sentry/JsonSerializa public fun getSymbolAddr ()Ljava/lang/String; public fun getUnknown ()Ljava/util/Map; public fun getVars ()Ljava/util/Map; + public fun hashCode ()I public fun isInApp ()Ljava/lang/Boolean; public fun isNative ()Ljava/lang/Boolean; public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V diff --git a/sentry/src/main/java/io/sentry/ProfileChunk.java b/sentry/src/main/java/io/sentry/ProfileChunk.java index 91628ea25b7..0aa6b7e524d 100644 --- a/sentry/src/main/java/io/sentry/ProfileChunk.java +++ b/sentry/src/main/java/io/sentry/ProfileChunk.java @@ -11,7 +11,6 @@ import java.math.BigDecimal; import java.math.RoundingMode; import java.util.HashMap; -import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; @@ -21,12 +20,15 @@ @ApiStatus.Internal public final class ProfileChunk implements JsonUnknown, JsonSerializable { + public static final String PLATFORM_ANDROID = "android"; + public static final String PLATFORM_JAVA = "java"; + private @Nullable DebugMeta debugMeta; private @NotNull SentryId profilerId; private @NotNull SentryId chunkId; private @Nullable SdkVersion clientSdk; private final @NotNull Map measurements; - private @NotNull Platform platform; + private @NotNull String platform; private @NotNull String release; private @Nullable String environment; private @NotNull String version; @@ -48,7 +50,7 @@ public ProfileChunk() { new File("dummy"), new HashMap<>(), 0.0, - Platform.ANDROID, + PLATFORM_ANDROID, SentryOptions.empty()); } @@ -58,7 +60,7 @@ public ProfileChunk( final @NotNull File traceFile, final @NotNull Map measurements, final @NotNull Double timestamp, - final @NotNull Platform platform, + final @NotNull String platform, final @NotNull SentryOptions options) { this.profilerId = profilerId; this.chunkId = chunkId; @@ -97,7 +99,7 @@ public void setDebugMeta(final @Nullable DebugMeta debugMeta) { return environment; } - public @NotNull Platform getPlatform() { + public @NotNull String getPlatform() { return platform; } @@ -180,7 +182,7 @@ public static final class Builder { private final @NotNull File traceFile; private final double timestamp; - private final @NotNull Platform platform; + private final @NotNull String platform; public Builder( final @NotNull SentryId profilerId, @@ -188,7 +190,7 @@ public Builder( final @NotNull Map measurements, final @NotNull File traceFile, final @NotNull SentryDate timestamp, - final @NotNull Platform platform) { + final @NotNull String platform) { this.profilerId = profilerId; this.chunkId = chunkId; this.measurements = new ConcurrentHashMap<>(measurements); @@ -203,33 +205,6 @@ public ProfileChunk build(SentryOptions options) { } } - public enum Platform implements JsonSerializable { - ANDROID, - JAVA; - - public String apiName() { - return name().toLowerCase(Locale.ROOT); - } - - // JsonElementSerializer - - @Override - public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger logger) - throws IOException { - writer.value(apiName()); - } - - // JsonElementDeserializer - - public static final class Deserializer implements JsonDeserializer { - @Override - public @NotNull Platform deserialize(@NotNull ObjectReader reader, @NotNull ILogger logger) - throws Exception { - return Platform.valueOf(reader.nextString().toUpperCase(Locale.ROOT)); - } - } - } - // JsonSerializable public static final class JsonKeys { @@ -348,7 +323,7 @@ public static final class Deserializer implements JsonDeserializer } break; case JsonKeys.PLATFORM: - Platform platform = reader.nextOrNull(logger, new Platform.Deserializer()); + String platform = reader.nextStringOrNull(); if (platform != null) { data.platform = platform; } diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index 491da449ca1..1b44d3d80f2 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -938,17 +938,18 @@ public void flush(long timeoutMillis) { final @NotNull ISpanFactory spanFactory = maybeSpanFactory == null ? getOptions().getSpanFactory() : maybeSpanFactory; - // If continuous profiling is enabled in trace mode, let's start it. Profiler will sample on - // its own. + // If continuous profiling is enabled in trace mode, let's start it unless skipProfiling is + // true in TransactionOptions. + // Profiler will sample on its own. // Profiler is started before the transaction is created, so that the profiler id is available // when the transaction starts - if (samplingDecision.getSampled()) { - if (getOptions().isContinuousProfilingEnabled() - && getOptions().getProfileLifecycle() == ProfileLifecycle.TRACE) { - getOptions() - .getContinuousProfiler() - .startProfiler(ProfileLifecycle.TRACE, getOptions().getInternalTracesSampler()); - } + if (samplingDecision.getSampled() + && getOptions().isContinuousProfilingEnabled() + && getOptions().getProfileLifecycle() == ProfileLifecycle.TRACE + && transactionContext.getProfilerId().equals(SentryId.EMPTY_ID)) { + getOptions() + .getContinuousProfiler() + .startProfiler(ProfileLifecycle.TRACE, getOptions().getInternalTracesSampler()); } transaction = diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 0737e2417f7..0c9b07f7cd0 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -295,7 +295,7 @@ private static void ensureAttachmentSizeLimit( traceFile.getName())); } - if (ProfileChunk.Platform.JAVA == profileChunk.getPlatform()) { + if (ProfileChunk.PLATFORM_JAVA.equals(profileChunk.getPlatform())) { final IProfileConverter profileConverter = ProfilingServiceLoader.loadProfileConverter(); if (profileConverter != null) { @@ -303,7 +303,7 @@ private static void ensureAttachmentSizeLimit( final SentryProfile profile = profileConverter.convertFromFile(traceFile.toPath()); profileChunk.setSentryProfile(profile); - } catch (IOException e) { + } catch (Exception e) { throw new SentryEnvelopeException("Profile conversion failed"); } } @@ -340,7 +340,7 @@ private static void ensureAttachmentSizeLimit( "application-json", traceFile.getName(), null, - profileChunk.getPlatform().apiName(), + profileChunk.getPlatform(), null); // avoid method refs on Android due to some issues with older AGP setups diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 0496f407219..21d5088a181 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -83,8 +83,8 @@ public SentryTracer( setDefaultSpanData(root); - final @NotNull SentryId continuousProfilerId = - scopes.getOptions().getContinuousProfiler().getProfilerId(); + final @NotNull SentryId continuousProfilerId = getProfilerId(); + if (!continuousProfilerId.equals(SentryId.EMPTY_ID) && Boolean.TRUE.equals(isSampled())) { this.contexts.setProfile(new ProfileContext(continuousProfilerId)); } @@ -229,7 +229,7 @@ public void finish( } }); - // any un-finished childs will remain unfinished + // any un-finished children will remain unfinished // as relay takes care of setting the end-timestamp + deadline_exceeded // see // https://github.com/getsentry/relay/blob/40697d0a1c54e5e7ad8d183fc7f9543b94fe3839/relay-general/src/store/transactions/processor.rs#L374-L378 @@ -244,7 +244,8 @@ public void finish( .onTransactionFinish(this, performanceCollectionData.get(), scopes.getOptions()); } if (scopes.getOptions().isContinuousProfilingEnabled() - && scopes.getOptions().getProfileLifecycle() == ProfileLifecycle.TRACE) { + && scopes.getOptions().getProfileLifecycle() == ProfileLifecycle.TRACE + && root.getSpanContext().getProfilerId().equals(SentryId.EMPTY_ID)) { scopes.getOptions().getContinuousProfiler().stopProfiler(ProfileLifecycle.TRACE); } if (performanceCollectionData.get() != null) { @@ -543,8 +544,7 @@ private ISpan createChild( /** Sets the default data in the span, including profiler _id, thread id and thread name */ private void setDefaultSpanData(final @NotNull ISpan span) { final @NotNull IThreadChecker threadChecker = scopes.getOptions().getThreadChecker(); - final @NotNull SentryId profilerId = - scopes.getOptions().getContinuousProfiler().getProfilerId(); + final @NotNull SentryId profilerId = getProfilerId(); if (!profilerId.equals(SentryId.EMPTY_ID) && Boolean.TRUE.equals(span.isSampled())) { span.setData(SpanDataConvention.PROFILER_ID, profilerId.toString()); } @@ -553,6 +553,12 @@ private void setDefaultSpanData(final @NotNull ISpan span) { span.setData(SpanDataConvention.THREAD_NAME, threadChecker.getCurrentThreadName()); } + private @NotNull SentryId getProfilerId() { + return !root.getSpanContext().getProfilerId().equals(SentryId.EMPTY_ID) + ? root.getSpanContext().getProfilerId() + : scopes.getOptions().getContinuousProfiler().getProfilerId(); + } + @Override public @NotNull ISpan startChild(final @NotNull String operation) { return this.startChild(operation, (String) null); diff --git a/sentry/src/main/java/io/sentry/SpanContext.java b/sentry/src/main/java/io/sentry/SpanContext.java index 2999ea4a2b8..8bfc83e6458 100644 --- a/sentry/src/main/java/io/sentry/SpanContext.java +++ b/sentry/src/main/java/io/sentry/SpanContext.java @@ -55,6 +55,12 @@ public class SpanContext implements JsonUnknown, JsonSerializable { protected @Nullable Baggage baggage; + /** + * Set the profiler id associated with this transaction. If set to a non-empty id, this value will + * be sent to sentry instead of {@link SentryOptions#getContinuousProfiler} + */ + private @NotNull SentryId profilerId = SentryId.EMPTY_ID; + public SpanContext( final @NotNull String operation, final @Nullable TracesSamplingDecision samplingDecision) { this(new SentryId(), new SpanId(), operation, null, samplingDecision); @@ -304,6 +310,14 @@ public int hashCode() { return Objects.hash(traceId, spanId, parentSpanId, op, description, getStatus()); } + public @NotNull SentryId getProfilerId() { + return profilerId; + } + + public void setProfilerId(@NotNull SentryId profilerId) { + this.profilerId = profilerId; + } + // region JsonSerializable public static final class JsonKeys { diff --git a/sentry/src/main/java/io/sentry/profiling/ProfilingServiceLoader.java b/sentry/src/main/java/io/sentry/profiling/ProfilingServiceLoader.java index ec09de6075f..ef5a9373c42 100644 --- a/sentry/src/main/java/io/sentry/profiling/ProfilingServiceLoader.java +++ b/sentry/src/main/java/io/sentry/profiling/ProfilingServiceLoader.java @@ -5,6 +5,7 @@ import io.sentry.IProfileConverter; import io.sentry.ISentryExecutorService; import io.sentry.NoOpContinuousProfiler; +import io.sentry.ScopesAdapter; import io.sentry.SentryLevel; import java.util.Iterator; import java.util.ServiceLoader; @@ -49,16 +50,22 @@ public final class ProfilingServiceLoader { * @return an IProfileConverter instance or null if no provider is found */ public static @Nullable IProfileConverter loadProfileConverter() { + ILogger logger = ScopesAdapter.getInstance().getGlobalScope().getOptions().getLogger(); try { JavaProfileConverterProvider provider = loadSingleProvider(JavaProfileConverterProvider.class); if (provider != null) { + logger.log( + SentryLevel.DEBUG, + "Loaded profile converter from provider: %s", + provider.getClass().getName()); return provider.getProfileConverter(); } else { + logger.log(SentryLevel.DEBUG, "No profile converter provider found, returning null"); return null; } } catch (Throwable t) { - // Log error and return null to skip conversion + logger.log(SentryLevel.ERROR, "Failed to load profile converter provider, returning null", t); return null; } } diff --git a/sentry/src/main/java/io/sentry/protocol/SentryStackFrame.java b/sentry/src/main/java/io/sentry/protocol/SentryStackFrame.java index 0c2c725df2a..0b6d6571e91 100644 --- a/sentry/src/main/java/io/sentry/protocol/SentryStackFrame.java +++ b/sentry/src/main/java/io/sentry/protocol/SentryStackFrame.java @@ -11,6 +11,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -368,6 +369,63 @@ public static final class JsonKeys { public static final String POST_CONTEXT = "post_context"; } + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) return false; + SentryStackFrame that = (SentryStackFrame) o; + return Objects.equals(preContext, that.preContext) + && Objects.equals(postContext, that.postContext) + && Objects.equals(vars, that.vars) + && Objects.equals(framesOmitted, that.framesOmitted) + && Objects.equals(filename, that.filename) + && Objects.equals(function, that.function) + && Objects.equals(module, that.module) + && Objects.equals(lineno, that.lineno) + && Objects.equals(colno, that.colno) + && Objects.equals(absPath, that.absPath) + && Objects.equals(contextLine, that.contextLine) + && Objects.equals(inApp, that.inApp) + && Objects.equals(_package, that._package) + && Objects.equals(_native, that._native) + && Objects.equals(platform, that.platform) + && Objects.equals(imageAddr, that.imageAddr) + && Objects.equals(symbolAddr, that.symbolAddr) + && Objects.equals(instructionAddr, that.instructionAddr) + && Objects.equals(addrMode, that.addrMode) + && Objects.equals(symbol, that.symbol) + && Objects.equals(unknown, that.unknown) + && Objects.equals(rawFunction, that.rawFunction) + && Objects.equals(lock, that.lock); + } + + @Override + public int hashCode() { + return Objects.hash( + preContext, + postContext, + vars, + framesOmitted, + filename, + function, + module, + lineno, + colno, + absPath, + contextLine, + inApp, + _package, + _native, + platform, + imageAddr, + symbolAddr, + instructionAddr, + addrMode, + symbol, + unknown, + rawFunction, + lock); + } + @Override public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger logger) throws IOException { diff --git a/sentry/src/main/java/io/sentry/transport/RateLimiter.java b/sentry/src/main/java/io/sentry/transport/RateLimiter.java index 6c3460c8eb0..fc07e59c063 100644 --- a/sentry/src/main/java/io/sentry/transport/RateLimiter.java +++ b/sentry/src/main/java/io/sentry/transport/RateLimiter.java @@ -20,6 +20,8 @@ import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; @@ -171,8 +173,13 @@ private void markHintWhenSendingFailed(final @NotNull Hint hint, final boolean r */ @SuppressWarnings({"JdkObsolete", "JavaUtilDate"}) private boolean isRetryAfter(final @NotNull String itemType) { - final DataCategory dataCategory = getCategoryFromItemType(itemType); - return isActiveForCategory(dataCategory); + final List dataCategory = getCategoryFromItemType(itemType); + for (DataCategory category : dataCategory) { + if (isActiveForCategory(category)) { + return true; + } + } + return false; } /** @@ -181,33 +188,33 @@ private boolean isRetryAfter(final @NotNull String itemType) { * @param itemType the item itemType (eg event, session, attachment, ...) * @return the DataCategory eg (DataCategory.Error, DataCategory.Session, DataCategory.Attachment) */ - private @NotNull DataCategory getCategoryFromItemType(final @NotNull String itemType) { + private @NotNull List getCategoryFromItemType(final @NotNull String itemType) { switch (itemType) { case "event": - return DataCategory.Error; + return Collections.singletonList(DataCategory.Error); case "session": - return DataCategory.Session; + return Collections.singletonList(DataCategory.Session); case "attachment": - return DataCategory.Attachment; + return Collections.singletonList(DataCategory.Attachment); case "profile": - return DataCategory.Profile; + return Collections.singletonList(DataCategory.Profile); // When we send a profile chunk, we have to check for profile_chunk_ui rate limiting, - // because that's what relay returns to rate limit Android. When (if) we will implement JVM - // profiling we will have to check both rate limits. + // because that's what relay returns to rate limit Android. + // And ProfileChunk rate limiting for JVM. case "profile_chunk": - return DataCategory.ProfileChunkUi; + return Arrays.asList(DataCategory.ProfileChunkUi, DataCategory.ProfileChunk); case "transaction": - return DataCategory.Transaction; + return Collections.singletonList(DataCategory.Transaction); case "check_in": - return DataCategory.Monitor; + return Collections.singletonList(DataCategory.Monitor); case "replay_video": - return DataCategory.Replay; + return Collections.singletonList(DataCategory.Replay); case "feedback": - return DataCategory.Feedback; + return Collections.singletonList(DataCategory.Feedback); case "log": - return DataCategory.LogItem; + return Collections.singletonList(DataCategory.LogItem); default: - return DataCategory.Unknown; + return Collections.singletonList(DataCategory.Unknown); } } diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index d2b998aa94b..ba8b5507abb 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -968,7 +968,7 @@ class JsonSerializerTest { fixture.traceFile, HashMap(), 5.3, - ProfileChunk.Platform.ANDROID, + ProfileChunk.PLATFORM_ANDROID, fixture.options, ) val measurementNow = SentryNanotimeDate().nanoTimestamp() @@ -1127,7 +1127,7 @@ class JsonSerializerTest { assertEquals(SdkVersion("test", "1.2.3"), profileChunk.clientSdk) assertEquals(chunkId, profileChunk.chunkId) assertEquals("environment", profileChunk.environment) - assertEquals(ProfileChunk.Platform.ANDROID, profileChunk.platform) + assertEquals(ProfileChunk.PLATFORM_ANDROID, profileChunk.platform) assertEquals(profilerId, profileChunk.profilerId) assertEquals("release", profileChunk.release) assertEquals("sampled profile in base 64", profileChunk.sampledProfile) diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index c3e563f3a52..981a5d201f4 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -113,7 +113,7 @@ class SentryClientTest { profilingTraceFile, emptyMap(), 1.0, - ProfileChunk.Platform.ANDROID, + ProfileChunk.PLATFORM_ANDROID, sentryOptions, ) } diff --git a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt index 70b8b18de09..858ca43ab11 100644 --- a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt +++ b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt @@ -486,11 +486,11 @@ class SentryEnvelopeItemTest { val profileChunk = mock { whenever(it.traceFile).thenReturn(file) - whenever(it.platform).thenReturn(ProfileChunk.Platform.ANDROID) + whenever(it.platform).thenReturn("chunk platform") } val chunk = SentryEnvelopeItem.fromProfileChunk(profileChunk, mock()) - assertEquals("android", chunk.header.platform) + assertEquals("chunk platform", chunk.header.platform) } @Test @@ -499,7 +499,7 @@ class SentryEnvelopeItemTest { val profileChunk = mock { whenever(it.traceFile).thenReturn(file) - whenever(it.platform).thenReturn(ProfileChunk.Platform.ANDROID) + whenever(it.platform).thenReturn(ProfileChunk.PLATFORM_ANDROID) } file.writeBytes(fixture.bytes) @@ -514,7 +514,7 @@ class SentryEnvelopeItemTest { val profileChunk = mock { whenever(it.traceFile).thenReturn(file) - whenever(it.platform).thenReturn(ProfileChunk.Platform.ANDROID) + whenever(it.platform).thenReturn(ProfileChunk.PLATFORM_ANDROID) } file.writeBytes(fixture.bytes) @@ -531,7 +531,7 @@ class SentryEnvelopeItemTest { val profileChunk = mock { whenever(it.traceFile).thenReturn(file) - whenever(it.platform).thenReturn(ProfileChunk.Platform.ANDROID) + whenever(it.platform).thenReturn(ProfileChunk.PLATFORM_ANDROID) } assertFailsWith( @@ -547,7 +547,7 @@ class SentryEnvelopeItemTest { val profileChunk = mock { whenever(it.traceFile).thenReturn(file) - whenever(it.platform).thenReturn(ProfileChunk.Platform.ANDROID) + whenever(it.platform).thenReturn(ProfileChunk.PLATFORM_ANDROID) } file.writeBytes(fixture.bytes) file.setReadable(false) @@ -565,7 +565,7 @@ class SentryEnvelopeItemTest { val profileChunk = mock { whenever(it.traceFile).thenReturn(file) - whenever(it.platform).thenReturn(ProfileChunk.Platform.ANDROID) + whenever(it.platform).thenReturn(ProfileChunk.PLATFORM_ANDROID) } val chunk = SentryEnvelopeItem.fromProfileChunk(profileChunk, mock()) @@ -580,7 +580,7 @@ class SentryEnvelopeItemTest { val profileChunk = mock { whenever(it.traceFile).thenReturn(file) - whenever(it.platform).thenReturn(ProfileChunk.Platform.ANDROID) + whenever(it.platform).thenReturn(ProfileChunk.PLATFORM_ANDROID) } val exception = diff --git a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt index 5bea60408d3..ab997a26f76 100644 --- a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt +++ b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt @@ -456,7 +456,7 @@ class RateLimiterTest { } @Test - fun `drop profileChunk items as lost`() { + fun `drop profileChunkUi items as lost`() { val rateLimiter = fixture.getSUT() val profileChunkItem = SentryEnvelopeItem.fromProfileChunk(ProfileChunk(), fixture.serializer) @@ -481,6 +481,32 @@ class RateLimiterTest { verifyNoMoreInteractions(fixture.clientReportRecorder) } + @Test + fun `drop profileChunk items as lost`() { + val rateLimiter = fixture.getSUT() + + val profileChunkItem = SentryEnvelopeItem.fromProfileChunk(ProfileChunk(), fixture.serializer) + val attachmentItem = + SentryEnvelopeItem.fromAttachment( + fixture.serializer, + NoOpLogger.getInstance(), + Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), + 1000, + ) + val envelope = + SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(profileChunkItem, attachmentItem)) + + rateLimiter.updateRetryAfterLimits("60:profile_chunk:key", null, 1) + val result = rateLimiter.filter(envelope, Hint()) + + assertNotNull(result) + assertEquals(1, result.items.toList().size) + + verify(fixture.clientReportRecorder, times(1)) + .recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileChunkItem)) + verifyNoMoreInteractions(fixture.clientReportRecorder) + } + @Test fun `drop feedback items as lost`() { val rateLimiter = fixture.getSUT()