ref(session): align processing sequence in sentry__capture_event() with docs

src/sentry_core.c

@@ -389,6 +389,14 @@ sentry_capture_event(sentry_value_t event)
     }
 }
 
+bool
+sentry__roll_dice(double probability)
+{
+    uint64_t rnd;
+    return probability >= 1.0 || sentry__getrandom(&rnd, sizeof(rnd))
+        || ((double)rnd / (double)UINT64_MAX) <= probability;
+}
+
 sentry_uuid_t
 sentry__capture_event(sentry_value_t event)
 {
@@ -416,8 +424,15 @@ sentry__capture_event(sentry_value_t event)
                 mut_options->session->init = false;
                 sentry__options_unlock();
             }
-            sentry__capture_envelope(options->transport, envelope);
-            was_sent = true;
+
+            bool should_skip = !sentry__roll_dice(options->sample_rate);
+            if (should_skip) {
+                SENTRY_DEBUG("throwing away event due to sample rate");
+                sentry_envelope_free(envelope);
+            } else {
+                sentry__capture_envelope(options->transport, envelope);
+                was_sent = true;
+            }
         }
     }
     if (!was_captured) {
@@ -426,14 +441,6 @@ sentry__capture_event(sentry_value_t event)
     return was_sent ? event_id : sentry_uuid_nil();
 }
 
-bool
-sentry__roll_dice(double probability)
-{
-    uint64_t rnd;
-    return probability >= 1.0 || sentry__getrandom(&rnd, sizeof(rnd))
-        || ((double)rnd / (double)UINT64_MAX) <= probability;
-}
-
 bool
 sentry__should_send_transaction(sentry_value_t tx_cxt)
 {
@@ -461,12 +468,6 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event,
         sentry__record_errors_on_current_session(1);
     }
 
-    bool should_skip = !sentry__roll_dice(options->sample_rate);
-    if (should_skip) {
-        SENTRY_DEBUG("throwing away event due to sample rate");
-        goto fail;
-    }
-
     SENTRY_WITH_SCOPE (scope) {
         SENTRY_TRACE("merging scope into event");
         sentry_scope_mode_t mode = SENTRY_SCOPE_ALL;

tests/unit/test_basic.c

@@ -1,7 +1,6 @@
 #include "sentry_core.h"
 #include "sentry_database.h"
 #include "sentry_testsupport.h"
-#include "sentry_utils.h"
 
 static void
 send_envelope_test_basic(const sentry_envelope_t *envelope, void *data)
@@ -63,14 +62,20 @@ SENTRY_TEST(basic_function_transport)
     TEST_CHECK_INT_EQUAL(called, 2);
 }
 
+static void
+counting_transport_func(const sentry_envelope_t *UNUSED(envelope), void *data)
+{
+    uint64_t *called = data;
+    *called += 1;
+}
+
 static sentry_value_t
 before_send(sentry_value_t event, void *UNUSED(hint), void *data)
 {
     uint64_t *called = data;
     *called += 1;
 
-    sentry_value_decref(event);
-    return sentry_value_new_null();
+    return event;
 }
 
 SENTRY_TEST(sampling_before_send)
@@ -82,7 +87,7 @@ SENTRY_TEST(sampling_before_send)
     sentry_options_set_dsn(options, "https://foo@sentry.invalid/42");
     sentry_options_set_transport(options,
         sentry_new_function_transport(
-            send_envelope_test_basic, &called_transport));
+            counting_transport_func, &called_transport));
     sentry_options_set_before_send(options, before_send, &called_beforesend);
     sentry_options_set_sample_rate(options, 0.75);
     sentry_init(options);
@@ -94,9 +99,46 @@ SENTRY_TEST(sampling_before_send)
 
     sentry_close();
 
+    // The behavior here has changed with version 0.4.19:
+    // the documentation (https://develop.sentry.dev/sdk/sessions/#filter-order)
+    // requires that the sampling-rate filter for all SDKs is executed last.
+    // This means the `before_send` callback will be invoked every time and only
+    // the actual transport will be randomly sampled.
+    TEST_CHECK(called_transport > 50 && called_transport < 100);
+    TEST_CHECK_INT_EQUAL(called_beforesend, 100);
+}
+
+static sentry_value_t
+discarding_before_send(sentry_value_t event, void *UNUSED(hint), void *data)
+{
+    uint64_t *called = data;
+    *called += 1;
+
+    sentry_value_decref(event);
+    return sentry_value_new_null();
+}
+
+SENTRY_TEST(discarding_before_send)
+{
+    uint64_t called_beforesend = 0;
+    uint64_t called_transport = 0;
+
+    sentry_options_t *options = sentry_options_new();
+    sentry_options_set_dsn(options, "https://foo@sentry.invalid/42");
+    sentry_options_set_transport(options,
+        sentry_new_function_transport(
+            counting_transport_func, &called_transport));
+    sentry_options_set_before_send(
+        options, discarding_before_send, &called_beforesend);
+    sentry_init(options);
+
+    sentry_capture_event(
+        sentry_value_new_message_event(SENTRY_LEVEL_INFO, NULL, "foo"));
+
+    sentry_close();
+
     TEST_CHECK_INT_EQUAL(called_transport, 0);
-    // well, its random after all
-    TEST_CHECK(called_beforesend > 50 && called_beforesend < 100);
+    TEST_CHECK_INT_EQUAL(called_beforesend, 1);
 }
 
 SENTRY_TEST(crash_marker)

tests/unit/tests.inc

@@ -18,15 +18,16 @@ XX(child_spans)
 XX(concurrent_init)
 XX(concurrent_uninit)
 XX(count_sampled_events)
-XX(crashed_last_run)
 XX(crash_marker)
+XX(crashed_last_run)
 XX(custom_logger)
+XX(discarding_before_send)
 XX(distributed_headers)
 XX(drop_unfinished_spans)
 XX(dsn_parsing_complete)
 XX(dsn_parsing_invalid)
-XX(dsn_store_url_without_path)
 XX(dsn_store_url_with_path)
+XX(dsn_store_url_without_path)
 XX(empty_transport)
 XX(fuzz_json)
 XX(init_failure)