Skip to content

Commit

Permalink
fix: store transaction data in event extra (#986)
Browse files Browse the repository at this point in the history
  • Loading branch information
supervacuus authored May 8, 2024
1 parent 46456ca commit 97b9c76
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**:

Expand Down
6 changes: 6 additions & 0 deletions examples/example.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(
Expand Down
68 changes: 30 additions & 38 deletions src/sentry_tracing.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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));
}
}

Expand All @@ -600,23 +591,24 @@ 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));
}
}

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);
}
}

Expand Down
13 changes: 9 additions & 4 deletions tests/assertions.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,26 @@ 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",
"release": release,
"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",
Expand Down
6 changes: 5 additions & 1 deletion tests/test_integration_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/test_tracing.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 97b9c76

Please sign in to comment.