feat(tracing): Spans now carry pointers to the Transactions they belong to

examples/example.c

@@ -246,8 +246,8 @@ main(int argc, char **argv)
                     grandchild, SENTRY_SPAN_STATUS_ALREADY_EXISTS);
             }
 
-            sentry_span_finish(tx, grandchild);
-            sentry_span_finish(tx, child);
+            sentry_span_finish(grandchild);
+            sentry_span_finish(child);
         }
 
         sentry_transaction_finish(tx);

include/sentry.h

@@ -1276,13 +1276,22 @@ typedef struct sentry_span_s sentry_span_t;
  * Also see https://develop.sentry.dev/sdk/event-payloads/transaction/#anatomy
  * for an explanation of `operation`, in addition to other properties and
  * actions that can be performed on a Transaction.
+ *
+ * The returned value is not thread-safe. Users are expected to ensure that
+ * appropriate locking mechanisms are implemented over the Transaction Context
+ * if it needs to be mutated across threads. Methods operating on the
+ * Transaction Context will mention what kind of expectations they carry if they
+ * ned to mutate or access the object in a thread-safe way.
  */
 SENTRY_EXPERIMENTAL_API sentry_transaction_context_t *
 sentry_transaction_context_new(const char *name, const char *operation);
 
 /**
  * Sets the `name` on a Transaction Context, which will be used in the
  * Transaction constructed off of the context.
+ *
+ * The Transaction Context passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_context_set_name(
     sentry_transaction_context_t *tx_cxt, const char *name);
@@ -1293,6 +1302,9 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_context_set_name(
  *
  * See https://develop.sentry.dev/sdk/performance/span-operations/ for
  * conventions on `operation`s.
+ *
+ * The Transaction Context passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_context_set_operation(
     sentry_transaction_context_t *tx_cxt, const char *operation);
@@ -1304,6 +1316,9 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_context_set_operation(
  * When passed any value above 0, the Transaction will bypass all sampling
  * options and always be sent to sentry. If passed 0, this Transaction and its
  * child spans will never be sent to sentry.
+ *
+ * The Transaction Context passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_context_set_sampled(
     sentry_transaction_context_t *tx_cxt, int sampled);
@@ -1313,28 +1328,39 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_context_set_sampled(
  * Transaction constructed off of the context.
  *
  * The Transaction will use the sampling rate as defined in `sentry_options`.
+ *
+ * The Transaction Context passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_context_remove_sampled(
     sentry_transaction_context_t *tx_cxt);
 
 /**
  * Starts a new Transaction based on the provided context, restored from an
  * external integration (i.e. a span from a different SDK) or manually
- * constructed by a user. Returns a Transaction, which is expected to be
- * manually managed by the caller. Manual management involves ensuring that
- * `sentry_transaction_finish` is invoked for the Transaction, and that the
- * caller manually starts and finishes any child Spans as needed on the
- * Transaction.
+ * constructed by a user.
+ *
+ * Returns a Transaction, which is expected to be manually managed by the
+ * caller. Manual management involves ensuring that `sentry_transaction_finish`
+ * is invoked for the Transaction, and that the caller manually starts and
+ * finishes any child Spans as needed on the Transaction.
  *
- * `sentry_transaction_finish` must be called in order for this Transaction to
- * be sent to sentry.
+ * Not invoking `sentry_transaction_finish` with the returned Transaction means
+ * it will be discarded, and will not be sent to sentry.
  *
  * 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.
  *
- * Takes ownership of `transaction_context`.
+ * Takes ownership of `transaction_context`. The Transaction Context passed in
+ * must have an exclusive write lock to be thread-safe.
+ *
+ * The returned value is not thread-safe. Users are expected to ensure that
+ * appropriate locking mechanisms are implemented over the Transaction if it
+ * needs to be mutated across threads. Methods operating on the Transaction will
+ * mention what kind of expectations they carry if they ned to mutate or access
+ * the object in a thread-safe way.
  */
 SENTRY_EXPERIMENTAL_API sentry_transaction_t *sentry_transaction_start(
     sentry_transaction_context_t *tx_cxt);
@@ -1345,8 +1371,10 @@ SENTRY_EXPERIMENTAL_API sentry_transaction_t *sentry_transaction_start(
  * otherwise.
  *
  * Always takes ownership of `transaction`, regardless of whether the operation
- * was successful or not. If `sentry_set_span` was invoked with `transaction`,
- * this will remove the
+ * was successful or not.
+ *
+ * The Transaction passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish(
     sentry_transaction_t *tx);
@@ -1364,6 +1392,8 @@ SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish(
  * 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 function.
+ *
+ * The Transaction passed in must have a read lock to be thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_set_span(sentry_transaction_t *tx);
 
@@ -1386,6 +1416,15 @@ SENTRY_EXPERIMENTAL_API void sentry_set_span(sentry_transaction_t *tx);
  * finishing the Span means it will be discarded, and will not be sent to
  * sentry. `sentry_value_null` will be returned if the child Span could not be
  * created.
+ *
+ * The Transaction passed in must have a read lock to be thread-safe. This
+ * increases the number of references pointing to the Transaction.
+ *
+ * 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 kind of expectations they carry if they ned to mutate or access
+ * the object in a thread-safe way.
  */
 SENTRY_EXPERIMENTAL_API sentry_span_t *sentry_transaction_start_child(
     sentry_transaction_t *parent, char *operation, char *description);
@@ -1408,45 +1447,65 @@ SENTRY_EXPERIMENTAL_API sentry_span_t *sentry_transaction_start_child(
  * finishing the Span means it will be discarded, and will not be sent to
  * sentry. `sentry_value_null` will be returned if the child Span could not be
  * created.
+ *
+ * The Span passed in must have a read lock to be thread-safe. This increases
+ * the number of references pointing to the passed-in Span's parent Transaction.
+ *
+ * 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 kind of expectations they carry if they ned to mutate or access
+ * the object in a thread-safe way.
  */
 SENTRY_EXPERIMENTAL_API sentry_span_t *sentry_span_start_child(
     sentry_span_t *parent, char *operation, char *description);
 
 /**
  * Finishes a Span.
  *
- * `root_transaction` is either the parent Transaction of the Span, or
- * the ancestor Transaction of the Span if the Span is not a direct descendant
- * of a Transaction.
- *
  * This takes ownership of `span`, as child Spans must always occur within the
  * total duration of a parent Span and cannot outlive their parent Spans.
+ *
+ * The Span passed in must have an exclusive write lock to be thread-safe. This
+ * will mutate the Span's parent Transaction, and therefore also requires an
+ * exclusive write lock on that Transaction as well.
  */
-SENTRY_EXPERIMENTAL_API void sentry_span_finish(
-    sentry_transaction_t *root_transaction, sentry_span_t *span);
+SENTRY_EXPERIMENTAL_API void sentry_span_finish(sentry_span_t *span);
 
 /**
  * Sets a tag on a transaction to the given string value.
  *
  * Tags longer than 200 bytes will be truncated.
+ *
+ * The Transaction passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_set_tag(
     sentry_transaction_t *transaction, const char *tag, const char *value);
 
 /**
  * Removes a tag from a transaction.
+ *
+ * The Transaction passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_tag(
     sentry_transaction_t *transaction, const char *tag);
 
 /**
  * Sets the given key in a transaction's "data" section to the given value.
+ *
+ * The Transaction passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_set_data(
     sentry_transaction_t *transaction, const char *key, sentry_value_t value);
 
 /**
  * Removes a key from a transaction's "data" section.
+ *
+ * The Transaction passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_data(
     sentry_transaction_t *transaction, const char *key);
@@ -1455,30 +1514,45 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_data(
  * Sets a tag on a span to the given string value.
  *
  * Tags longer than 200 bytes will be truncated.
+ *
+ * The Span passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_span_set_tag(
     sentry_span_t *span, const char *tag, const char *value);
 
 /**
  * Removes a tag from a span.
+ *
+ * The Transaction passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_span_remove_tag(
     sentry_span_t *span, const char *tag);
 
 /**
  * Sets the given key in a span's "data" section to the given value.
+ *
+ * The Span passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_span_set_data(
     sentry_span_t *span, const char *key, sentry_value_t value);
 
 /**
  * Removes a key from a span's "data" section.
+ *
+ * The Span passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_span_remove_data(
     sentry_span_t *span, const char *key);
 
 /**
  * Sets a transaction's name.
+ *
+ * The Transaction passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_set_name(
     sentry_transaction_t *transaction, const char *name);
@@ -1543,12 +1617,18 @@ typedef enum {
 
 /**
  * Sets a span's status.
+ *
+ * The Span passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_span_set_status(
     sentry_span_t *span, sentry_span_status_t status);
 
 /**
  * Sets a transaction's status.
+ *
+ * The Transaction passed in must have an exclusive write lock to be
+ * thread-safe.
  */
 SENTRY_EXPERIMENTAL_API void sentry_transaction_set_status(
     sentry_transaction_t *tx, sentry_span_status_t status);

src/sentry_core.c

@@ -864,17 +864,21 @@ sentry_transaction_start_child(
         max_spans = options->max_spans;
     }
 
-    return sentry__start_child(max_spans, parent, operation, description);
-    // TODO: add pointer to transaction on child span
+    sentry_value_t span
+        = sentry__value_span_new(max_spans, parent, operation, description);
+    return sentry__span_new(opaque_parent, span);
 }
 
 sentry_span_t *
 sentry_span_start_child(
     sentry_span_t *opaque_parent, char *operation, char *description)
 {
-    // TODO: check and validate ref to ancestor transaction on parent span
     if (!opaque_parent || sentry_value_is_null(opaque_parent->inner)) {
-        SENTRY_DEBUG("no parent available to create a child under");
+        SENTRY_DEBUG("no parent span available to create a child span under");
+        return NULL;
+    }
+    if (!opaque_parent->transaction) {
+        SENTRY_DEBUG("no root transaction to create a child span under");
         return NULL;
     }
     sentry_value_t parent = opaque_parent->inner;
@@ -886,21 +890,25 @@ sentry_span_start_child(
         max_spans = options->max_spans;
     }
 
-    return sentry__start_child(max_spans, parent, operation, description);
-    // TODO: add pointer to ancestor transaction on child span
+    sentry_value_t span
+        = sentry__value_span_new(max_spans, parent, operation, description);
+
+    return sentry__span_new(opaque_parent->transaction, span);
 }
 
-// TODO: don't accept the root transaction as a param, the span should have a
-// ref to it already in itself
 void
-sentry_span_finish(
-    sentry_transaction_t *opaque_root_transaction, sentry_span_t *opaque_span)
+sentry_span_finish(sentry_span_t *opaque_span)
 {
-    if (!opaque_root_transaction || !opaque_span
-        || sentry_value_is_null(opaque_root_transaction->inner)
-        || sentry_value_is_null(opaque_span->inner)) {
+    if (!opaque_span || sentry_value_is_null(opaque_span->inner)) {
+        SENTRY_DEBUG("no span to finish");
+        goto fail;
+    }
+
+    sentry_transaction_t *opaque_root_transaction = opaque_span->transaction;
+    if (!opaque_root_transaction
+        || sentry_value_is_null(opaque_root_transaction->inner)) {
         SENTRY_DEBUG(
-            "missing root transaction or span to finish, aborting span finish");
+            "no root transaction to finish span on, aborting span finish");
         goto fail;
     }