feat(options): Add before_send_feedback hook

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+**Features**:
+
+- Add `sentry_options_set_before_send_feedback` to filter or enrich user feedback. Feedback does not go through `before_send`. ([#1923](https://github.com/getsentry/sentry-native/pull/1923))
+
 ## 0.16.0
 
 **Features**:

include/sentry.h

@@ -3766,6 +3766,30 @@ SENTRY_API void sentry_capture_feedback_with_hint(
 SENTRY_API sentry_uuid_t sentry_scope_capture_feedback(
     sentry_scope_t *scope, sentry_value_t user_feedback, sentry_hint_t *hint);
 
+/**
+ * Type of the `before_send_feedback` callback.
+ *
+ * The callback takes ownership of the `feedback` event and should usually
+ * return that same event. In case the feedback should be discarded, the
+ * callback needs to call `sentry_value_decref` on the provided event and
+ * return a `sentry_value_new_null()` instead.
+ *
+ * The hint can be used to add attachments to the in-flight feedback event.
+ *
+ * Feedback events do not go through the `before_send` callback.
+ */
+typedef sentry_value_t (*sentry_before_send_feedback_function_t)(
+    sentry_value_t feedback, sentry_hint_t *hint, void *user_data);
+
+/**
+ * Sets the `before_send_feedback` callback.
+ *
+ * See the `sentry_before_send_feedback_function_t` typedef above for more
+ * information.
+ */
+SENTRY_API void sentry_options_set_before_send_feedback(sentry_options_t *opts,
+    sentry_before_send_feedback_function_t func, void *user_data);
+
 /**
  * The status of a Span or Transaction.
  *

src/sentry_core.c

@@ -838,55 +838,68 @@ prepare_user_feedback(const sentry_options_t *options,
     sentry_value_set_by_key(
         event, "level", sentry__value_new_level(SENTRY_LEVEL_INFO));
 
-    sentry_attachment_t *all_attachments = NULL;
-    if (hint) {
-        sentry__attachments_extend(&all_attachments, hint->attachments);
-    }
-
     if (local_scope) {
+        SENTRY_DEBUG("merging local scope into feedback event");
         sentry__scope_apply_to_event(
             local_scope, options, event, SENTRY_SCOPE_NONE);
-        // must be taken before the scope is freed below
-        sentry__attachments_extend(&all_attachments, local_scope->attachments);
-        sentry__scope_free_one_shot(local_scope);
     }
     SENTRY_WITH_SCOPE (scope) {
+        SENTRY_DEBUG("merging global scope into feedback event");
         sentry__scope_apply_to_event(scope, options, event, SENTRY_SCOPE_NONE);
     }
 
+    if (options->before_send_feedback_func) {
+        SENTRY_DEBUG("invoking `before_send_feedback` hook");
+        event = options->before_send_feedback_func(
+            event, hint, options->before_send_feedback_data);
+        if (sentry_value_is_null(event)) {
+            SENTRY_DEBUG(
+                "feedback was discarded by the `before_send_feedback` hook");
+            sentry__client_report_discard(SENTRY_DISCARD_REASON_BEFORE_SEND,
+                SENTRY_DATA_CATEGORY_FEEDBACK, 1);
+            sentry__scope_free_one_shot(local_scope);
+            return NULL;
+        }
+    }
+
     sentry__ensure_event_id(event, event_id);
 
     sentry_envelope_t *envelope = sentry__envelope_new();
     if (!envelope || !sentry__envelope_add_feedback_event(envelope, event)) {
         goto fail;
     }
 
+    sentry_attachment_t *all_attachments = NULL;
+    if (hint) {
+        sentry__attachments_extend(&all_attachments, hint->attachments);
+    }
+    if (local_scope) {
+        sentry__attachments_extend(&all_attachments, local_scope->attachments);
+    }
+
     SENTRY_WITH_SCOPE (scope) {
+        const sentry_attachment_t *attachments = scope->attachments;
         if (all_attachments) {
-            // all attachments merged from the hint and the scopes
             sentry__attachments_extend(&all_attachments, scope->attachments);
-            sentry__envelope_add_attachments(
-                envelope, all_attachments, options);
-        } else {
-            // only the global scope has attachments
-            sentry__envelope_add_attachments(
-                envelope, scope->attachments, options);
+            attachments = all_attachments;
         }
+        sentry__envelope_add_attachments(envelope, attachments, options);
         if (options->run) {
-            sentry__cache_attachment_refs(envelope,
-                all_attachments ? all_attachments : scope->attachments, options,
+            sentry__cache_attachment_refs(envelope, attachments, options,
                 options->run->cache_path, options->run->run_path);
         }
     }
+
     sentry__attachments_free(all_attachments);
+    sentry__scope_free_one_shot(local_scope);
 
     return envelope;
 
 fail:
     SENTRY_WARN("dropping user feedback");
     sentry_envelope_free(envelope);
     sentry_value_decref(event);
-    sentry__attachments_free(all_attachments);
+    sentry__scope_free_one_shot(local_scope);
     return NULL;
 }
 
@@ -1832,6 +1845,10 @@ capture_feedback(sentry_value_t user_feedback, sentry_hint_t *hint,
     bool was_sent = false;
     SENTRY_WITH_OPTIONS (options) {
         was_captured = true;
+        // Give the hook something to attach to when the caller passed no hint.
+        if (!hint && options->before_send_feedback_func) {
+            hint = sentry_hint_new();
+        }
         sentry_envelope_t *envelope = prepare_user_feedback(
             options, user_feedback, hint, local_scope, &event_id);
         if (envelope) {

src/sentry_options.c

@@ -255,6 +255,14 @@ sentry_options_set_before_breadcrumb(sentry_options_t *opts,
     opts->before_breadcrumb_data = user_data;
 }
 
+void
+sentry_options_set_before_send_feedback(sentry_options_t *opts,
+    sentry_before_send_feedback_function_t func, void *user_data)
+{
+    opts->before_send_feedback_func = func;
+    opts->before_send_feedback_data = user_data;
+}
+
 void
 sentry_options_set_dsn_n(
     sentry_options_t *opts, const char *raw_dsn, size_t raw_dsn_len)

src/sentry_options.h

@@ -72,6 +72,8 @@ struct sentry_options_s {
     void *before_send_log_data;
     sentry_before_breadcrumb_function_t before_breadcrumb_func;
     void *before_breadcrumb_data;
+    sentry_before_send_feedback_function_t before_send_feedback_func;
+    void *before_send_feedback_data;
 
     /* Experimentally exposed */
     double traces_sample_rate;

tests/unit/test_feedback.c

@@ -21,7 +21,8 @@ send_envelope_test_feedback(sentry_envelope_t *envelope, void *_data)
 }
 
 static void
-setup_feedback_test(sentry_feedback_testdata_t *testdata)
+setup_feedback_test_with_before_send(sentry_feedback_testdata_t *testdata,
+    sentry_before_send_feedback_function_t func, void *user_data)
 {
     testdata->called = 0;
     sentry__stringbuilder_init(&testdata->serialized_envelope);
@@ -31,6 +32,7 @@ setup_feedback_test(sentry_feedback_testdata_t *testdata)
     sentry_options_set_dsn(options, "https://foo@sentry.invalid/42");
     sentry_options_set_release(options, "my-app@1.2.3");
     sentry_options_set_environment(options, "staging");
+    sentry_options_set_before_send_feedback(options, func, user_data);
     sentry_transport_t *transport
         = sentry_transport_new(send_envelope_test_feedback);
     sentry_transport_set_state(transport, testdata);
@@ -39,6 +41,12 @@ setup_feedback_test(sentry_feedback_testdata_t *testdata)
     sentry_init(options);
 }
 
+static void
+setup_feedback_test(sentry_feedback_testdata_t *testdata)
+{
+    setup_feedback_test_with_before_send(testdata, 0, NULL);
+}
+
 SENTRY_TEST(feedback_without_hint)
 {
     sentry_feedback_testdata_t testdata;
@@ -433,3 +441,198 @@ SENTRY_TEST(feedback_with_scope_and_hint)
 
     TEST_CHECK_INT_EQUAL(testdata.called, 1);
 }
+
+static sentry_value_t
+before_send_feedback_inspect(
+    sentry_value_t feedback, sentry_hint_t *hint, void *user_data)
+{
+    (void)hint;
+    uint64_t *called = user_data;
+    *called += 1;
+
+    // the hook runs after scopes have been applied
+    TEST_CHECK_STRING_EQUAL(
+        sentry_value_as_string(sentry_value_get_by_key(feedback, "release")),
+        "my-app@1.2.3");
+    TEST_CHECK_STRING_EQUAL(
+        sentry_value_as_string(sentry_value_get_by_key(
+            sentry_value_get_by_key(feedback, "tags"), "global_tag")),
+        "from_global");
+
+    return feedback;
+}
+
+SENTRY_TEST(feedback_before_send_receives_scope_data)
+{
+    sentry_feedback_testdata_t testdata;
+    uint64_t before_send_called = 0;
+    setup_feedback_test_with_before_send(
+        &testdata, before_send_feedback_inspect, &before_send_called);
+
+    sentry_set_tag("global_tag", "from_global");
+
+    sentry_uuid_t event_id
+        = sentry_uuid_from_string("4c035723-8638-4c3a-923f-2ab9d08b4018");
+    sentry_value_t feedback = sentry_value_new_feedback(
+        "test message", "test@example.com", "Test User", &event_id);
+
+    sentry_capture_feedback(feedback);
+
+    TEST_CHECK_INT_EQUAL(before_send_called, 1);
+
+    char *serialized
+        = sentry_stringbuilder_take_string(&testdata.serialized_envelope);
+    TEST_CHECK(strstr(serialized, "{\"type\":\"feedback\"") != NULL);
+    sentry_free(serialized);
+
+    sentry_close();
+
+    TEST_CHECK_INT_EQUAL(testdata.called, 1);
+}
+
+static sentry_value_t
+before_send_feedback_modify(
+    sentry_value_t feedback, sentry_hint_t *hint, void *user_data)
+{
+    (void)hint;
+    (void)user_data;
+    sentry_value_set_by_key(
+        feedback, "logger", sentry_value_new_string("modified-by-hook"));
+    return feedback;
+}
+
+SENTRY_TEST(feedback_before_send_can_modify)
+{
+    sentry_feedback_testdata_t testdata;
+    setup_feedback_test_with_before_send(
+        &testdata, before_send_feedback_modify, NULL);
+
+    sentry_uuid_t event_id
+        = sentry_uuid_from_string("4c035723-8638-4c3a-923f-2ab9d08b4018");
+    sentry_value_t feedback = sentry_value_new_feedback(
+        "test message", "test@example.com", "Test User", &event_id);
+
+    sentry_capture_feedback(feedback);
+
+    char *serialized
+        = sentry_stringbuilder_take_string(&testdata.serialized_envelope);
+    const char *item = strstr(serialized, "{\"type\":\"feedback\"");
+    TEST_ASSERT(item != NULL);
+    TEST_CHECK(strstr(item, "\"logger\":\"modified-by-hook\"") != NULL);
+    sentry_free(serialized);
+
+    sentry_close();
+
+    TEST_CHECK_INT_EQUAL(testdata.called, 1);
+}
+
+static sentry_value_t
+before_send_feedback_discard(
+    sentry_value_t feedback, sentry_hint_t *hint, void *user_data)
+{
+    (void)hint;
+    uint64_t *called = user_data;
+    *called += 1;
+    sentry_value_decref(feedback);
+    return sentry_value_new_null();
+}
+
+SENTRY_TEST(feedback_before_send_discards_with_scope_and_hint)
+{
+    sentry_feedback_testdata_t testdata;
+    uint64_t before_send_called = 0;
+    setup_feedback_test_with_before_send(
+        &testdata, before_send_feedback_discard, &before_send_called);
+
+    sentry_uuid_t event_id
+        = sentry_uuid_from_string("4c035723-8638-4c3a-923f-2ab9d08b4018");
+    sentry_value_t feedback = sentry_value_new_feedback(
+        "test message", "test@example.com", "Test User", &event_id);
+
+    // The scope and the hint are consumed by the discarding capture;
+    // ASan fails the test if they leak.
+    sentry_scope_t *local_scope = sentry_local_scope_new();
+    sentry_scope_attach_bytes(local_scope, "dropped", 7, "dropped.txt");
+
+    sentry_hint_t *hint = sentry_hint_new();
+    sentry_hint_attach_bytes(hint, "dropped", 7, "hint.txt");
+
+    sentry_uuid_t feedback_id
+        = sentry_scope_capture_feedback(local_scope, feedback, hint);
+
+    TEST_CHECK_INT_EQUAL(before_send_called, 1);
+    TEST_CHECK(sentry_uuid_is_nil(&feedback_id));
+    TEST_CHECK_INT_EQUAL(testdata.called, 0);
+
+    char *serialized
+        = sentry_stringbuilder_take_string(&testdata.serialized_envelope);
+    TEST_CHECK(strstr(serialized, "\"type\":\"feedback\"") == NULL);
+    sentry_free(serialized);
+
+    sentry_close();
+}
+
+static sentry_value_t
+before_send_feedback_attach(
+    sentry_value_t feedback, sentry_hint_t *hint, void *user_data)
+{
+    (void)user_data;
+    sentry_hint_attach_bytes(hint, "from hook", 9, "hook.txt");
+    return feedback;
+}
+
+SENTRY_TEST(feedback_before_send_can_attach_to_hint)
+{
+    sentry_feedback_testdata_t testdata;
+    setup_feedback_test_with_before_send(
+        &testdata, before_send_feedback_attach, NULL);
+
+    sentry_uuid_t event_id
+        = sentry_uuid_from_string("4c035723-8638-4c3a-923f-2ab9d08b4018");
+    sentry_value_t feedback = sentry_value_new_feedback(
+        "test message", "test@example.com", "Test User", &event_id);
+
+    sentry_hint_t *hint = sentry_hint_new();
+    sentry_hint_attach_bytes(hint, "before hook", 11, "before.txt");
+
+    // The callback adds an additional attachment to the hint.
+    sentry_capture_feedback_with_hint(feedback, hint);
+
+    char *serialized
+        = sentry_stringbuilder_take_string(&testdata.serialized_envelope);
+    TEST_CHECK(strstr(serialized, "\"filename\":\"before.txt\"") != NULL);
+    TEST_CHECK(strstr(serialized, "before hook") != NULL);
+    TEST_CHECK(strstr(serialized, "\"filename\":\"hook.txt\"") != NULL);
+    TEST_CHECK(strstr(serialized, "from hook") != NULL);
+    sentry_free(serialized);
+
+    sentry_close();
+
+    TEST_CHECK_INT_EQUAL(testdata.called, 1);
+}
+
+SENTRY_TEST(feedback_before_send_can_attach_without_hint)
+{
+    sentry_feedback_testdata_t testdata;
+    setup_feedback_test_with_before_send(
+        &testdata, before_send_feedback_attach, NULL);
+
+    sentry_uuid_t event_id
+        = sentry_uuid_from_string("4c035723-8638-4c3a-923f-2ab9d08b4018");
+    sentry_value_t feedback = sentry_value_new_feedback(
+        "test message", "test@example.com", "Test User", &event_id);
+
+    // No hint is passed; capture should create one so the callback can attach
+    // additional files to it.
+    sentry_capture_feedback(feedback);
+
+    char *serialized
+        = sentry_stringbuilder_take_string(&testdata.serialized_envelope);
+    TEST_CHECK(strstr(serialized, "\"filename\":\"hook.txt\"") != NULL);
+    TEST_CHECK(strstr(serialized, "from hook") != NULL);
+    sentry_free(serialized);
+
+    sentry_close();
+
+    TEST_CHECK_INT_EQUAL(testdata.called, 1);
+}