Merge 772c391 into e37359d

getsentry · Mar 7, 2024 · 7434738 · 7434738
2 parents e37359d + 772c391
commit 7434738
Show file tree

Hide file tree

Showing 6 changed files with 140 additions and 4 deletions.
diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api
@@ -461,6 +461,7 @@ public final class io/sentry/Hub : io/sentry/IHub, io/sentry/metrics/MetricsApi$
 	public fun setTransaction (Ljava/lang/String;)V
 	public fun setUser (Lio/sentry/protocol/User;)V
 	public fun startSession ()V
+	public fun startSpanForMetric (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan;
 	public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction;
 	public fun traceHeaders ()Lio/sentry/SentryTraceHeader;
 	public fun withScope (Lio/sentry/ScopeCallback;)V
@@ -3536,6 +3537,7 @@ public abstract interface class io/sentry/metrics/MetricsApi$IMetricsInterface {
 	public abstract fun getDefaultTagsForMetrics ()Ljava/util/Map;
 	public abstract fun getLocalMetricsAggregator ()Lio/sentry/metrics/LocalMetricsAggregator;
 	public abstract fun getMetricsAggregator ()Lio/sentry/IMetricsAggregator;
+	public abstract fun startSpanForMetric (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan;
 }
 
 public final class io/sentry/metrics/MetricsHelper {
@@ -3570,6 +3572,7 @@ public final class io/sentry/metrics/NoopMetricsAggregator : io/sentry/IMetricsA
 	public fun increment (Ljava/lang/String;DLio/sentry/MeasurementUnit;Ljava/util/Map;JILio/sentry/metrics/LocalMetricsAggregator;)V
 	public fun set (Ljava/lang/String;ILio/sentry/MeasurementUnit;Ljava/util/Map;JILio/sentry/metrics/LocalMetricsAggregator;)V
 	public fun set (Ljava/lang/String;Ljava/lang/String;Lio/sentry/MeasurementUnit;Ljava/util/Map;JILio/sentry/metrics/LocalMetricsAggregator;)V
+	public fun startSpanForMetric (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan;
 	public fun timing (Ljava/lang/String;Ljava/lang/Runnable;Lio/sentry/MeasurementUnit$Duration;Ljava/util/Map;ILio/sentry/metrics/LocalMetricsAggregator;)V
 }
 

diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java
@@ -978,6 +978,15 @@ private IScope buildLocalScope(
     return Collections.unmodifiableMap(tags);
   }
 
+  @Override
+  public @Nullable ISpan startSpanForMetric(@NotNull String op, @NotNull String description) {
+    final @Nullable ISpan span = getSpan();
+    if (span != null) {
+      return span.startChild(op, description);
+    }
+    return null;
+  }
+
   @Override
   public @Nullable LocalMetricsAggregator getLocalMetricsAggregator() {
     if (!options.isEnableSpanLocalMetricAggregation()) {

diff --git a/sentry/src/main/java/io/sentry/metrics/MetricsApi.java b/sentry/src/main/java/io/sentry/metrics/MetricsApi.java
@@ -1,6 +1,7 @@
 package io.sentry.metrics;
 
 import io.sentry.IMetricsAggregator;
+import io.sentry.ISpan;
 import io.sentry.MeasurementUnit;
 import java.util.Map;
 import org.jetbrains.annotations.ApiStatus;
@@ -19,6 +20,9 @@ public interface IMetricsInterface {
 
     @NotNull
     Map<String, String> getDefaultTagsForMetrics();
+
+    @Nullable
+    ISpan startSpanForMetric(final @NotNull String op, final @NotNull String description);
   }
 
   private final @NotNull MetricsApi.IMetricsInterface aggregator;
@@ -554,8 +558,20 @@ public void timing(
     final @Nullable LocalMetricsAggregator localMetricsAggregator =
         aggregator.getLocalMetricsAggregator();
 
-    aggregator
-        .getMetricsAggregator()
-        .timing(key, callback, durationUnit, enrichedTags, stackLevel, localMetricsAggregator);
+    final @Nullable ISpan span = aggregator.startSpanForMetric("metric.timing", key);
+    if (span != null && tags != null) {
+      for (final @NotNull Map.Entry<String, String> entry : tags.entrySet()) {
+        span.setTag(entry.getKey(), entry.getValue());
+      }
+    }
+    try {
+      aggregator
+          .getMetricsAggregator()
+          .timing(key, callback, durationUnit, enrichedTags, stackLevel, localMetricsAggregator);
+    } finally {
+      if (span != null) {
+        span.finish();
+      }
+    }
   }
 }
diff --git a/sentry/src/main/java/io/sentry/metrics/NoopMetricsAggregator.java b/sentry/src/main/java/io/sentry/metrics/NoopMetricsAggregator.java
@@ -1,6 +1,7 @@
 package io.sentry.metrics;
 
 import io.sentry.IMetricsAggregator;
+import io.sentry.ISpan;
 import io.sentry.MeasurementUnit;
 import java.io.IOException;
 import java.util.Collections;
@@ -102,4 +103,9 @@ public void close() throws IOException {}
   public @NotNull Map<String, String> getDefaultTagsForMetrics() {
     return Collections.emptyMap();
   }
+
+  @Override
+  public @Nullable ISpan startSpanForMetric(@NotNull String op, @NotNull String description) {
+    return null;
+  }
 }
diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt
@@ -2075,6 +2075,27 @@ class HubTest {
         assertNotNull(hub.localMetricsAggregator)
     }
 
+    @Test
+    fun `hub startSpanForMetric starts a child span`() {
+        val hub = generateHub {
+            it.isEnableMetrics = true
+            it.isEnableSpanLocalMetricAggregation = true
+            it.sampleRate = 1.0
+        } as Hub
+
+        val txn = hub.startTransaction(
+            "name.txn",
+            "op.txn",
+            TransactionOptions().apply { isBindToScope = true }
+        )
+
+        val span = hub.startSpanForMetric("op", "key")!!
+
+        assertEquals("op", span.spanContext.op)
+        assertEquals("key", span.spanContext.description)
+        assertEquals(span.spanContext.parentSpanId, txn.spanContext.spanId)
+    }
+
     private val dsnTest = "https://key@sentry.io/proj"
 
     private fun generateHub(optionsConfiguration: Sentry.OptionsConfiguration<SentryOptions>? = null): IHub {

diff --git a/sentry/src/test/java/io/sentry/metrics/MetricsApiTest.kt b/sentry/src/test/java/io/sentry/metrics/MetricsApiTest.kt
@@ -1,21 +1,32 @@
 package io.sentry.metrics
 
 import io.sentry.IMetricsAggregator
+import io.sentry.ISpan
+import io.sentry.MeasurementUnit
 import io.sentry.metrics.MetricsApi.IMetricsInterface
 import org.mockito.kotlin.any
 import org.mockito.kotlin.anyOrNull
 import org.mockito.kotlin.eq
 import org.mockito.kotlin.mock
+import org.mockito.kotlin.never
 import org.mockito.kotlin.verify
 import kotlin.test.Test
+import kotlin.test.assertEquals
 
 class MetricsApiTest {
 
     class Fixture {
         val aggregator = mock<IMetricsAggregator>()
         val localMetricsAggregator = mock<LocalMetricsAggregator>()
 
-        fun getSut(defaultTags: Map<String, String> = emptyMap()): MetricsApi {
+        var lastSpan: ISpan? = null
+        var lastOp: String? = null
+        var lastDescription: String? = null
+
+        fun getSut(
+            defaultTags: Map<String, String> = emptyMap(),
+            spanProvider: () -> ISpan? = { mock<ISpan>() }
+        ): MetricsApi {
             val localAggregator = localMetricsAggregator
 
             return MetricsApi(object : IMetricsInterface {
@@ -26,6 +37,13 @@ class MetricsApiTest {
                 override fun getLocalMetricsAggregator(): LocalMetricsAggregator? = localAggregator
 
                 override fun getDefaultTagsForMetrics(): Map<String, String> = defaultTags
+
+                override fun startSpanForMetric(op: String, description: String): ISpan? {
+                    lastOp = op
+                    lastDescription = description
+                    lastSpan = spanProvider()
+                    return lastSpan
+                }
             })
         }
     }
@@ -288,4 +306,67 @@ class MetricsApiTest {
             eq(fixture.localMetricsAggregator)
         )
     }
+
+    @Test
+    fun `timing starts and finishes a span`() {
+        val api = fixture.getSut()
+
+        api.timing("key") {
+            // no-op
+        }
+
+        assertEquals("metric.timing", fixture.lastOp)
+        assertEquals("key", fixture.lastDescription)
+
+        verify(fixture.lastSpan!!).finish()
+    }
+
+    @Test
+    fun `timing applies metric tags as span tags`() {
+        val span = mock<ISpan>()
+        val api = fixture.getSut(
+            spanProvider = {
+                span
+            },
+            defaultTags = mapOf(
+                "release" to "1.0"
+            )
+        )
+        // when timing is called
+        api.timing("key", {
+            // no-op
+        }, MeasurementUnit.Duration.NANOSECOND, mapOf("a" to "b"))
+
+        // the last span should have the metric tags, without the default ones
+        verify(fixture.lastSpan!!, never()).setTag("release", "1.0")
+        verify(fixture.lastSpan!!).setTag("a", "b")
+    }
+
+    @Test
+    fun `if timing throws an exception, span still finishes`() {
+        val api = fixture.getSut()
+
+        try {
+            api.timing("key") {
+                throw IllegalStateException()
+            }
+        } catch (e: IllegalStateException) {
+            // ignored
+        }
+
+        assertEquals("metric.timing", fixture.lastOp)
+        assertEquals("key", fixture.lastDescription)
+        verify(fixture.lastSpan!!).finish()
+    }
+
+    @Test
+    fun `if timing does retrieve a null span, it still works`() {
+        val api = fixture.getSut(
+            spanProvider = { null }
+        )
+        api.timing("key") {
+            // no-op
+        }
+        // no crash
+    }
 }