feat(tracing): Defer some transaction validation and allow creation o…

…f internal spans

src/sentry_core.c

@@ -730,14 +730,24 @@ sentry_set_level(sentry_level_t level)
 void
 sentry_transaction_start(sentry_value_t tx_cxt)
 {
+    // TODO: it would be nice if we could just merge tx_cxt into tx.
+    // `sentry_value_new_transaction_event()` is also an option, but risks
+    // causing more confusion as there's already a
+    // `sentry_value_new_transaction`. The ending timestamp is stripped as well
+    // to avoid misleading ourselves later down the line.
     sentry_value_t tx = sentry_value_new_event();
+    sentry_value_remove_by_key(tx, "timestamp");
 
     bool should_sample = sentry__should_send_transaction(tx_cxt);
     sentry_value_set_by_key(
         tx, "sampled", sentry_value_new_bool(should_sample));
 
+    sentry_value_set_by_key(tx, "transaction",
+        sentry_value_get_by_key_owned(tx_cxt, "transaction"));
+
     sentry_value_set_by_key(
-        tx, "transaction", sentry_value_get_by_key_owned(tx_cxt, "name"));
+        tx, "status", sentry_value_get_by_key_owned(tx_cxt, "status"));
+
     sentry_value_set_by_key(tx, "start_timestamp",
         sentry__value_new_string_owned(
             sentry__msec_time_to_iso8601(sentry__msec_time())));
@@ -786,10 +796,13 @@ sentry_transaction_finish()
             sentry__msec_time_to_iso8601(sentry__msec_time())));
     sentry_value_set_by_key(tx, "level", sentry_value_new_string("info"));
 
-    // TODO: add tracestate
-    // set up trace context so it mirrors the final json value
-    sentry_value_set_by_key(tx, "status", sentry_value_new_string("ok"));
+    sentry_value_t name = sentry_value_get_by_key(tx, "transaction");
+    if (sentry_value_is_null(name) || sentry_value_get_length(name) == 0) {
+        sentry_value_set_by_key(tx, "transaction",
+            sentry_value_new_string("<unlabeled transaction>"));
+    }
 
+    // TODO: add tracestate
     sentry_value_t trace_context = sentry__span_get_trace_context(tx);
     sentry_value_t contexts = sentry_value_new_object();
     sentry_value_set_by_key(contexts, "trace", trace_context);

src/sentry_value.c

@@ -1125,27 +1125,40 @@ sentry_value_new_stacktrace(void **ips, size_t len)
 }
 
 sentry_value_t
-sentry_value_new_transaction(const char *name, const char *operation)
+sentry__value_new_span(sentry_value_t parent, const char *operation)
 {
-    sentry_value_t transaction = sentry_value_new_object();
+    sentry_value_t span = sentry_value_new_object();
+
+    sentry_transaction_set_operation(span, operation);
+    sentry_value_set_by_key(span, "status", sentry_value_new_string("ok"));
+
+    if (!sentry_value_is_null(parent)) {
+        sentry_value_set_by_key(span, "trace_id",
+            sentry_value_get_by_key_owned(parent, "trace_id"));
+        sentry_value_set_by_key(span, "parent_span_id",
+            sentry_value_get_by_key_owned(parent, "span_id"));
+        sentry_value_set_by_key(
+            span, "sampled", sentry_value_get_by_key_owned(parent, "sampled"));
+    }
+
+    return span;
+}
 
+sentry_value_t
+sentry_value_new_transaction(const char *name, const char *operation)
+{
+    sentry_value_t transaction
+        = sentry__value_new_span(sentry_value_new_null(), operation);
     sentry_transaction_set_name(transaction, name);
-    sentry_transaction_set_operation(transaction, operation);
 
     return transaction;
 }
 
 void
 sentry_transaction_set_name(sentry_value_t transaction, const char *name)
 {
-    sentry_value_t sv_name = sentry_value_new_string(name);
-    // TODO: Consider doing this checking right before sending or flushing
-    // the transaction.
-    if (sentry_value_is_null(sv_name) || sentry__string_eq(name, "")) {
-        sentry_value_decref(sv_name);
-        sv_name = sentry_value_new_string("<unlabeled transaction>");
-    }
-    sentry_value_set_by_key(transaction, "name", sv_name);
+    sentry_value_set_by_key(
+        transaction, "transaction", sentry_value_new_string(name));
 }
 
 void

src/sentry_value.h

@@ -61,6 +61,13 @@ sentry_value_t sentry__value_new_list_with_size(size_t size);
  */
 sentry_value_t sentry__value_new_object_with_size(size_t size);
 
+/**
+ * Constructs a new Span.
+ *
+ */
+sentry_value_t sentry__value_new_span(
+    sentry_value_t parent, const char *operation);
+
 /**
  * This will parse the Value into a UUID, or return a `nil` UUID on error.
  * See also `sentry_uuid_from_string`.

tests/unit/test_tracing.c

@@ -43,34 +43,25 @@ SENTRY_TEST(basic_transaction)
 {
     sentry_value_t tx_cxt = sentry_value_new_transaction(NULL, NULL);
     TEST_CHECK(!sentry_value_is_null(tx_cxt));
-    const char *tx_name
-        = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name"));
-    TEST_CHECK_STRING_EQUAL(tx_name, "<unlabeled transaction>");
-    const char *tx_op
-        = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "op"));
-    TEST_CHECK_STRING_EQUAL(tx_op, "");
+    CHECK_STRING_PROPERTY(tx_cxt, "transaction", "");
+    CHECK_STRING_PROPERTY(tx_cxt, "op", "");
 
     sentry_value_decref(tx_cxt);
     tx_cxt = sentry_value_new_transaction("", "");
     TEST_CHECK(!sentry_value_is_null(tx_cxt));
-    tx_name = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name"));
-    TEST_CHECK_STRING_EQUAL(tx_name, "<unlabeled transaction>");
-    TEST_CHECK_STRING_EQUAL(tx_op, "");
+    CHECK_STRING_PROPERTY(tx_cxt, "transaction", "");
+    CHECK_STRING_PROPERTY(tx_cxt, "op", "");
 
     sentry_value_decref(tx_cxt);
     tx_cxt = sentry_value_new_transaction("honk.beep", "beepbeep");
-    tx_name = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name"));
-    TEST_CHECK_STRING_EQUAL(tx_name, "honk.beep");
-    tx_op = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "op"));
-    TEST_CHECK_STRING_EQUAL(tx_op, "beepbeep");
+    CHECK_STRING_PROPERTY(tx_cxt, "transaction", "honk.beep");
+    CHECK_STRING_PROPERTY(tx_cxt, "op", "beepbeep");
 
     sentry_transaction_set_name(tx_cxt, "");
-    tx_name = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name"));
-    TEST_CHECK_STRING_EQUAL(tx_name, "<unlabeled transaction>");
+    CHECK_STRING_PROPERTY(tx_cxt, "transaction", "");
 
     sentry_transaction_set_operation(tx_cxt, "");
-    tx_op = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "op"));
-    TEST_CHECK_STRING_EQUAL(tx_op, "");
+    CHECK_STRING_PROPERTY(tx_cxt, "op", "");
 
     sentry_transaction_set_sampled(tx_cxt, 1);
     TEST_CHECK(
@@ -79,6 +70,46 @@ SENTRY_TEST(basic_transaction)
     sentry_value_decref(tx_cxt);
 }
 
+static void
+check_backfilled_name(sentry_envelope_t *envelope, void *data)
+{
+    uint64_t *called = data;
+    *called += 1;
+
+    sentry_value_t transaction = sentry_envelope_get_transaction(envelope);
+    TEST_CHECK(!sentry_value_is_null(transaction));
+    CHECK_STRING_PROPERTY(
+        transaction, "transaction", "<unlabeled transaction>");
+
+    sentry_envelope_free(envelope);
+}
+
+SENTRY_TEST(transaction_name_backfill_on_finish)
+{
+    uint64_t called = 0;
+
+    sentry_options_t *options = sentry_options_new();
+    sentry_options_set_dsn(options, "https://foo@sentry.invalid/42");
+
+    sentry_transport_t *transport = sentry_transport_new(check_backfilled_name);
+    sentry_transport_set_state(transport, &called);
+    sentry_options_set_transport(options, transport);
+
+    sentry_options_set_traces_sample_rate(options, 1.0);
+    sentry_init(options);
+
+    sentry_value_t transaction = sentry_value_new_transaction(NULL, NULL);
+    sentry_transaction_start(transaction);
+    sentry_transaction_finish();
+
+    transaction = sentry_value_new_transaction("", "");
+    sentry_transaction_start(transaction);
+    sentry_transaction_finish();
+
+    sentry_close();
+    TEST_CHECK_INT_EQUAL(called, 2);
+}
+
 static void
 send_transaction_envelope_test_basic(sentry_envelope_t *envelope, void *data)
 {

tests/unit/tests.inc

@@ -49,6 +49,7 @@ XX(session_basics)
 XX(slice)
 XX(symbolizer)
 XX(task_queue)
+XX(transaction_name_backfill_on_finish)
 XX(transactions_skip_before_send)
 XX(transport_sampling_transactions)
 XX(uninitialized)