feat(tracing): Only ever allow one active transaction, stuff transact…

…ion into scope

examples/example.c

@@ -221,8 +221,8 @@ main(int argc, char **argv)
             sentry_transaction_context_set_sampled(tx_ctx, 0);
         }
 
-        sentry_value_t tx = sentry_transaction_start(tx_ctx);
-        sentry_transaction_finish(tx);
+        sentry_transaction_start(tx_ctx);
+        sentry_transaction_finish();
     }
 
     // make sure everything flushes

include/sentry.h

@@ -1296,16 +1296,15 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_context_remove_sampled(
  * external integration (i.e. a span from a different SDK) or manually
  * constructed by a user.
  */
-SENTRY_EXPERIMENTAL_API sentry_value_t sentry_transaction_start(
+SENTRY_EXPERIMENTAL_API void sentry_transaction_start(
     sentry_value_t transaction_context);
 
 /**
- * Finishes and sends a transaction to sentry. The event ID of the transaction
- * will be returned if this was successful; A nil UUID will be returned
- * otherwise.
+ * Finishes and sends the current transaction to sentry. The event ID of the
+ * transaction will be returned if this was successful; A nil UUID will be
+ * returned otherwise.
  */
-SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish(
-    sentry_value_t transaction);
+SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish();
 
 #ifdef __cplusplus
 }

src/sentry_core.c

@@ -711,12 +711,11 @@ sentry_set_level(sentry_level_t level)
     }
 }
 
-sentry_value_t
+void
 sentry_transaction_start(sentry_value_t tx_cxt)
 {
     sentry_value_t tx = sentry_value_new_event();
 
-    // TODO(tracing): stuff transaction into the scope
     bool should_sample = sentry__should_send_transaction(tx_cxt);
     sentry_value_set_by_key(
         tx, "sampled", sentry_value_new_bool(should_sample));
@@ -738,27 +737,40 @@ sentry_transaction_start(sentry_value_t tx_cxt)
         sentry__value_new_string_owned(
             sentry__msec_time_to_iso8601(sentry__msec_time())));
 
+    sentry__scope_set_span(tx);
     sentry_value_decref(tx_cxt);
-
-    return tx;
 }
 
 sentry_uuid_t
-sentry_transaction_finish(sentry_value_t tx)
-{
-    // The sampling decision should already be made for transactions during
-    // their construction. No need to recalculate here. See
-    // `sentry__should_skip_transaction`.
-    sentry_value_t sampled = sentry_value_get_by_key(tx, "sampled");
-    if (!sentry_value_is_null(sampled) && !sentry_value_is_true(sampled)) {
-        SENTRY_DEBUG("throwing away transaction due to sample rate or "
-                     "user-provided sampling value in transaction context");
-        sentry_value_decref(sampled);
-        sentry_value_decref(tx);
-        // TODO(tracing): remove from scope
+sentry_transaction_finish()
+{
+    sentry_value_t tx = sentry_value_new_null();
+    SENTRY_WITH_SCOPE (scope) {
+        if (sentry_value_is_null(scope->span)) {
+            SENTRY_DEBUG("could not find a transaction on the scope to finish");
+            return sentry_uuid_nil();
+        }
+
+        // The sampling decision should already be made for transactions during
+        // their construction. No need to recalculate here. See
+        // `sentry__should_skip_transaction`.
+        sentry_value_t sampled
+            = sentry_value_get_by_key(scope->span, "sampled");
+        if (!sentry_value_is_true(sampled)) {
+            SENTRY_DEBUG("throwing away transaction due to sample rate or "
+                         "user-provided sampling value in transaction context");
+            sentry__scope_remove_span();
+            return sentry_uuid_nil();
+        }
+        tx = sentry__value_clone(scope->span);
+    }
+    sentry__scope_remove_span();
+    if (sentry_value_is_null(tx)) {
+        SENTRY_DEBUG("could not find a transaction on the scope to finish");
         return sentry_uuid_nil();
     }
-    sentry_value_decref(sampled);
+
+    sentry_value_remove_by_key(tx, "sampled");
 
     sentry_value_set_by_key(tx, "type", sentry_value_new_string("transaction"));
     sentry_value_set_by_key(tx, "timestamp",

src/sentry_core.h

@@ -118,7 +118,7 @@ void sentry__options_unlock(void);
     for (sentry_options_t *Options = sentry__options_lock(); Options;          \
          sentry__options_unlock(), Options = NULL)
 
-// these for now are only needed for tests
+// these for now are only needed outside of core for tests
 #ifdef SENTRY_UNITTEST
 bool sentry__roll_dice(double probability);
 bool sentry__should_send_transaction(sentry_value_t tx_cxt);

src/sentry_scope.c

@@ -230,9 +230,30 @@ sentry__symbolize_stacktrace(sentry_value_t stacktrace)
 void
 sentry__scope_set_span(sentry_value_t span)
 {
-    // TODO: implement this function and get rid of this line.
-    (void)span;
-    return;
+    SENTRY_WITH_SCOPE_MUT (scope) {
+        sentry_value_decref(scope->span);
+        scope->span = span;
+    }
+}
+
+#ifdef SENTRY_UNITTEST
+sentry_value_t
+sentry__scope_get_span()
+{
+    SENTRY_WITH_SCOPE (scope) {
+        return scope->span;
+    }
+    return sentry_value_new_null();
+}
+#endif
+
+void
+sentry__scope_remove_span()
+{
+    SENTRY_WITH_SCOPE_MUT (scope) {
+        sentry_value_decref(scope->span);
+        scope->span = sentry_value_new_null();
+    }
 }
 
 void

src/sentry_scope.h

@@ -77,10 +77,23 @@ void sentry__scope_apply_to_event(const sentry_scope_t *scope,
 
 /**
  * Sets the span (actually transaction) on the scope. An internal way to pass
- * around contextual information needed from a transaction into other events.
+ * around contextual information needed from a transaction into other events. If
+ * the scope already contains an unfinished transaction, that transaction will
+ * be discarded and will not be sent to sentry.
+ *
+ * This takes ownership of the span.
  */
 void sentry__scope_set_span(sentry_value_t span);
 
+/**
+ * Removes the current span (actually transaction) on the scope. If the
+ * transaction has not yet finished, this does not finish the transaction
+ * nor does it send it to sentry; The transaction will be discarded.
+ *
+ * Remove at your own discretion.
+ */
+void sentry__scope_remove_span();
+
 /**
  * These are convenience macros to automatically lock/unlock a scope inside a
  * code block.
@@ -96,3 +109,8 @@ void sentry__scope_set_span(sentry_value_t span);
          sentry__scope_unlock(), Scope = NULL)
 
 #endif
+
+// this is only used in unit tests
+#ifdef SENTRY_UNITTEST
+sentry_value_t sentry__scope_get_span();
+#endif

tests/unit/test_tracing.c

@@ -1,7 +1,12 @@
+#include "sentry_scope.h"
 #include "sentry_testsupport.h"
 #include "sentry_tracing.h"
 #include "sentry_uuid.h"
 
+#define CHECK_STRING_PROPERTY(Src, Field, Expected)                            \
+    TEST_CHECK_STRING_EQUAL(                                                   \
+        sentry_value_as_string(sentry_value_get_by_key(Src, Field)), Expected)
+
 SENTRY_TEST(basic_tracing_context)
 {
     sentry_value_t span = sentry_value_new_object();
@@ -128,23 +133,23 @@ SENTRY_TEST(basic_function_transport_transaction)
 
     sentry_value_t transaction = sentry_value_new_transaction_context(
         "How could you", "Don't capture this.");
-    transaction = sentry_transaction_start(transaction);
-    sentry_uuid_t event_id = sentry_transaction_finish(transaction);
+    sentry_transaction_start(transaction);
+    sentry_uuid_t event_id = sentry_transaction_finish();
     // TODO: `sentry_capture_event` acts as if the event was sent if user
     // consent was not given
     TEST_CHECK(!sentry_uuid_is_nil(&event_id));
     sentry_user_consent_give();
 
     transaction = sentry_value_new_transaction_context("honk", "beep");
-    transaction = sentry_transaction_start(transaction);
-    event_id = sentry_transaction_finish(transaction);
+    sentry_transaction_start(transaction);
+    event_id = sentry_transaction_finish();
     TEST_CHECK(!sentry_uuid_is_nil(&event_id));
 
     sentry_user_consent_revoke();
     transaction = sentry_value_new_transaction_context(
         "How could you again", "Don't capture this either.");
-    transaction = sentry_transaction_start(transaction);
-    event_id = sentry_transaction_finish(transaction);
+    sentry_transaction_start(transaction);
+    event_id = sentry_transaction_finish();
     // TODO: `sentry_capture_event` acts as if the event was sent if user
     // consent was not given
     TEST_CHECK(!sentry_uuid_is_nil(&event_id));
@@ -173,16 +178,16 @@ SENTRY_TEST(transport_sampling_transactions)
     for (int i = 0; i < 100; i++) {
         sentry_value_t transaction
             = sentry_value_new_transaction_context("honk", "beep");
-        transaction = sentry_transaction_start(transaction);
-        sentry_uuid_t event_id = sentry_transaction_finish(transaction);
+        sentry_transaction_start(transaction);
+        sentry_uuid_t event_id = sentry_transaction_finish();
         if (!sentry_uuid_is_nil(&event_id)) {
             sent_transactions += 1;
         }
     }
 
     sentry_close();
 
-    // well, its random after all
+    // exact value is nondeterministic because of rng
     TEST_CHECK(called_transport > 50 && called_transport < 100);
     TEST_CHECK(called_transport == sent_transactions);
 }
@@ -216,12 +221,62 @@ SENTRY_TEST(transactions_skip_before_send)
 
     sentry_value_t transaction
         = sentry_value_new_transaction_context("honk", "beep");
-    transaction = sentry_transaction_start(transaction);
-    sentry_uuid_t event_id = sentry_transaction_finish(transaction);
+    sentry_transaction_start(transaction);
+    sentry_uuid_t event_id = sentry_transaction_finish();
     TEST_CHECK(!sentry_uuid_is_nil(&event_id));
 
     sentry_close();
 
     TEST_CHECK_INT_EQUAL(called_transport, 1);
     TEST_CHECK_INT_EQUAL(called_beforesend, 0);
 }
+
+static void
+before_transport(sentry_envelope_t *envelope, void *data)
+{
+    uint64_t *called = data;
+    *called += 1;
+
+    sentry_envelope_free(envelope);
+}
+
+SENTRY_TEST(multiple_transactions)
+{
+    uint64_t called_transport = 0;
+
+    sentry_options_t *options = sentry_options_new();
+    sentry_options_set_dsn(options, "https://[email protected]/42");
+
+    sentry_transport_t *transport = sentry_transport_new(before_transport);
+    sentry_transport_set_state(transport, &called_transport);
+    sentry_options_set_transport(options, transport);
+
+    sentry_options_set_traces_sample_rate(options, 1.0);
+    sentry_init(options);
+
+    sentry_value_t tx_cxt = sentry_value_new_transaction_context("wow!", NULL);
+    sentry_transaction_start(tx_cxt);
+
+    sentry_value_t scope_tx = sentry__scope_get_span();
+    CHECK_STRING_PROPERTY(scope_tx, "transaction", "wow!");
+
+    sentry_uuid_t event_id = sentry_transaction_finish();
+    scope_tx = sentry__scope_get_span();
+    TEST_CHECK(sentry_value_is_null(scope_tx));
+    TEST_CHECK(!sentry_uuid_is_nil(&event_id));
+
+    tx_cxt = sentry_value_new_transaction_context("whoa!", NULL);
+    sentry_transaction_start(tx_cxt);
+    tx_cxt = sentry_value_new_transaction_context("wowee!", NULL);
+    sentry_transaction_start(tx_cxt);
+    scope_tx = sentry__scope_get_span();
+    CHECK_STRING_PROPERTY(scope_tx, "transaction", "wowee!");
+    event_id = sentry_transaction_finish();
+    TEST_CHECK(!sentry_uuid_is_nil(&event_id));
+
+    sentry_close();
+
+    TEST_CHECK_INT_EQUAL(called_transport, 2);
+}
+
+#undef CHECK_STRING_PROPERTY

tests/unit/tests.inc

@@ -29,6 +29,7 @@ XX(module_finder)
 XX(mpack_newlines)
 XX(mpack_removed_tags)
 XX(multiple_inits)
+XX(multiple_transactions)
 XX(os)
 XX(page_allocator)
 XX(path_basics)