feat(tracing): Allow setting a scope on a span (#667)

include/sentry.h

@@ -1366,8 +1366,8 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_context_update_from_header(
  *
  * To ensure that any Events or Message Events are associated with this
  * Transaction while it is active, invoke and pass in the Transaction returned
- * by this function to `sentry_set_span`. Further documentation on this can be
- * found in `sentry_set_span`'s docstring.
+ * by this function to `sentry_set_transaction_object`. Further documentation on
+ * this can be found in `sentry_set_transaction_object`'s docstring.
  *
  * Takes ownership of `transaction_context`. A Transaction Context cannot be
  * modified or re-used after it is used to start a Transaction.
@@ -1394,20 +1394,32 @@ SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish(
     sentry_transaction_t *tx);
 
 /**
- * Sets the Span (actually Transaction) so any Events sent while the Transaction
+ * Sets the Transaction so any Events sent while the Transaction
  * is active will be associated with the Transaction.
  *
  * If the Transaction being passed in is unsampled, it will still be associated
  * with any new Events. This will lead to some Events pointing to orphan or
  * missing traces in sentry, see
  * https://docs.sentry.io/product/sentry-basics/tracing/trace-view/#orphan-traces-and-broken-subtraces
  *
- * This increases the number of references pointing to the transaction. Invoke
- * `sentry_transaction_finish` to remove the Span set by this function as well
- * as its reference by passing in the same Transaction as the one passed into
+ * This increases the number of references pointing to the Transaction. Invoke
+ * `sentry_transaction_finish` to remove the Transaction set by this function as
+ * well as its reference by passing in the same Transaction as the one passed
+ * into this function.
+ */
+SENTRY_EXPERIMENTAL_API void sentry_set_transaction_object(
+    sentry_transaction_t *tx);
+
+/**
+ * Sets the Span so any Events sent while the Span
+ * is active will be associated with the Span.
+ *
+ * This increases the number of references pointing to the Span. Invoke
+ * `sentry_span_finish` to remove the Span set by this function as well
+ * as its reference by passing in the same Span as the one passed into
  * this function.
  */
-SENTRY_EXPERIMENTAL_API void sentry_set_span(sentry_transaction_t *tx);
+SENTRY_EXPERIMENTAL_API void sentry_set_span(sentry_span_t *span);
 
 /**
  * Starts a new Span.
@@ -1429,6 +1441,11 @@ SENTRY_EXPERIMENTAL_API void sentry_set_span(sentry_transaction_t *tx);
  * sentry. `sentry_value_null` will be returned if the child Span could not be
  * created.
  *
+ * To ensure that any Events or Message Events are associated with this
+ * Span while it is active, invoke and pass in the Span returned
+ * by this function to `sentry_set_span`. Further documentation on this can be
+ * found in `sentry_set_span`'s docstring.
+ *
  * This increases the number of references pointing to the Transaction.
  *
  * The returned value is not thread-safe. Users are expected to ensure that
@@ -1459,6 +1476,11 @@ SENTRY_EXPERIMENTAL_API sentry_span_t *sentry_transaction_start_child(
  * sentry. `sentry_value_null` will be returned if the child Span could not be
  * created.
  *
+ * To ensure that any Events or Message Events are associated with this
+ * Span while it is active, invoke and pass in the Span returned
+ * by this function to `sentry_set_span`. Further documentation on this can be
+ * found in `sentry_set_span`'s docstring.
+ *
  * The returned value is not thread-safe. Users are expected to ensure that
  * appropriate locking mechanisms are implemented over the Span if it needs
  * to be mutated across threads. Methods operating on the Span will mention what

src/sentry_core.c

@@ -714,8 +714,8 @@ sentry_set_transaction(const char *transaction)
         scope->transaction = sentry__string_clone(transaction);
 
 #ifdef SENTRY_PERFORMANCE_MONITORING
-        if (scope->span) {
-            sentry_transaction_set_name(scope->span, transaction);
+        if (scope->transaction_object) {
+            sentry_transaction_set_name(scope->transaction_object, transaction);
         }
 #endif
     }
@@ -778,16 +778,16 @@ sentry_transaction_finish(sentry_transaction_t *opaque_tx)
     sentry_value_t tx = sentry__value_clone(opaque_tx->inner);
 
     SENTRY_WITH_SCOPE_MUT (scope) {
-        if (scope->span) {
-            sentry_value_t scope_tx = scope->span->inner;
+        if (scope->transaction_object) {
+            sentry_value_t scope_tx = scope->transaction_object->inner;
 
             const char *tx_id = sentry_value_as_string(
                 sentry_value_get_by_key(tx, "trace_id"));
             const char *scope_tx_id = sentry_value_as_string(
                 sentry_value_get_by_key(scope_tx, "trace_id"));
             if (sentry__string_eq(tx_id, scope_tx_id)) {
-                sentry__transaction_decref(scope->span);
-                scope->span = NULL;
+                sentry__transaction_decref(scope->transaction_object);
+                scope->transaction_object = NULL;
             }
         }
     }
@@ -819,7 +819,7 @@ sentry_transaction_finish(sentry_transaction_t *opaque_tx)
 
     // TODO: add tracestate
     sentry_value_t trace_context
-        = sentry__transaction_get_trace_context(opaque_tx);
+        = sentry__value_get_trace_context(opaque_tx->inner);
     sentry_value_t contexts = sentry_value_new_object();
     sentry_value_set_by_key(contexts, "trace", trace_context);
     sentry_value_set_by_key(tx, "contexts", contexts);
@@ -843,12 +843,26 @@ sentry_transaction_finish(sentry_transaction_t *opaque_tx)
 }
 
 void
-sentry_set_span(sentry_transaction_t *tx)
+sentry_set_transaction_object(sentry_transaction_t *tx)
 {
     SENTRY_WITH_SCOPE_MUT (scope) {
-        sentry__transaction_decref(scope->span);
+        sentry__span_decref(scope->span);
+        scope->span = NULL;
+        sentry__transaction_decref(scope->transaction_object);
         sentry__transaction_incref(tx);
-        scope->span = tx;
+        scope->transaction_object = tx;
+    }
+}
+
+void
+sentry_set_span(sentry_span_t *span)
+{
+    SENTRY_WITH_SCOPE_MUT (scope) {
+        sentry__transaction_decref(scope->transaction_object);
+        scope->transaction_object = NULL;
+        sentry__span_decref(scope->span);
+        sentry__span_incref(span);
+        scope->span = span;
     }
 }
 
@@ -928,6 +942,21 @@ sentry_span_finish(sentry_span_t *opaque_span)
 
     sentry_value_t span = sentry__value_clone(opaque_span->inner);
 
+    SENTRY_WITH_SCOPE_MUT (scope) {
+        if (scope->span) {
+            sentry_value_t scope_span = scope->span->inner;
+
+            const char *span_id = sentry_value_as_string(
+                sentry_value_get_by_key(span, "trace_id"));
+            const char *scope_span_id = sentry_value_as_string(
+                sentry_value_get_by_key(scope_span, "trace_id"));
+            if (sentry__string_eq(span_id, scope_span_id)) {
+                sentry__span_decref(scope->span);
+                scope->span = NULL;
+            }
+        }
+    }
+
     if (!sentry_value_is_null(sentry_value_get_by_key(span, "timestamp"))) {
         SENTRY_DEBUG("span is already finished, aborting span finish");
         sentry_value_decref(span);

src/sentry_scope.c

@@ -82,6 +82,7 @@ get_scope(void)
     g_scope.client_sdk = get_client_sdk();
 
 #ifdef SENTRY_PERFORMANCE_MONITORING
+    g_scope.transaction_object = NULL;
     g_scope.span = NULL;
 #endif
 
@@ -106,7 +107,8 @@ sentry__scope_cleanup(void)
         sentry_value_decref(g_scope.client_sdk);
 
 #ifdef SENTRY_PERFORMANCE_MONITORING
-        sentry__transaction_decref(g_scope.span);
+        sentry__transaction_decref(g_scope.transaction_object);
+        sentry__span_decref(g_scope.span);
 #endif
     }
     sentry__mutex_unlock(&g_lock);
@@ -240,16 +242,24 @@ sentry__symbolize_stacktrace(sentry_value_t stacktrace)
 }
 
 #ifdef SENTRY_PERFORMANCE_MONITORING
+sentry_value_t
+sentry__get_span_or_transaction(const sentry_scope_t *scope)
+{
+    if (scope->span) {
+        return scope->span->inner;
+    } else if (scope->transaction_object) {
+        return scope->transaction_object->inner;
+    } else {
+        return sentry_value_new_null();
+    }
+}
+
 #    ifdef SENTRY_UNITTEST
 sentry_value_t
-sentry__scope_get_span()
+sentry__scope_get_span_or_transaction()
 {
     SENTRY_WITH_SCOPE (scope) {
-        if (!scope->span) {
-            return sentry_value_new_null();
-        } else {
-            return scope->span->inner;
-        }
+        return sentry__get_span_or_transaction(scope);
     }
     return sentry_value_new_null();
 }
@@ -320,8 +330,8 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope,
     sentry_value_t contexts = sentry__value_clone(scope->contexts);
     // prep contexts sourced from scope; data about transaction on scope needs
     // to be extracted and inserted
-    sentry_value_t scope_trace
-        = sentry__transaction_get_trace_context(scope->span);
+    sentry_value_t scope_trace = sentry__value_get_trace_context(
+        sentry__get_span_or_transaction(scope));
     if (!sentry_value_is_null(scope_trace)) {
         if (sentry_value_is_null(contexts)) {
             contexts = sentry_value_new_object();

src/sentry_scope.h

@@ -21,12 +21,16 @@ typedef struct sentry_scope_s {
     sentry_value_t client_sdk;
 
 #ifdef SENTRY_PERFORMANCE_MONITORING
-    // Not to be confused with transaction, which is a legacy value. This is
-    // also known as a transaction, but to maintain consistency with other SDKs
-    // and to avoid a conflict with the existing transaction field this is named
-    // span. Whenever possible, `transaction` should pull its value from the
-    // `name` property nested in this field.
-    sentry_transaction_t *span;
+    // The span attached to this scope, if any.
+    //
+    // Conceptually, every transaction is a span, so it should be possible to
+    // attach spans or transactions to a scope. But sentry_span_t and
+    // sentry_transaction_t are unrelated types in the native SDK, so we need
+    // two distinct pointers. At most one of them should ever be non-null.
+    // Whenever possible, `transaction` should pull its value from the
+    // `name` property nested in transaction_object or span.
+    sentry_transaction_t *transaction_object;
+    sentry_span_t *span;
 #endif
 } sentry_scope_t;
 
@@ -97,6 +101,6 @@ void sentry__scope_apply_to_event(const sentry_scope_t *scope,
 #ifdef SENTRY_PERFORMANCE_MONITORING
 // this is only used in unit tests
 #ifdef SENTRY_UNITTEST
-sentry_value_t sentry__scope_get_span();
+sentry_value_t sentry__scope_get_span_or_transaction();
 #endif
 #endif

src/sentry_tracing.c

@@ -205,6 +205,27 @@ sentry__transaction_decref(sentry_transaction_t *tx)
     };
 }
 
+void
+sentry__span_incref(sentry_span_t *span)
+{
+    sentry_value_incref(span->inner);
+}
+
+void
+sentry__span_decref(sentry_span_t *span)
+{
+    if (!span) {
+        return;
+    }
+
+    if (sentry_value_refcount(span->inner) <= 1) {
+        sentry_value_decref(span->inner);
+        sentry_free(span);
+    } else {
+        sentry_value_decref(span->inner);
+    };
+}
+
 sentry_span_t *
 sentry__span_new(sentry_transaction_t *tx, sentry_value_t inner)
 {
@@ -280,15 +301,14 @@ sentry__span_free(sentry_span_t *span)
 }
 
 sentry_value_t
-sentry__transaction_get_trace_context(sentry_transaction_t *opaque_tx)
+sentry__value_get_trace_context(sentry_value_t span)
 {
-    if (!opaque_tx || sentry_value_is_null(opaque_tx->inner)) {
+    if (sentry_value_is_null(span)) {
         return sentry_value_new_null();
     }
 
-    sentry_value_t tx = opaque_tx->inner;
-    if (sentry_value_is_null(sentry_value_get_by_key(tx, "trace_id"))
-        || sentry_value_is_null(sentry_value_get_by_key(tx, "span_id"))) {
+    if (sentry_value_is_null(sentry_value_get_by_key(span, "trace_id"))
+        || sentry_value_is_null(sentry_value_get_by_key(span, "span_id"))) {
         return sentry_value_new_null();
     }
 
@@ -303,12 +323,12 @@ sentry__transaction_get_trace_context(sentry_transaction_t *opaque_tx)
         }                                                                      \
     } while (0)
 
-    PLACE_VALUE("trace_id", tx);
-    PLACE_VALUE("span_id", tx);
-    PLACE_VALUE("parent_span_id", tx);
-    PLACE_VALUE("op", tx);
-    PLACE_VALUE("description", tx);
-    PLACE_VALUE("status", tx);
+    PLACE_VALUE("trace_id", span);
+    PLACE_VALUE("span_id", span);
+    PLACE_VALUE("parent_span_id", span);
+    PLACE_VALUE("op", span);
+    PLACE_VALUE("description", span);
+    PLACE_VALUE("status", span);
 
     // TODO: freeze this
     return trace_context;

src/sentry_tracing.h

@@ -33,6 +33,9 @@ sentry_transaction_t *sentry__transaction_new(sentry_value_t inner);
 void sentry__transaction_incref(sentry_transaction_t *tx);
 void sentry__transaction_decref(sentry_transaction_t *tx);
 
+void sentry__span_incref(sentry_span_t *span);
+void sentry__span_decref(sentry_span_t *span);
+
 sentry_value_t sentry__value_span_new(size_t max_spans, sentry_value_t parent,
     char *operation, char *description);
 sentry_span_t *sentry__span_new(
@@ -41,10 +44,9 @@ void sentry__span_free(sentry_span_t *span);
 
 /**
  * Returns an object containing tracing information extracted from a
- * transaction (/span) which should be included in an event.
+ * transaction / span which should be included in an event.
  * See https://develop.sentry.dev/sdk/event-payloads/transaction/#examples
  */
-sentry_value_t sentry__transaction_get_trace_context(
-    sentry_transaction_t *span);
+sentry_value_t sentry__value_get_trace_context(sentry_value_t span);
 
 #endif