Polish Performance API (#1165)

Author: maciejwalkowiak

CHANGELOG.md

@@ -1,13 +1,14 @@
 # vNext
 
-* Enchancement: Support SENTRY_TRACES_SAMPLE_RATE conf. via env variables (#1171)
+* Enhancement: Support SENTRY_TRACES_SAMPLE_RATE conf. via env variables (#1171)
 * Enhancement: Pass request to CustomSamplingContext in Spring integration (#1172)
 * Ref: Set SpanContext on SentryTransaction to avoid potential NPE (#1173)
 * Fix: Free Local Refs manually due to Android local ref. count limits
 * Enhancement: Move `SentrySpanClientHttpRequestInterceptor` to Spring module (#1181)
 * Enhancement: Add overload for `transaction/span.finish(SpanStatus)` (#1182)
 * Fix: Bring back support for setting transaction name without ongoing transaction (#1183)
 * Enhancement: Simplify registering traces sample callback in Spring integration (#1184)
+* Enhancement: Polish Performance API (#1165)
 * Enhancement: Set "debug" through external properties (#1186)
 
 # 4.0.0-alpha.3

sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java

@@ -2,11 +2,11 @@
 
 import io.sentry.Breadcrumb;
 import io.sentry.EventProcessor;
+import io.sentry.ISpan;
 import io.sentry.ITransaction;
 import io.sentry.Sentry;
 import io.sentry.SentryEvent;
 import io.sentry.SentryLevel;
-import io.sentry.Span;
 import io.sentry.SpanStatus;
 import io.sentry.protocol.Message;
 import io.sentry.protocol.User;
@@ -72,7 +72,7 @@ public static void main(String[] args) throws InterruptedException {
           options.setTracesSampler(
               context -> {
                 // only 10% of transactions with "/product" prefix will be collected
-                if (!context.getTransactionContexts().getName().startsWith("/products")) {
+                if (!context.getTransactionContext().getName().startsWith("/products")) {
                   return 0.1;
                 } else {
                   return 0.5;
@@ -140,10 +140,10 @@ public static void main(String[] args) throws InterruptedException {
     // and finish of transaction.
     ITransaction transaction = Sentry.startTransaction("transaction name");
     // Transactions can contain one or more Spans
-    Span outerSpan = transaction.startChild();
+    ISpan outerSpan = transaction.startChild("child");
     Thread.sleep(100);
     // Spans create a tree structure. Each span can have one ore more spans inside.
-    Span innerSpan = outerSpan.startChild("jdbc", "select * from product where id = :id");
+    ISpan innerSpan = outerSpan.startChild("jdbc", "select * from product where id = :id");
     innerSpan.setStatus(SpanStatus.OK);
     Thread.sleep(300);
     // Finish the span and mark the end time of the span execution.

sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt

@@ -58,7 +58,7 @@ class SentrySpanRestTemplateCustomizerTest {
         assertThat(result).isEqualTo("OK")
         assertThat(fixture.transaction.spans).hasSize(1)
         val span = fixture.transaction.spans.first()
-        assertThat(span.operation).isEqualTo("http")
+        assertThat(span.operation).isEqualTo("http.client")
         assertThat(span.description).isEqualTo("GET /test/{id}")
         assertThat(span.status).isEqualTo(SpanStatus.OK)
         fixture.mockServer.verify()
@@ -71,7 +71,7 @@ class SentrySpanRestTemplateCustomizerTest {
         } catch (e: Throwable) {}
         assertThat(fixture.transaction.spans).hasSize(1)
         val span = fixture.transaction.spans.first()
-        assertThat(span.operation).isEqualTo("http")
+        assertThat(span.operation).isEqualTo("http.client")
         assertThat(span.description).isEqualTo("GET /test/{id}")
         assertThat(span.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
         fixture.mockServer.verify()

sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpan.java

@@ -19,21 +19,21 @@
    *
    * @return description
    */
-  @AliasFor("value")
   String description() default "";
 
   /**
    * Span operation.
    *
    * @return operation.
    */
+  @AliasFor("value")
   String operation() default "";
 
   /**
-   * Span description.
+   * Span operation.
    *
-   * @return description.
+   * @return operation.
    */
-  @AliasFor("description")
+  @AliasFor("operation")
   String value() default "";
 }

sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanAdvice.java

@@ -37,13 +37,12 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
       final Method mostSpecificMethod =
           AopUtils.getMostSpecificMethod(invocation.getMethod(), invocation.getThis().getClass());
       final Class<?> targetClass = invocation.getMethod().getDeclaringClass();
-      final ISpan span = activeSpan.startChild();
-
       final SentrySpan sentrySpan =
           AnnotationUtils.findAnnotation(mostSpecificMethod, SentrySpan.class);
-      span.setDescription(resolveSpanDescription(targetClass, mostSpecificMethod, sentrySpan));
-      if (sentrySpan != null && !StringUtils.isEmpty(sentrySpan.operation())) {
-        span.setOperation(sentrySpan.operation());
+      final String operation = resolveSpanOperation(targetClass, mostSpecificMethod, sentrySpan);
+      final ISpan span = activeSpan.startChild(operation);
+      if (sentrySpan != null && !StringUtils.isEmpty(sentrySpan.description())) {
+        span.setDescription(sentrySpan.description());
       }
       try {
         final Object result = invocation.proceed();
@@ -59,7 +58,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
     }
   }
 
-  private String resolveSpanDescription(
+  private String resolveSpanOperation(
       Class<?> targetClass, Method method, @Nullable SentrySpan sentrySpan) {
     return sentrySpan == null || StringUtils.isEmpty(sentrySpan.value())
         ? targetClass.getSimpleName() + "." + method.getName()

sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java

@@ -41,10 +41,9 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) {
       return execution.execute(request, body);
     }
 
-    final ISpan span = activeSpan.startChild();
+    final ISpan span = activeSpan.startChild("http.client");
     span.setDescription(
         request.getMethodValue() + " " + ensureLeadingSlash(urlTemplate.get().poll()));
-    span.setOperation("http");
 
     final SentryTraceHeader sentryTraceHeader = activeSpan.toSentryTrace();
     request.getHeaders().add(sentryTraceHeader.getName(), sentryTraceHeader.getValue());

sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentrySpanAdviceTest.kt

@@ -67,8 +67,8 @@ class SentrySpanAdviceTest {
         val result = sampleService.methodWithoutSpanDescriptionSet()
         assertEquals(2, result)
         assertEquals(1, tx.spans.size)
-        assertEquals("SampleService.methodWithoutSpanDescriptionSet", tx.spans.first().description)
-        assertNull(tx.spans.first().operation)
+        assertEquals("SampleService.methodWithoutSpanDescriptionSet", tx.spans.first().operation)
+        assertNull(tx.spans.first().description)
     }
 
     @Test

sentry/api/sentry.api

@@ -124,7 +124,7 @@ public final class io/sentry/Hub : io/sentry/IHub {
 	public fun setExtra (Ljava/lang/String;Ljava/lang/String;)V
 	public fun setFingerprint (Ljava/util/List;)V
 	public fun setLevel (Lio/sentry/SentryLevel;)V
-	public fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/SpanContext;)V
+	public fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;)V
 	public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 	public fun setTransaction (Ljava/lang/String;)V
 	public fun setUser (Lio/sentry/protocol/User;)V
@@ -163,7 +163,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub {
 	public fun setExtra (Ljava/lang/String;Ljava/lang/String;)V
 	public fun setFingerprint (Ljava/util/List;)V
 	public fun setLevel (Lio/sentry/SentryLevel;)V
-	public fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/SpanContext;)V
+	public fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;)V
 	public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 	public fun setTransaction (Ljava/lang/String;)V
 	public fun setUser (Lio/sentry/protocol/User;)V
@@ -217,7 +217,7 @@ public abstract interface class io/sentry/IHub {
 	public abstract fun setExtra (Ljava/lang/String;Ljava/lang/String;)V
 	public abstract fun setFingerprint (Ljava/util/List;)V
 	public abstract fun setLevel (Lio/sentry/SentryLevel;)V
-	public abstract fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/SpanContext;)V
+	public abstract fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;)V
 	public abstract fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 	public abstract fun setTransaction (Ljava/lang/String;)V
 	public abstract fun setUser (Lio/sentry/protocol/User;)V
@@ -287,8 +287,8 @@ public abstract interface class io/sentry/ISpan {
 	public abstract fun setStatus (Lio/sentry/SpanStatus;)V
 	public abstract fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 	public abstract fun setThrowable (Ljava/lang/Throwable;)V
-	public abstract fun startChild ()Lio/sentry/Span;
-	public abstract fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/Span;
+	public abstract fun startChild (Ljava/lang/String;)Lio/sentry/ISpan;
+	public abstract fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan;
 	public abstract fun toSentryTrace ()Lio/sentry/SentryTraceHeader;
 }
 
@@ -303,8 +303,6 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan {
 	public abstract fun isSampled ()Ljava/lang/Boolean;
 	public abstract fun setName (Ljava/lang/String;)V
 	public abstract fun setRequest (Lio/sentry/protocol/Request;)V
-	public abstract fun startChild (Lio/sentry/SpanId;)Lio/sentry/Span;
-	public abstract fun startChild (Lio/sentry/SpanId;Ljava/lang/String;Ljava/lang/String;)Lio/sentry/Span;
 }
 
 public abstract interface class io/sentry/ITransportFactory {
@@ -341,13 +339,29 @@ public final class io/sentry/NoOpLogger : io/sentry/ILogger {
 	public fun log (Lio/sentry/SentryLevel;Ljava/lang/Throwable;Ljava/lang/String;[Ljava/lang/Object;)V
 }
 
+public final class io/sentry/NoOpSpan : io/sentry/ISpan {
+	public fun finish ()V
+	public fun finish (Lio/sentry/SpanStatus;)V
+	public static fun getInstance ()Lio/sentry/NoOpSpan;
+	public fun getSpanContext ()Lio/sentry/SpanContext;
+	public fun getThrowable ()Ljava/lang/Throwable;
+	public fun setDescription (Ljava/lang/String;)V
+	public fun setOperation (Ljava/lang/String;)V
+	public fun setStatus (Lio/sentry/SpanStatus;)V
+	public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
+	public fun setThrowable (Ljava/lang/Throwable;)V
+	public fun startChild (Ljava/lang/String;)Lio/sentry/ISpan;
+	public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan;
+	public fun toSentryTrace ()Lio/sentry/SentryTraceHeader;
+}
+
 public final class io/sentry/NoOpTransaction : io/sentry/ITransaction {
-	public fun <init> ()V
 	public fun finish ()V
 	public fun finish (Lio/sentry/SpanStatus;)V
 	public fun getContexts ()Lio/sentry/protocol/Contexts;
 	public fun getDescription ()Ljava/lang/String;
 	public fun getEventId ()Lio/sentry/protocol/SentryId;
+	public static fun getInstance ()Lio/sentry/NoOpTransaction;
 	public fun getLatestActiveSpan ()Lio/sentry/Span;
 	public fun getRequest ()Lio/sentry/protocol/Request;
 	public fun getSpanContext ()Lio/sentry/SpanContext;
@@ -362,10 +376,8 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction {
 	public fun setStatus (Lio/sentry/SpanStatus;)V
 	public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 	public fun setThrowable (Ljava/lang/Throwable;)V
-	public fun startChild ()Lio/sentry/Span;
-	public fun startChild (Lio/sentry/SpanId;)Lio/sentry/Span;
-	public fun startChild (Lio/sentry/SpanId;Ljava/lang/String;Ljava/lang/String;)Lio/sentry/Span;
-	public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/Span;
+	public fun startChild (Ljava/lang/String;)Lio/sentry/ISpan;
+	public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan;
 	public fun toSentryTrace ()Lio/sentry/SentryTraceHeader;
 }
 
@@ -394,7 +406,7 @@ public final class io/sentry/RequestDetails {
 public final class io/sentry/SamplingContext {
 	public fun <init> (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;)V
 	public fun getCustomSamplingContext ()Lio/sentry/CustomSamplingContext;
-	public fun getTransactionContexts ()Lio/sentry/TransactionContext;
+	public fun getTransactionContext ()Lio/sentry/TransactionContext;
 }
 
 public final class io/sentry/Scope : java/lang/Cloneable {
@@ -824,10 +836,8 @@ public final class io/sentry/SentryTransaction : io/sentry/SentryBaseEvent, io/s
 	public fun setName (Ljava/lang/String;)V
 	public fun setOperation (Ljava/lang/String;)V
 	public fun setStatus (Lio/sentry/SpanStatus;)V
-	public fun startChild ()Lio/sentry/Span;
-	public fun startChild (Lio/sentry/SpanId;)Lio/sentry/Span;
-	public fun startChild (Lio/sentry/SpanId;Ljava/lang/String;Ljava/lang/String;)Lio/sentry/Span;
-	public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/Span;
+	public fun startChild (Ljava/lang/String;)Lio/sentry/ISpan;
+	public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan;
 	public fun toSentryTrace ()Lio/sentry/SentryTraceHeader;
 }
 
@@ -886,8 +896,8 @@ public final class io/sentry/Span : io/sentry/SpanContext, io/sentry/ISpan {
 	public fun getThrowable ()Ljava/lang/Throwable;
 	public fun getTimestamp ()Ljava/util/Date;
 	public fun setThrowable (Ljava/lang/Throwable;)V
-	public fun startChild ()Lio/sentry/Span;
-	public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/Span;
+	public fun startChild (Ljava/lang/String;)Lio/sentry/ISpan;
+	public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan;
 	public fun toSentryTrace ()Lio/sentry/SentryTraceHeader;
 }
 
@@ -910,7 +920,6 @@ public class io/sentry/SpanContext : java/lang/Cloneable {
 	public fun getTags ()Ljava/util/Map;
 	public fun setDescription (Ljava/lang/String;)V
 	public fun setOperation (Ljava/lang/String;)V
-	public fun setSampled (Ljava/lang/Boolean;)V
 	public fun setStatus (Lio/sentry/SpanStatus;)V
 	public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 }
@@ -958,7 +967,7 @@ public final class io/sentry/SystemOutLogger : io/sentry/ILogger {
 
 public final class io/sentry/TransactionContext : io/sentry/SpanContext {
 	public fun <init> (Ljava/lang/String;)V
-	public fun <init> (Ljava/lang/String;Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/SpanId;Ljava/lang/Boolean;)V
+	public fun <init> (Ljava/lang/String;Ljava/lang/Boolean;)V
 	public static fun fromSentryTrace (Ljava/lang/String;Lio/sentry/SentryTraceHeader;)Lio/sentry/TransactionContext;
 	public fun getName ()Ljava/lang/String;
 	public fun getParentSampled ()Ljava/lang/Boolean;

sentry/src/main/java/io/sentry/Hub.java

@@ -18,9 +18,8 @@ public final class Hub implements IHub {
   private final @NotNull SentryOptions options;
   private volatile boolean isEnabled;
   private final @NotNull Stack stack;
-  private final @NotNull TracingSampler tracingSampler;
-  private final @NotNull WeakHashMap<Throwable, SpanContext> throwableToSpanContext =
-      new WeakHashMap<>();
+  private final @NotNull TracesSampler tracesSampler;
+  private final @NotNull WeakHashMap<Throwable, ISpan> throwableToSpan = new WeakHashMap<>();
 
   public Hub(final @NotNull SentryOptions options) {
     this(options, createRootStackItem(options));
@@ -32,7 +31,7 @@ private Hub(final @NotNull SentryOptions options, final @NotNull Stack stack) {
     validateOptions(options);
 
     this.options = options;
-    this.tracingSampler = new TracingSampler(options);
+    this.tracesSampler = new TracesSampler(options);
     this.stack = stack;
     this.lastEventId = SentryId.EMPTY_ID;
 
@@ -171,10 +170,10 @@ public SentryId captureEnvelope(
 
   private void assignTraceContext(final @NotNull SentryEvent event) {
     if (event.getThrowable() != null) {
-      final SpanContext spanContext = throwableToSpanContext.get(event.getThrowable());
-      if (spanContext != null) {
+      final ISpan span = throwableToSpan.get(event.getThrowable());
+      if (span != null && span.getSpanContext() != null) {
         if (event.getContexts().getTrace() == null) {
-          event.getContexts().setTrace(spanContext);
+          event.getContexts().setTrace(span.getSpanContext());
         }
       }
     }
@@ -556,20 +555,15 @@ public void flush(long timeoutMillis) {
 
   @Override
   public @NotNull ITransaction startTransaction(
-      final @NotNull TransactionContext transactionContexts) {
-    return this.startTransaction(transactionContexts, null);
+      final @NotNull TransactionContext transactionContext) {
+    return this.startTransaction(transactionContext, null);
   }
 
   @Override
   public @NotNull ITransaction startTransaction(
-      final @NotNull TransactionContext transactionContexts,
+      final @NotNull TransactionContext transactionContext,
       final @Nullable CustomSamplingContext customSamplingContext) {
-    Objects.requireNonNull(transactionContexts, "transactionContexts is required");
-
-    final SamplingContext samplingContext =
-        new SamplingContext(transactionContexts, customSamplingContext);
-    boolean samplingDecision = tracingSampler.sample(samplingContext);
-    transactionContexts.setSampled(samplingDecision);
+    Objects.requireNonNull(transactionContext, "transactionContext is required");
 
     ITransaction transaction;
     if (!isEnabled()) {
@@ -578,9 +572,14 @@ public void flush(long timeoutMillis) {
           .log(
               SentryLevel.WARNING,
               "Instance is disabled and this 'startTransaction' returns a no-op.");
-      transaction = new NoOpTransaction();
+      transaction = NoOpTransaction.getInstance();
     } else {
-      transaction = new SentryTransaction(transactionContexts, this);
+      final SamplingContext samplingContext =
+          new SamplingContext(transactionContext, customSamplingContext);
+      boolean samplingDecision = tracesSampler.sample(samplingContext);
+      transactionContext.setSampled(samplingDecision);
+
+      transaction = new SentryTransaction(transactionContext, this);
     }
     return transaction;
   }
@@ -616,16 +615,20 @@ public void flush(long timeoutMillis) {
   }
 
   @Override
-  public void setSpanContext(
-      final @NotNull Throwable throwable, final @NotNull SpanContext spanContext) {
+  @ApiStatus.Internal
+  public void setSpanContext(final @NotNull Throwable throwable, final @NotNull ISpan span) {
     Objects.requireNonNull(throwable, "throwable is required");
-    Objects.requireNonNull(spanContext, "spanContext is required");
-    this.throwableToSpanContext.put(throwable, spanContext);
+    Objects.requireNonNull(span, "span is required");
+    this.throwableToSpan.put(throwable, span);
   }
 
   @Override
   public @Nullable SpanContext getSpanContext(final @NotNull Throwable throwable) {
     Objects.requireNonNull(throwable, "throwable is required");
-    return this.throwableToSpanContext.get(throwable);
+    final ISpan span = this.throwableToSpan.get(throwable);
+    if (span != null) {
+      return span.getSpanContext();
+    }
+    return null;
   }
 }

sentry/src/main/java/io/sentry/HubAdapter.java

@@ -181,7 +181,7 @@ public IHub clone() {
   }
 
   @Override
-  public void setSpanContext(final @NotNull Throwable t, final @NotNull SpanContext sc) {
+  public void setSpanContext(final @NotNull Throwable t, final @NotNull ISpan sc) {
     Sentry.getCurrentHub().setSpanContext(t, sc);
   }