Merge branch 'main' into 8.x.x

# Conflicts: # CHANGELOG.md # sentry/src/main/java/io/sentry/SentryTracer.java
getsentry · May 27, 2024 · 337f79a · 337f79a
2 parents 5a3f296 + 619c9b9
commit 337f79a
Show file tree

Hide file tree

Showing 9 changed files with 90 additions and 79 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,15 @@
 # Changelog
 
+## Unreleased
+
+### Features
+
+- Publish Gradle module metadata ([#3422](https://github.com/getsentry/sentry-java/pull/3422))
+
+### Fixes
+
+- Fix faulty `span.frame_delay` calculation for early app start spans ([#3427](https://github.com/getsentry/sentry-java/pull/3427))
+
 ## 8.0.0-alpha.1
 
 Version 8 of the Sentry Android/Java SDK brings a variety of features and fixes. The most notable changes are:

diff --git a/build.gradle.kts b/build.gradle.kts
@@ -160,16 +160,7 @@ subprojects {
             if (this@subprojects.name.contains("-compose")) {
                 this.configureForMultiplatform(this@subprojects)
             } else {
-                this.getByName("main").contents {
-                    // non android modules
-                    from("build${sep}libs")
-                    from("build${sep}publications${sep}maven")
-                    // android modules
-                    from("build${sep}outputs${sep}aar") {
-                        include("*-release*")
-                    }
-                    from("build${sep}publications${sep}release")
-                }
+                this.configureForJvm(this@subprojects)
             }
             // craft only uses zip archives
             this.forEach { dist ->

diff --git a/buildSrc/src/main/java/Publication.kt b/buildSrc/src/main/java/Publication.kt
@@ -10,9 +10,12 @@ private object Consts {
 // configure distZip tasks for multiplatform
 fun DistributionContainer.configureForMultiplatform(project: Project) {
     val sep = File.separator
+    val version = project.properties["versionName"].toString()
 
     this.maybeCreate("android").contents {
-        from("build${sep}publications${sep}androidRelease")
+        from("build${sep}publications${sep}androidRelease") {
+            renameModule(project.name, "android", version = version)
+        }
         from("build${sep}outputs${sep}aar") {
             include("*-release*")
             rename {
@@ -25,7 +28,9 @@ fun DistributionContainer.configureForMultiplatform(project: Project) {
         }
     }
     this.getByName("main").contents {
-        from("build${sep}publications${sep}kotlinMultiplatform")
+        from("build${sep}publications${sep}kotlinMultiplatform") {
+            renameModule(project.name, version = version)
+        }
         from("build${sep}kotlinToolingMetadata")
         from("build${sep}libs") {
             include("*compose-kotlin*")
@@ -39,7 +44,9 @@ fun DistributionContainer.configureForMultiplatform(project: Project) {
     }
     this.maybeCreate("desktop").contents {
         // kotlin multiplatform modules
-        from("build${sep}publications${sep}desktop")
+        from("build${sep}publications${sep}desktop") {
+            renameModule(project.name, "desktop", version = version)
+        }
         from("build${sep}libs") {
             include("*desktop*")
             withJavadoc(renameTo = "compose-desktop")
@@ -53,6 +60,26 @@ fun DistributionContainer.configureForMultiplatform(project: Project) {
     project.tasks.getByName("distZip").finalizedBy(*platformDists)
 }
 
+fun DistributionContainer.configureForJvm(project: Project) {
+    val sep = File.separator
+    val version = project.properties["versionName"].toString()
+
+    this.getByName("main").contents {
+        // non android modules
+        from("build${sep}libs")
+        from("build${sep}publications${sep}maven") {
+            renameModule(project.name, version = version)
+        }
+        // android modules
+        from("build${sep}outputs${sep}aar") {
+            include("*-release*")
+        }
+        from("build${sep}publications${sep}release") {
+            renameModule(project.name, version = version)
+        }
+    }
+}
+
 private fun CopySpec.withJavadoc(renameTo: String = "compose") {
     include("*javadoc*")
     rename {
@@ -63,3 +90,13 @@ private fun CopySpec.withJavadoc(renameTo: String = "compose") {
         }
     }
 }
+
+private fun CopySpec.renameModule(projectName: String, renameTo: String = "", version: String) {
+    var target = ""
+    if (renameTo.isNotEmpty()) {
+        target = "-$renameTo"
+    }
+    rename {
+        it.replace("module.json", "$projectName$target-$version.module")
+    }
+}
diff --git a/sentry-android-core/build.gradle.kts b/sentry-android-core/build.gradle.kts
@@ -77,6 +77,7 @@ dependencies {
     compileOnly(projects.sentryAndroidFragment)
     compileOnly(projects.sentryAndroidTimber)
     compileOnly(projects.sentryCompose)
+    compileOnly(projects.sentryComposeHelper)
 
     // lifecycle processor, session tracking
     implementation(Config.Libs.lifecycleProcess)

diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java
@@ -7,9 +7,7 @@
 import io.sentry.NoOpSpan;
 import io.sentry.NoOpTransaction;
 import io.sentry.SentryDate;
-import io.sentry.SentryLongDate;
 import io.sentry.SentryNanotimeDate;
-import io.sentry.SentryTracer;
 import io.sentry.SpanDataConvention;
 import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
 import io.sentry.protocol.MeasurementValue;
@@ -33,7 +31,7 @@ public class SpanFrameMetricsCollector
   // grow indefinitely in case of a long running span
   private static final int MAX_FRAMES_COUNT = 3600;
   private static final long ONE_SECOND_NANOS = TimeUnit.SECONDS.toNanos(1);
-  private static final SentryNanotimeDate UNIX_START_DATE = new SentryNanotimeDate(new Date(0), 0);
+  private static final SentryNanotimeDate EMPTY_NANO_TIME = new SentryNanotimeDate(new Date(0), 0);
 
   private final boolean enabled;
   private final @NotNull Object lock = new Object();
@@ -125,7 +123,7 @@ public void onSpanFinished(final @NotNull ISpan span) {
       } else {
         // otherwise only remove old/irrelevant frames
         final @NotNull ISpan oldestSpan = runningSpans.first();
-        frames.headSet(new Frame(realNanos(oldestSpan.getStartDate()))).clear();
+        frames.headSet(new Frame(toNanoTime(oldestSpan.getStartDate()))).clear();
       }
     }
   }
@@ -138,22 +136,20 @@ private void captureFrameMetrics(@NotNull final ISpan span) {
         return;
       }
 
-      // Ignore spans with no finish date, but SentryTracer is not finished when executing this
-      // callback, yet, so in that case we use the current timestamp.
-      final @Nullable SentryDate spanFinishDate =
-          span instanceof SentryTracer ? new SentryNanotimeDate() : span.getFinishDate();
+      final @Nullable SentryDate spanFinishDate = span.getFinishDate();
       if (spanFinishDate == null) {
         return;
       }
-      final long spanEndNanos = realNanos(spanFinishDate);
 
-      final @NotNull SentryFrameMetrics frameMetrics = new SentryFrameMetrics();
-      final long spanStartNanos = realNanos(span.getStartDate());
-      if (spanStartNanos >= spanEndNanos) {
+      final long spanStartNanos = toNanoTime(span.getStartDate());
+      final long spanEndNanos = toNanoTime(spanFinishDate);
+      final long spanDurationNanos = spanEndNanos - spanStartNanos;
+      if (spanDurationNanos <= 0) {
         return;
       }
 
-      final long spanDurationNanos = spanEndNanos - spanStartNanos;
+      final @NotNull SentryFrameMetrics frameMetrics = new SentryFrameMetrics();
+
       long frameDurationNanos = lastKnownFrameDurationNanos;
 
       if (!frames.isEmpty()) {
@@ -199,11 +195,15 @@ private void captureFrameMetrics(@NotNull final ISpan span) {
       int totalFrameCount = frameMetrics.getTotalFrameCount();
 
       final long nextScheduledFrameNanos = frameMetricsCollector.getLastKnownFrameStartTimeNanos();
-      totalFrameCount +=
-          addPendingFrameDelay(
-              frameMetrics, frameDurationNanos, spanEndNanos, nextScheduledFrameNanos);
-      totalFrameCount += interpolateFrameCount(frameMetrics, frameDurationNanos, spanDurationNanos);
-
+      // nextScheduledFrameNanos might be -1 if no frames have been scheduled for drawing yet
+      // e.g. can happen during early app start
+      if (nextScheduledFrameNanos != -1) {
+        totalFrameCount +=
+            addPendingFrameDelay(
+                frameMetrics, frameDurationNanos, spanEndNanos, nextScheduledFrameNanos);
+        totalFrameCount +=
+            interpolateFrameCount(frameMetrics, frameDurationNanos, spanDurationNanos);
+      }
       final long frameDelayNanos =
           frameMetrics.getSlowFrameDelayNanos() + frameMetrics.getFrozenFrameDelayNanos();
       final double frameDelayInSeconds = frameDelayNanos / 1e9d;
@@ -305,19 +305,20 @@ private static int addPendingFrameDelay(
    * diff does ¯\_(ツ)_/¯
    *
    * @param date the input date
-   * @return a timestamp in nano precision
+   * @return a non-unix timestamp in nano precision, similar to {@link System#nanoTime()}.
    */
-  private static long realNanos(final @NotNull SentryDate date) {
-    // SentryNanotimeDate nanotime is based on System.nanotime(), like UNIX_START_DATE
+  private static long toNanoTime(final @NotNull SentryDate date) {
+    // SentryNanotimeDate nanotime is based on System.nanotime(), like EMPTY_NANO_TIME,
+    // thus diff will simply return the System.nanotime() value of date
     if (date instanceof SentryNanotimeDate) {
-      return date.diff(UNIX_START_DATE);
+      return date.diff(EMPTY_NANO_TIME);
     }
 
-    // SentryLongDate nanotime is based on current date converted to nanoseconds, which is a
-    // different order than frames based System.nanotime(). So we have to convert the nanotime of
-    // the SentryLongDate to a System.nanotime() compatible one.
-    return date.diff(new SentryLongDate(DateUtils.millisToNanos(System.currentTimeMillis())))
-        + System.nanoTime();
+    // e.g. SentryLongDate is unix time based - upscaled to nanos,
+    // we need to project it back to System.nanotime() format
+    long nowUnixInNanos = DateUtils.millisToNanos(System.currentTimeMillis());
+    long shiftInNanos = nowUnixInNanos - date.nanoTimestamp();
+    return System.nanoTime() - shiftInNanos;
   }
 
   private static class Frame implements Comparable<Frame> {

diff --git a/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts b/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts
@@ -97,6 +97,7 @@ dependencies {
     if (applySentryIntegrations) {
         implementation(projects.sentryAndroid)
         implementation(projects.sentryCompose)
+        implementation(projects.sentryComposeHelper)
     } else {
         implementation(projects.sentryAndroidCore)
     }

diff --git a/sentry-compose/build.gradle.kts b/sentry-compose/build.gradle.kts
@@ -1,6 +1,4 @@
 import com.android.build.gradle.internal.tasks.LibraryAarJarsTask
-import groovy.util.Node
-import groovy.util.NodeList
 import io.gitlab.arturbosch.detekt.Detekt
 import org.jetbrains.dokka.gradle.DokkaTask
 
@@ -44,7 +42,7 @@ kotlin {
                 compileOnly(compose.runtime)
                 compileOnly(compose.ui)
 
-                api(projects.sentryComposeHelper)
+                compileOnly(projects.sentryComposeHelper)
             }
         }
         val androidMain by getting {
@@ -149,32 +147,3 @@ dependencies {
 tasks.withType<LibraryAarJarsTask> {
     mainScopeClassFiles.setFrom(embedComposeHelperConfig)
 }
-
-// we embed the sentry-compose-helper classes to the same .jar above
-// so we need to exclude the dependency from the .pom publication and .module metadata
-configure<PublishingExtension> {
-    publications.withType(MavenPublication::class.java).all {
-        this.pom {
-            this.withXml {
-                (asNode().get("dependencies") as NodeList)
-                    .flatMap {
-                        if (it is Node) it.children() else NodeList()
-                    }
-                    .filterIsInstance<Node>()
-                    .filter { dependency ->
-                        val artifactIdNodes = dependency.get("artifactId") as NodeList
-                        artifactIdNodes.any {
-                            (it is Node && it.value().toString().contains("sentry-compose-helper"))
-                        }
-                    }
-                    .forEach { dependency ->
-                        dependency.parent().remove(dependency)
-                    }
-            }
-        }
-    }
-}
-
-tasks.withType<GenerateModuleMetadata> {
-    enabled = false
-}
diff --git a/sentry-samples/sentry-samples-android/build.gradle.kts b/sentry-samples/sentry-samples-android/build.gradle.kts
@@ -111,6 +111,7 @@ dependencies {
     implementation(projects.sentryAndroidFragment)
     implementation(projects.sentryAndroidTimber)
     implementation(projects.sentryCompose)
+    implementation(projects.sentryComposeHelper)
     implementation(Config.Libs.fragment)
     implementation(Config.Libs.timber)
 

diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java
@@ -200,6 +200,13 @@ public void finish(
     this.finishStatus = FinishStatus.finishing(status);
     if (!root.isFinished()
         && (!transactionOptions.isWaitForChildren() || hasAllChildrenFinished())) {
+
+      // any un-finished childs 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
+      root.finish(finishStatus.spanStatus, finishTimestamp);
+
       List<PerformanceCollectionData> performanceCollectionData = null;
       if (transactionPerformanceCollector != null) {
         performanceCollectionData = transactionPerformanceCollector.stop(this);
@@ -217,13 +224,6 @@ public void finish(
         performanceCollectionData.clear();
       }
 
-      // any un-finished childs 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
-
-      root.finish(finishStatus.spanStatus, finishTimestamp);
-
       scopes.configureScope(
           scope -> {
             scope.withTransaction(