diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 6fcd4cc1b6c..b63894dba58 100644 --- 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 index 7dab81e766e..6b6da88f093 100644 --- 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 index 5b51f164af7..86896f466bd 100644 --- 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 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 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 index 4edabfb0c3b..ebc6c3d5037 100644 --- 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 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 index 1eb343d5de0..8fac963e703 100644 --- 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? = null): IHub { diff --git a/sentry/src/test/java/io/sentry/metrics/MetricsApiTest.kt b/sentry/src/test/java/io/sentry/metrics/MetricsApiTest.kt index d275981a6be..71a07bb7f38 100644 --- a/sentry/src/test/java/io/sentry/metrics/MetricsApiTest.kt +++ b/sentry/src/test/java/io/sentry/metrics/MetricsApiTest.kt @@ -1,13 +1,17 @@ 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 { @@ -15,7 +19,14 @@ class MetricsApiTest { val aggregator = mock() val localMetricsAggregator = mock() - fun getSut(defaultTags: Map = emptyMap()): MetricsApi { + var lastSpan: ISpan? = null + var lastOp: String? = null + var lastDescription: String? = null + + fun getSut( + defaultTags: Map = emptyMap(), + spanProvider: () -> ISpan? = { mock() } + ): MetricsApi { val localAggregator = localMetricsAggregator return MetricsApi(object : IMetricsInterface { @@ -26,6 +37,13 @@ class MetricsApiTest { override fun getLocalMetricsAggregator(): LocalMetricsAggregator? = localAggregator override fun getDefaultTagsForMetrics(): Map = 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() + 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 + } }