From 80450234a1a2f8b222adefa32257e120c93efbef Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 3 May 2024 12:31:25 +0200 Subject: [PATCH] Add ttid/ttfd contribution flags (#3386) * Add ttid/ttfd contribution flags * Update Changelog * Add ui namespace * Update according to develop docs * consider main thread info for contribution flags --- CHANGELOG.md | 1 + .../PerformanceAndroidEventProcessor.java | 70 +++++ .../PerformanceAndroidEventProcessorTest.kt | 277 ++++++++++++++++++ sentry/api/sentry.api | 3 + .../java/io/sentry/SpanDataConvention.java | 2 + .../java/io/sentry/protocol/SentrySpan.java | 6 +- 6 files changed, 358 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d54fafd87a..f72b0fad7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Add start_type to app context ([#3379](https://github.com/getsentry/sentry-java/pull/3379)) +- Add ttid/ttfd contribution flags ([#3386](https://github.com/getsentry/sentry-java/pull/3386)) ### Fixes diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index 5e80f91839..00ba9122e7 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -116,6 +116,8 @@ public SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) { appContext.setStartType(appStartType); } + setContributingFlags(transaction); + final SentryId eventId = transaction.getEventId(); final SpanContext spanContext = transaction.getContexts().getTrace(); @@ -135,6 +137,71 @@ public SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) { return transaction; } + private void setContributingFlags(SentryTransaction transaction) { + + @Nullable SentrySpan ttidSpan = null; + @Nullable SentrySpan ttfdSpan = null; + for (final @NotNull SentrySpan span : transaction.getSpans()) { + if (ActivityLifecycleIntegration.TTID_OP.equals(span.getOp())) { + ttidSpan = span; + } else if (ActivityLifecycleIntegration.TTFD_OP.equals(span.getOp())) { + ttfdSpan = span; + } + // once both are found we can early exit + if (ttidSpan != null && ttfdSpan != null) { + break; + } + } + + if (ttidSpan == null && ttfdSpan == null) { + return; + } + + for (final @NotNull SentrySpan span : transaction.getSpans()) { + // as ttid and ttfd spans are artificially created, we don't want to set the flags on them + if (span == ttidSpan || span == ttfdSpan) { + continue; + } + + // let's assume main thread, unless it's set differently + boolean spanOnMainThread = true; + final @Nullable Map spanData = span.getData(); + if (spanData != null) { + final @Nullable Object threadName = spanData.get(SpanDataConvention.THREAD_NAME); + spanOnMainThread = threadName == null || "main".equals(threadName); + } + + // for ttid, only main thread spans are relevant + final boolean withinTtid = + (ttidSpan != null) + && isTimestampWithinSpan(span.getStartTimestamp(), ttidSpan) + && spanOnMainThread; + + final boolean withinTtfd = + (ttfdSpan != null) && isTimestampWithinSpan(span.getStartTimestamp(), ttfdSpan); + + if (withinTtid || withinTtfd) { + @Nullable Map data = span.getData(); + if (data == null) { + data = new ConcurrentHashMap<>(); + span.setData(data); + } + if (withinTtid) { + data.put(SpanDataConvention.CONTRIBUTES_TTID, true); + } + if (withinTtfd) { + data.put(SpanDataConvention.CONTRIBUTES_TTFD, true); + } + } + } + } + + private static boolean isTimestampWithinSpan( + final double timestamp, final @NotNull SentrySpan target) { + return timestamp >= target.getStartTimestamp() + && (target.getTimestamp() == null || timestamp <= target.getTimestamp()); + } + private boolean hasAppStartSpan(final @NotNull SentryTransaction txn) { final @NotNull List spans = txn.getSpans(); for (final @NotNull SentrySpan span : spans) { @@ -253,6 +320,9 @@ private static SentrySpan timeSpanToSentrySpan( defaultSpanData.put(SpanDataConvention.THREAD_ID, Looper.getMainLooper().getThread().getId()); defaultSpanData.put(SpanDataConvention.THREAD_NAME, "main"); + defaultSpanData.put(SpanDataConvention.CONTRIBUTES_TTID, true); + defaultSpanData.put(SpanDataConvention.CONTRIBUTES_TTFD, true); + return new SentrySpan( span.getStartTimestampSecs(), span.getProjectedStopTimestampSecs(), diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt index d7bef488c2..4283326677 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -7,6 +7,7 @@ import io.sentry.IHub import io.sentry.MeasurementUnit import io.sentry.SentryTracer import io.sentry.SpanContext +import io.sentry.SpanDataConvention import io.sentry.SpanId import io.sentry.SpanStatus import io.sentry.TracesSamplingDecision @@ -519,6 +520,282 @@ class PerformanceAndroidEventProcessorTest { ) } + @Test + fun `adds ttid and ttfd contributing span data`() { + val sut = fixture.getSut() + + val context = TransactionContext("Activity", UI_LOAD_OP) + val tracer = SentryTracer(context, fixture.hub) + val tr = SentryTransaction(tracer) + + // given a ttid from 0.0 -> 1.0 + // and a ttfd from 0.0 -> 2.0 + val ttid = SentrySpan( + 0.0, + 1.0, + tr.contexts.trace!!.traceId, + SpanId(), + null, + ActivityLifecycleIntegration.TTID_OP, + "App Start", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + null + ) + + val ttfd = SentrySpan( + 0.0, + 2.0, + tr.contexts.trace!!.traceId, + SpanId(), + null, + ActivityLifecycleIntegration.TTFD_OP, + "App Start", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + null + ) + tr.spans.add(ttid) + tr.spans.add(ttfd) + + // and 3 spans + // one from 0.0 -> 0.5 + val ttidContrib = SentrySpan( + 0.0, + 0.5, + tr.contexts.trace!!.traceId, + SpanId(), + null, + "example.op", + "", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + null + ) + + // and another from 1.5 -> 3.5 + val ttfdContrib = SentrySpan( + 1.5, + 3.5, + tr.contexts.trace!!.traceId, + SpanId(), + null, + "example.op", + "", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + null + ) + + // and another from 2.1 -> 2.2 + val outsideSpan = SentrySpan( + 2.1, + 2.2, + tr.contexts.trace!!.traceId, + SpanId(), + null, + "example.op", + "", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + mutableMapOf( + "tag" to "value" + ) + ) + + tr.spans.add(ttidContrib) + tr.spans.add(ttfdContrib) + + // when the processor processes the txn + sut.process(tr, Hint()) + + // then the ttid/ttfd spans themselves should have no flags set + assertNull(ttid.data?.get(SpanDataConvention.CONTRIBUTES_TTID)) + assertNull(ttid.data?.get(SpanDataConvention.CONTRIBUTES_TTFD)) + + assertNull(ttfd.data?.get(SpanDataConvention.CONTRIBUTES_TTID)) + assertNull(ttfd.data?.get(SpanDataConvention.CONTRIBUTES_TTFD)) + + // then the first span should have ttid and ttfd contributing flags + assertTrue(ttidContrib.data?.get(SpanDataConvention.CONTRIBUTES_TTID) == true) + assertTrue(ttidContrib.data?.get(SpanDataConvention.CONTRIBUTES_TTFD) == true) + + // and the second one should contribute to ttfd only + assertNull(ttfdContrib.data?.get(SpanDataConvention.CONTRIBUTES_TTID)) + assertTrue(ttfdContrib.data?.get(SpanDataConvention.CONTRIBUTES_TTFD) == true) + + // and the third span should have no flags attached, as it's outside ttid/ttfd + assertNull(outsideSpan.data?.get(SpanDataConvention.CONTRIBUTES_TTID)) + assertNull(outsideSpan.data?.get(SpanDataConvention.CONTRIBUTES_TTFD)) + } + + @Test + fun `adds no ttid and ttfd contributing span data if txn contains no ttid or ttfd`() { + val sut = fixture.getSut() + + val context = TransactionContext("Activity", UI_LOAD_OP) + val tracer = SentryTracer(context, fixture.hub) + val tr = SentryTransaction(tracer) + + val span = SentrySpan( + 0.0, + 1.0, + tr.contexts.trace!!.traceId, + SpanId(), + null, + "example.op", + "", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + null + ) + + tr.spans.add(span) + + // when the processor processes the txn + sut.process(tr, Hint()) + + // the span should have no flags attached + assertNull(span.data?.get(SpanDataConvention.CONTRIBUTES_TTID)) + assertNull(span.data?.get(SpanDataConvention.CONTRIBUTES_TTFD)) + } + + @Test + fun `sets ttid and ttfd contributing flags according to span threads`() { + val sut = fixture.getSut() + + val context = TransactionContext("Activity", UI_LOAD_OP) + val tracer = SentryTracer(context, fixture.hub) + val tr = SentryTransaction(tracer) + + // given a ttid from 0.0 -> 1.0 + // and a ttfd from 0.0 -> 1.0 + val ttid = SentrySpan( + 0.0, + 1.0, + tr.contexts.trace!!.traceId, + SpanId(), + null, + ActivityLifecycleIntegration.TTID_OP, + "App Start", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + null + ) + + val ttfd = SentrySpan( + 0.0, + 1.0, + tr.contexts.trace!!.traceId, + SpanId(), + null, + ActivityLifecycleIntegration.TTFD_OP, + "App Start", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + null + ) + tr.spans.add(ttid) + tr.spans.add(ttfd) + + // one span with no thread info + val noThreadSpan = SentrySpan( + 0.0, + 0.5, + tr.contexts.trace!!.traceId, + SpanId(), + null, + "example.op", + "", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + null + ) + + // one span on the main thread + val mainThreadSpan = SentrySpan( + 0.0, + 0.5, + tr.contexts.trace!!.traceId, + SpanId(), + null, + "example.op", + "", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + mutableMapOf( + "thread.name" to "main" + ) + ) + + // and another one off the main thread + val backgroundThreadSpan = SentrySpan( + 0.0, + 0.5, + tr.contexts.trace!!.traceId, + SpanId(), + null, + "example.op", + "", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + mutableMapOf( + "thread.name" to "background" + ) + ) + + tr.spans.add(noThreadSpan) + tr.spans.add(mainThreadSpan) + tr.spans.add(backgroundThreadSpan) + + // when the processor processes the txn + sut.process(tr, Hint()) + + // then the span with no thread info + main thread span should contribute to ttid and ttfd + assertTrue(noThreadSpan.data?.get(SpanDataConvention.CONTRIBUTES_TTID) == true) + assertTrue(noThreadSpan.data?.get(SpanDataConvention.CONTRIBUTES_TTFD) == true) + + assertTrue(mainThreadSpan.data?.get(SpanDataConvention.CONTRIBUTES_TTID) == true) + assertTrue(mainThreadSpan.data?.get(SpanDataConvention.CONTRIBUTES_TTFD) == true) + + // and the background thread span only contributes to ttfd + assertNull(backgroundThreadSpan.data?.get(SpanDataConvention.CONTRIBUTES_TTID)) + assertTrue(backgroundThreadSpan.data?.get(SpanDataConvention.CONTRIBUTES_TTFD) == true) + } + private fun setAppStart(options: SentryAndroidOptions, coldStart: Boolean = true) { AppStartMetrics.getInstance().apply { appStartType = when (coldStart) { diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 827bd1a955..61cf316b6c 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -2740,6 +2740,8 @@ public final class io/sentry/SpanContext$JsonKeys { public abstract interface class io/sentry/SpanDataConvention { public static final field BLOCKED_MAIN_THREAD_KEY Ljava/lang/String; public static final field CALL_STACK_KEY Ljava/lang/String; + public static final field CONTRIBUTES_TTFD Ljava/lang/String; + public static final field CONTRIBUTES_TTID Ljava/lang/String; public static final field DB_NAME_KEY Ljava/lang/String; public static final field DB_SYSTEM_KEY Ljava/lang/String; public static final field FRAMES_DELAY Ljava/lang/String; @@ -4459,6 +4461,7 @@ public final class io/sentry/protocol/SentrySpan : io/sentry/JsonSerializable, i public fun getUnknown ()Ljava/util/Map; public fun isFinished ()Z public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V + public fun setData (Ljava/util/Map;)V public fun setUnknown (Ljava/util/Map;)V } diff --git a/sentry/src/main/java/io/sentry/SpanDataConvention.java b/sentry/src/main/java/io/sentry/SpanDataConvention.java index b96bf41e66..f8fb82c3c8 100644 --- a/sentry/src/main/java/io/sentry/SpanDataConvention.java +++ b/sentry/src/main/java/io/sentry/SpanDataConvention.java @@ -21,4 +21,6 @@ public interface SpanDataConvention { String FRAMES_SLOW = "frames.slow"; String FRAMES_FROZEN = "frames.frozen"; String FRAMES_DELAY = "frames.delay"; + String CONTRIBUTES_TTID = "ui.contributes_to_ttid"; + String CONTRIBUTES_TTFD = "ui.contributes_to_ttfd"; } diff --git a/sentry/src/main/java/io/sentry/protocol/SentrySpan.java b/sentry/src/main/java/io/sentry/protocol/SentrySpan.java index 4c91c8fa00..2be4411d44 100644 --- a/sentry/src/main/java/io/sentry/protocol/SentrySpan.java +++ b/sentry/src/main/java/io/sentry/protocol/SentrySpan.java @@ -40,7 +40,7 @@ public final class SentrySpan implements JsonUnknown, JsonSerializable { private final @Nullable String origin; private final @NotNull Map tags; - private final @Nullable Map data; + private @Nullable Map data; private final @NotNull Map measurements; private final @Nullable Map> metricsSummaries; @@ -159,6 +159,10 @@ public boolean isFinished() { return data; } + public void setData(final @Nullable Map data) { + this.data = data; + } + public @Nullable String getOrigin() { return origin; }