WIP: fix(tracing): finish active trace on crash

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 **Fixes**:
 
+- Finish active trace on crash. ([#1667](https://github.com/getsentry/sentry-native/pull/1667))
 - Reject overly deep msgpack payloads during deserialization. ([#1727](https://github.com/getsentry/sentry-native/pull/1727))
 - Read lengths for variadic fingerprints. ([#1730](https://github.com/getsentry/sentry-native/pull/1730))
 - Guard against JSON token allocation overflow on 32-bit platforms. ([#1733](https://github.com/getsentry/sentry-native/pull/1733))

examples/example.c

@@ -596,7 +596,8 @@ main(int argc, char **argv)
             options, sentry_transport_new(print_envelope));
     }
 
-    if (has_arg(argc, argv, "capture-transaction")) {
+    if (has_arg(argc, argv, "capture-transaction")
+        || has_arg(argc, argv, "open-transaction")) {
         sentry_options_set_traces_sample_rate(options, 1.0);
     }
 
@@ -1048,6 +1049,20 @@ main(int argc, char **argv)
         fflush(stdout);
     }
 
+    if (has_arg(argc, argv, "open-transaction")) {
+        // Leave a transaction + nested children unfinished; the crash
+        // auto-finalize should close them all.
+        sentry_transaction_context_t *otx_ctx
+            = sentry_transaction_context_new("open.tx", "op");
+        sentry_transaction_t *otx
+            = sentry_transaction_start(otx_ctx, sentry_value_new_null());
+        sentry_set_transaction_object(otx);
+        sentry_span_t *ospan
+            = sentry_transaction_start_child(otx, "open.span", NULL);
+        sentry_set_span(
+            sentry_span_start_child(ospan, "open.grand.span", NULL));
+    }
+
     if (has_arg(argc, argv, "crash")) {
         trigger_crash();
     }

src/backends/sentry_backend_breakpad.cpp

@@ -20,6 +20,7 @@ extern "C" {
 #include "sentry_session_replay.h"
 #include "sentry_string.h"
 #include "sentry_sync.h"
+#include "sentry_tracing.h"
 #include "sentry_transport.h"
 #include "sentry_unix_pageallocator.h"
 #include "transports/sentry_disk_transport.h"
@@ -139,6 +140,9 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor,
     SENTRY_WITH_OPTIONS (options) {
         sentry__write_crash_marker(options);
 
+        sentry_value_t transaction
+            = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED);
+
         bool should_handle = true;
 
         if (options->on_crash_func) {
@@ -240,9 +244,18 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor,
             }
 
             if (!sentry__launch_external_crash_reporter(options, envelope)) {
-                // capture the envelope with the disk transport
+                // capture the envelopes with the disk transport
                 sentry_transport_t *disk_transport
                     = sentry_new_disk_transport(options->run);
+                if (!sentry_value_is_null(transaction)) {
+                    sentry_envelope_t *tx_envelope
+                        = sentry__prepare_transaction(
+                            options, transaction, nullptr);
+                    if (tx_envelope) {
+                        sentry__capture_envelope(
+                            disk_transport, tx_envelope, options);
+                    }
+                }
                 sentry__capture_envelope(disk_transport, envelope, options);
                 sentry__transport_dump_queue(disk_transport, options->run);
                 sentry_transport_free(disk_transport);
@@ -256,6 +269,7 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor,
             SENTRY_SIGNAL_SAFE_LOG(
                 "DEBUG event was discarded by the `on_crash` hook");
             sentry_value_decref(event);
+            sentry_value_decref(transaction);
         }
 
         // after capturing the crash event, try to dump all the in-flight

src/backends/sentry_backend_inproc.c

@@ -18,6 +18,7 @@
 #include "sentry_screenshot.h"
 #include "sentry_session_replay.h"
 #include "sentry_sync.h"
+#include "sentry_tracing.h"
 #include "sentry_transport.h"
 #include "sentry_unix_pageallocator.h"
 #include "transports/sentry_disk_transport.h"
@@ -1115,6 +1116,9 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx,
         bool should_handle = true;
         sentry__write_crash_marker(options);
 
+        sentry_value_t transaction
+            = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED);
+
         if (options->on_crash_func && !skip_hooks) {
             SENTRY_DEBUG("invoking `on_crash` hook");
             event = options->on_crash_func(uctx, event, options->on_crash_data);
@@ -1144,9 +1148,6 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx,
 
             sentry_envelope_t *envelope = sentry__prepare_event(options, event,
                 NULL, !options->on_crash_func && !skip_hooks, NULL);
-            // TODO(tracing): Revisit when investigating transaction flushing
-            //                during hard crashes.
-
             sentry_session_t *session = sentry__end_current_session_with_status(
                 SENTRY_SESSION_STATUS_CRASHED);
             sentry__envelope_add_session(envelope, session);
@@ -1173,16 +1174,26 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx,
             }
 
             if (!sentry__launch_external_crash_reporter(options, envelope)) {
-                // capture the envelope with the disk transport
+                // capture the envelopes with the disk transport
                 sentry_transport_t *disk_transport
                     = sentry_new_disk_transport(options->run);
+                if (!sentry_value_is_null(transaction)) {
+                    sentry_envelope_t *tx_envelope
+                        = sentry__prepare_transaction(
+                            options, transaction, NULL);
+                    if (tx_envelope) {
+                        sentry__capture_envelope(
+                            disk_transport, tx_envelope, options);
+                    }
+                }
                 sentry__capture_envelope(disk_transport, envelope, options);
                 sentry__transport_dump_queue(disk_transport, options->run);
                 sentry_transport_free(disk_transport);
             }
         } else {
             SENTRY_DEBUG("event was discarded by the `on_crash` hook");
             sentry_value_decref(event);
+            sentry_value_decref(transaction);
         }
 
         // after capturing the crash event, dump all the envelopes to disk

src/backends/sentry_backend_native.c

@@ -37,6 +37,7 @@
 #include "sentry_scope.h"
 #include "sentry_session.h"
 #include "sentry_sync.h"
+#include "sentry_tracing.h"
 #include "sentry_transport.h"
 #include "transports/sentry_disk_transport.h"
 
@@ -988,6 +989,9 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx)
         // Write crash marker
         sentry__write_crash_marker(options);
 
+        sentry_value_t transaction
+            = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED);
+
         // Create crash event
         sentry_value_t event = sentry_value_new_event();
         sentry_value_set_by_key(
@@ -1092,26 +1096,33 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx)
                     = sentry__end_current_session_with_status(
                         SENTRY_SESSION_STATUS_CRASHED);
 
-                if (session) {
-                    sentry_envelope_t *envelope = sentry__envelope_new();
-                    if (envelope) {
-                        sentry__envelope_add_session(envelope, session);
-
-                        // Write session envelope to disk
-                        sentry_transport_t *disk_transport
-                            = sentry_new_disk_transport(options->run);
-                        if (disk_transport) {
-                            // sentry__capture_envelope takes ownership of
-                            // envelope
-                            sentry__capture_envelope(
-                                disk_transport, envelope, options);
-                            sentry__transport_dump_queue(
-                                disk_transport, options->run);
-                            sentry_transport_free(disk_transport);
-                        } else {
-                            // Failed to create transport, free envelope
-                            sentry_envelope_free(envelope);
+                if (session || !sentry_value_is_null(transaction)) {
+                    sentry_transport_t *disk_transport
+                        = sentry_new_disk_transport(options->run);
+                    if (disk_transport) {
+                        if (!sentry_value_is_null(transaction)) {
+                            sentry_envelope_t *tx_envelope
+                                = sentry__prepare_transaction(
+                                    options, transaction, NULL);
+                            if (tx_envelope) {
+                                sentry__capture_envelope(
+                                    disk_transport, tx_envelope, options);
+                            }
+                        }
+                        if (session) {
+                            sentry_envelope_t *envelope
+                                = sentry__envelope_new();
+                            if (envelope) {
+                                sentry__envelope_add_session(envelope, session);
+                                sentry__capture_envelope(
+                                    disk_transport, envelope, options);
+                            }
                         }
+                        sentry__transport_dump_queue(
+                            disk_transport, options->run);
+                        sentry_transport_free(disk_transport);
+                    } else {
+                        sentry_value_decref(transaction);
                     }
                 }
 
@@ -1124,6 +1135,7 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx)
         } else {
             SENTRY_DEBUG("event was discarded by the `on_crash` hook");
             sentry_value_decref(event);
+            sentry_value_decref(transaction);
         }
     }
 }

src/sentry_core.c

@@ -1329,6 +1329,20 @@ sentry_transaction_finish(sentry_transaction_t *opaque_tx)
 sentry_uuid_t
 sentry_transaction_finish_ts(
     sentry_transaction_t *opaque_tx, uint64_t timestamp)
+{
+    sentry_value_t tx = sentry__transaction_finish_value(opaque_tx, timestamp);
+    if (sentry_value_is_null(tx)) {
+        return sentry_uuid_nil();
+    }
+
+    // This takes ownership of the transaction, generates an event ID, merges
+    // scope
+    return sentry__capture_event(tx, NULL);
+}
+
+sentry_value_t
+sentry__transaction_finish_value(
+    sentry_transaction_t *opaque_tx, uint64_t timestamp)
 {
     if (!opaque_tx || sentry_value_is_null(opaque_tx->inner)) {
         SENTRY_WARN("no transaction available to finish");
@@ -1409,12 +1423,10 @@ sentry_transaction_finish_ts(
 
     sentry__transaction_decref(opaque_tx);
 
-    // This takes ownership of the transaction, generates an event ID, merges
-    // scope
-    return sentry__capture_event(tx, NULL);
+    return tx;
 fail:
     sentry__transaction_decref(opaque_tx);
-    return sentry_uuid_nil();
+    return sentry_value_new_null();
 }
 
 void
@@ -1569,6 +1581,8 @@ sentry_span_finish_ts(sentry_span_t *opaque_span, uint64_t timestamp)
         goto fail;
     }
 
+    sentry__transaction_remove_child(opaque_root_transaction, opaque_span);
+
     sentry_value_t root_transaction = opaque_root_transaction->inner;
 
     if (!sentry_value_is_true(

src/sentry_core.h

@@ -89,6 +89,9 @@ sentry_uuid_t sentry__capture_event(
 sentry_envelope_t *sentry__prepare_transaction(const sentry_options_t *options,
     sentry_value_t transaction, sentry_uuid_t *event_id);
 
+sentry_value_t sentry__transaction_finish_value(
+    sentry_transaction_t *opaque_tx, uint64_t timestamp);
+
 /**
  * This function will submit the `envelope` to the given `transport`, first
  * checking for consent.

src/sentry_sampling_context.h

@@ -2,7 +2,8 @@
 #ifndef SENTRY_SAMPLING_CONTEXT_H_INCLUDED
 #define SENTRY_SAMPLING_CONTEXT_H_INCLUDED
 
-#include "sentry_tracing.h"
+#include "sentry_boot.h"
+#include "sentry_value.h"
 
 typedef struct sentry_sampling_context_s {
     sentry_transaction_context_t *transaction_context;

src/sentry_scope.c

@@ -436,13 +436,20 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope,
         sentry__value_merge_objects(event_extra, scope->extra);
     }
 
+    bool is_transaction = sentry__event_is_transaction(event);
     sentry_value_t contexts = sentry__value_clone(scope->contexts);
+    if (is_transaction && !sentry_value_is_null(contexts)) {
+        sentry_value_remove_by_key(contexts, "trace");
+    }
 
     // prep contexts sourced from scope; data about transaction on scope needs
     // to be extracted and inserted
-    sentry_value_t scoped_txn_or_span = get_span_or_transaction(scope);
-    sentry_value_t scope_trace
-        = sentry__value_get_trace_context(scoped_txn_or_span);
+    sentry_value_t scoped_txn_or_span = sentry_value_new_null();
+    sentry_value_t scope_trace = sentry_value_new_null();
+    if (!is_transaction) {
+        scoped_txn_or_span = get_span_or_transaction(scope);
+        scope_trace = sentry__value_get_trace_context(scoped_txn_or_span);
+    }
     if (!sentry_value_is_null(scope_trace)) {
         if (sentry_value_is_null(contexts)) {
             contexts = sentry_value_new_object();
@@ -461,7 +468,7 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope,
     sentry_value_t event_contexts = sentry_value_get_by_key(event, "contexts");
     if (sentry_value_is_null(event_contexts)) {
         // only merge in propagation context if there is no scoped span
-        if (sentry_value_is_null(scope_trace)) {
+        if (!is_transaction && sentry_value_is_null(scope_trace)) {
             sentry__value_merge_objects(contexts, scope->propagation_context);
         }
         PLACE_VALUE("contexts", contexts);