From 97b9c765fe7e02276e574b1762274fa0efe27558 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 8 May 2024 16:33:29 +0200 Subject: [PATCH] fix: store transaction `data` in event `extra` (#986) --- CHANGELOG.md | 1 + examples/example.c | 6 +++ src/sentry_tracing.c | 68 +++++++++++++++------------------- tests/assertions.py | 13 +++++-- tests/test_integration_http.py | 6 ++- tests/unit/test_tracing.c | 8 ++-- 6 files changed, 55 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a3a56019..6eedc7f48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Fixes**: - Allow `crashpad` to run under [Epic's Anti-Cheat Client](https://dev.epicgames.com/docs/game-services/anti-cheat/using-anti-cheat#external-crash-dumpers) by deferring the full `crashpad_handler` access rights to the client application until a crash occurred. ([#980](https://github.com/getsentry/sentry-native/pull/980), [crashpad#99](https://github.com/getsentry/crashpad/pull/99)) +- Store transaction `data` in the event property `extra` since the `data` property is discarded by `relay`. ([#986](https://github.com/getsentry/sentry-native/issues/986)) **Docs**: diff --git a/examples/example.c b/examples/example.c index 36a84bb44..5eac53ac8 100644 --- a/examples/example.c +++ b/examples/example.c @@ -387,6 +387,9 @@ main(int argc, char **argv) sentry_transaction_t *tx = sentry_transaction_start(tx_ctx, sentry_value_new_null()); + sentry_transaction_set_data( + tx, "url", sentry_value_new_string("https://example.com")); + if (has_arg(argc, argv, "error-status")) { sentry_transaction_set_status( tx, SENTRY_SPAN_STATUS_INTERNAL_ERROR); @@ -398,6 +401,9 @@ main(int argc, char **argv) sentry_span_t *grandchild = sentry_span_start_child(child, "littlest.teapot", NULL); + sentry_span_set_data( + child, "span_data_says", sentry_value_new_string("hi!")); + if (has_arg(argc, argv, "error-status")) { sentry_span_set_status(child, SENTRY_SPAN_STATUS_NOT_FOUND); sentry_span_set_status( diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index b32c30496..bbc7f00b4 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -512,24 +512,14 @@ sentry_span_remove_tag_n(sentry_span_t *span, const char *tag, size_t tag_len) } static void -set_data(sentry_value_t item, const char *key, sentry_value_t value) +set_data(sentry_value_t item, const char *data_key, size_t data_key_len, + const char *key, size_t key_len, sentry_value_t value) { - sentry_value_t data = sentry_value_get_by_key(item, "data"); + sentry_value_t data + = sentry_value_get_by_key_n(item, data_key, data_key_len); if (sentry_value_is_null(data)) { data = sentry_value_new_object(); - sentry_value_set_by_key(item, "data", data); - } - sentry_value_set_by_key(data, key, value); -} - -static void -set_data_n( - sentry_value_t item, const char *key, size_t key_len, sentry_value_t value) -{ - sentry_value_t data = sentry_value_get_by_key(item, "data"); - if (sentry_value_is_null(data)) { - data = sentry_value_new_object(); - sentry_value_set_by_key(item, "data", data); + sentry_value_set_by_key_n(item, data_key, data_key_len, data); } sentry_value_set_by_key_n(data, key, key_len, value); } @@ -538,50 +528,51 @@ void sentry_transaction_set_data( sentry_transaction_t *tx, const char *key, sentry_value_t value) { - if (tx) { - set_data(tx->inner, key, value); + if (key) { + sentry_transaction_set_data_n(tx, key, strlen(key), value); } } +static const char txn_data_key[] = "extra"; +static const size_t txn_data_key_len = sizeof(txn_data_key) - 1; + void sentry_transaction_set_data_n(sentry_transaction_t *tx, const char *key, size_t key_len, sentry_value_t value) { if (tx) { - set_data_n(tx->inner, key, key_len, value); + set_data( + tx->inner, txn_data_key, txn_data_key_len, key, key_len, value); } } void sentry_span_set_data(sentry_span_t *span, const char *key, sentry_value_t value) { - if (span) { - set_data(span->inner, key, value); + if (key) { + sentry_span_set_data_n(span, key, strlen(key), value); } } +static const char span_data_key[] = "data"; +static const size_t span_data_key_len = sizeof(span_data_key) - 1; + void sentry_span_set_data_n( sentry_span_t *span, const char *key, size_t key_len, sentry_value_t value) { if (span) { - set_data_n(span->inner, key, key_len, value); + set_data( + span->inner, span_data_key, span_data_key_len, key, key_len, value); } } static void -remove_data(sentry_value_t item, const char *key) +remove_data(sentry_value_t item, const char *data_key, size_t data_key_len, + const char *key, size_t key_len) { - sentry_value_t data = sentry_value_get_by_key(item, "data"); - if (!sentry_value_is_null(data)) { - sentry_value_remove_by_key(data, key); - } -} - -static void -remove_data_n(sentry_value_t item, const char *key, size_t key_len) -{ - sentry_value_t data = sentry_value_get_by_key(item, "data"); + sentry_value_t data + = sentry_value_get_by_key_n(item, data_key, data_key_len); if (!sentry_value_is_null(data)) { sentry_value_remove_by_key_n(data, key, key_len); } @@ -590,8 +581,8 @@ remove_data_n(sentry_value_t item, const char *key, size_t key_len) void sentry_transaction_remove_data(sentry_transaction_t *tx, const char *key) { - if (tx) { - remove_data(tx->inner, key); + if (key) { + sentry_transaction_remove_data_n(tx, key, strlen(key)); } } @@ -600,15 +591,15 @@ sentry_transaction_remove_data_n( sentry_transaction_t *tx, const char *key, size_t key_len) { if (tx) { - remove_data_n(tx->inner, key, key_len); + remove_data(tx->inner, txn_data_key, txn_data_key_len, key, key_len); } } void sentry_span_remove_data(sentry_span_t *span, const char *key) { - if (span) { - remove_data(span->inner, key); + if (key) { + sentry_span_remove_data_n(span, key, strlen(key)); } } @@ -616,7 +607,8 @@ void sentry_span_remove_data_n(sentry_span_t *span, const char *key, size_t key_len) { if (span) { - remove_data_n(span->inner, key, key_len); + remove_data( + span->inner, span_data_key, span_data_key_len, key, key_len); } } diff --git a/tests/assertions.py b/tests/assertions.py index 93f1b14ab..e9d86878c 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -55,10 +55,18 @@ def assert_meta( release="test-example-release", integration=None, transaction="test-transaction", + transaction_data=None, sdk_override=None, ): event = envelope.get_event() + extra = { + "extra stuff": "some value", + "…unicode key…": "őá…–🤮🚀¿ 한글 테스트", + } + if transaction_data: + extra.update(transaction_data) + expected = { "platform": "native", "environment": "development", @@ -66,10 +74,7 @@ def assert_meta( "user": {"id": 42, "username": "some_name"}, "transaction": transaction, "tags": {"expected-tag": "some value"}, - "extra": { - "extra stuff": "some value", - "…unicode key…": "őá…–🤮🚀¿ 한글 테스트", - }, + "extra": extra, } expected_sdk = { "name": "sentry.native", diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index d0ee056cb..02d00a07a 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -538,7 +538,11 @@ def test_transaction_only(cmake, httpserver, build_args): envelope.print_verbose() # The transaction is overwritten. - assert_meta(envelope, transaction="little.teapot") + assert_meta( + envelope, + transaction="little.teapot", + transaction_data={"url": "https://example.com"}, + ) # Extract the one-and-only-item (event,) = envelope.items diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index bdac0921f..bbc23107c 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -972,10 +972,10 @@ SENTRY_TEST(txn_data) sentry_transaction_set_data( txn, "os.name", sentry_value_new_string("Linux")); - check_after_set(txn->inner, "data", "os.name", "Linux"); + check_after_set(txn->inner, "extra", "os.name", "Linux"); sentry_transaction_remove_data(txn, "os.name"); - check_after_remove(txn->inner, "data", "os.name"); + check_after_remove(txn->inner, "extra", "os.name"); sentry__transaction_decref(txn); } @@ -1022,10 +1022,10 @@ SENTRY_TEST(txn_data_n) sentry_value_t data_value = sentry_value_new_string_n(data_v, sizeof(data_v)); sentry_transaction_set_data_n(txn, data_k, sizeof(data_k), data_value); - check_after_set(txn->inner, "data", "os.name", "Linux"); + check_after_set(txn->inner, "extra", "os.name", "Linux"); sentry_transaction_remove_data_n(txn, data_k, sizeof(data_k)); - check_after_remove(txn->inner, "data", "os.name"); + check_after_remove(txn->inner, "extra", "os.name"); sentry__transaction_decref(txn); }