diff --git a/CHANGELOG.md b/CHANGELOG.md index 84f5169a6..547da0656 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Breaking changes**: - Limiting the proguard rules in the NDK package moves the burden of the configuration to its users. Please ensure to [configure proguard](http://proguard.sourceforge.net/manual/examples.html#native) in your project so native methods in your namespace can be symbolicated if they appear in stack traces. ([#1250](https://github.com/getsentry/sentry-native/pull/1250)) +- When tags, contexts, and extra data are applied to events, the event data now takes precedence over the scope data as outlined in the [Hub & Scope Refactoring](https://develop.sentry.dev/sdk/miscellaneous/hub_and_scope_refactoring/#how-is-scope-data-applied-to-events) developer document and the linked RFC [code example](https://github.com/getsentry/rfcs/blob/fn/merge-hub-scope/text/0122-sdk-hub-scope-merge.md#applying-scopes). ([#1253](https://github.com/getsentry/sentry-native/pull/1253)) **Features**: @@ -14,6 +15,7 @@ - Reduce the scope of the proguard rules in the NDK package to local namespaces. ([#1250](https://github.com/getsentry/sentry-native/pull/1250)) - Close the file and return 0 on success when writing raw envelopes. ([#1260](https://github.com/getsentry/sentry-native/pull/1260)) +- Fix event tags, contexts, and extra data to take precedence when applying scope data. ([#1253](https://github.com/getsentry/sentry-native/pull/1253)) **Docs**: diff --git a/src/sentry_value.c b/src/sentry_value.c index fb6476b5b..9e573fe6f 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -977,7 +977,7 @@ sentry__value_merge_objects(sentry_value_t dst, sentry_value_t src) if (sentry__value_merge_objects(dst_val, src_val) != 0) { return 1; } - } else { + } else if (sentry_value_is_null(dst_val)) { if (sentry_value_set_by_key(dst, key, src_val) != 0) { return 1; } diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 7272dd3d5..23038f408 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -35,6 +35,7 @@ add_executable(sentry_test_unit test_path.c test_ratelimiter.c test_sampling.c + test_scope.c test_session.c test_slice.c test_symbolizer.c diff --git a/tests/unit/test_scope.c b/tests/unit/test_scope.c new file mode 100644 index 000000000..58e426d91 --- /dev/null +++ b/tests/unit/test_scope.c @@ -0,0 +1,132 @@ +#include "sentry.h" +#include "sentry_scope.h" +#include "sentry_testsupport.h" + +SENTRY_TEST(scope_contexts) +{ + SENTRY_TEST_OPTIONS_NEW(options); + sentry_init(options); + +#define TEST_CHECK_CONTEXT_EQUAL(event, key, value) \ + do { \ + sentry_value_t contexts = sentry_value_get_by_key(event, "contexts"); \ + TEST_CHECK_STRING_EQUAL( \ + sentry_value_as_string(sentry_value_get_by_key(contexts, key)), \ + value); \ + } while (0) + + // scope: {"both":"scope","scope":"scope"} + sentry_set_context("both", sentry_value_new_string("scope")); + sentry_set_context("scope", sentry_value_new_string("scope")); + + SENTRY_WITH_SCOPE (scope) { + // event: {"both":"event","event":"event"} + sentry_value_t event = sentry_value_new_object(); + { + sentry_value_t contexts = sentry_value_new_object(); + sentry_value_set_by_key( + contexts, "both", sentry_value_new_string("event")); + sentry_value_set_by_key( + contexts, "event", sentry_value_new_string("event")); + sentry_value_set_by_key(event, "contexts", contexts); + } + + // event <- scope: {"both":"event","event":"event","scope":"scope"} + sentry__scope_apply_to_event(scope, options, event, SENTRY_SCOPE_NONE); + TEST_CHECK_CONTEXT_EQUAL(event, "both", "event"); + TEST_CHECK_CONTEXT_EQUAL(event, "event", "event"); + TEST_CHECK_CONTEXT_EQUAL(event, "scope", "scope"); + + sentry_value_decref(event); + } + +#undef TEST_CHECK_CONTEXT_EQUAL + + sentry_close(); +} + +SENTRY_TEST(scope_extra) +{ + SENTRY_TEST_OPTIONS_NEW(options); + sentry_init(options); + +#define TEST_CHECK_EXTRA_EQUAL(event, key, value) \ + do { \ + sentry_value_t extra = sentry_value_get_by_key(event, "extra"); \ + TEST_CHECK_STRING_EQUAL( \ + sentry_value_as_string(sentry_value_get_by_key(extra, key)), \ + value); \ + } while (0) + + // scope: {"both":"scope","scope":"scope"} + sentry_set_extra("both", sentry_value_new_string("scope")); + sentry_set_extra("scope", sentry_value_new_string("scope")); + + SENTRY_WITH_SCOPE (scope) { + // event: {"both":"event","event":"event"} + sentry_value_t event = sentry_value_new_object(); + { + sentry_value_t extra = sentry_value_new_object(); + sentry_value_set_by_key( + extra, "both", sentry_value_new_string("event")); + sentry_value_set_by_key( + extra, "event", sentry_value_new_string("event")); + sentry_value_set_by_key(event, "extra", extra); + } + + // event <- scope: {"both":"event","event":"event","scope":"scope"} + sentry__scope_apply_to_event(scope, options, event, SENTRY_SCOPE_NONE); + TEST_CHECK_EXTRA_EQUAL(event, "both", "event"); + TEST_CHECK_EXTRA_EQUAL(event, "event", "event"); + TEST_CHECK_EXTRA_EQUAL(event, "scope", "scope"); + + sentry_value_decref(event); + } + +#undef TEST_CHECK_EXTRA_EQUAL + + sentry_close(); +} + +SENTRY_TEST(scope_tags) +{ + SENTRY_TEST_OPTIONS_NEW(options); + sentry_init(options); + +#define TEST_CHECK_TAG_EQUAL(event, key, value) \ + do { \ + sentry_value_t tags = sentry_value_get_by_key(event, "tags"); \ + TEST_CHECK_STRING_EQUAL( \ + sentry_value_as_string(sentry_value_get_by_key(tags, key)), \ + value); \ + } while (0) + + // scope: {"both":"scope","scope":"scope"} + sentry_set_tag("both", "scope"); + sentry_set_tag("scope", "scope"); + + SENTRY_WITH_SCOPE (scope) { + // event: {"both":"event","event":"event"} + sentry_value_t event = sentry_value_new_object(); + { + sentry_value_t tags = sentry_value_new_object(); + sentry_value_set_by_key( + tags, "both", sentry_value_new_string("event")); + sentry_value_set_by_key( + tags, "event", sentry_value_new_string("event")); + sentry_value_set_by_key(event, "tags", tags); + } + + // event <- scope: {"both":"event","event":"event","scope":"scope"} + sentry__scope_apply_to_event(scope, options, event, SENTRY_SCOPE_NONE); + TEST_CHECK_TAG_EQUAL(event, "both", "event"); + TEST_CHECK_TAG_EQUAL(event, "event", "event"); + TEST_CHECK_TAG_EQUAL(event, "scope", "scope"); + + sentry_value_decref(event); + } + +#undef TEST_CHECK_TAG_EQUAL + + sentry_close(); +} diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index ebb37deb1..40d0a2ae5 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -308,7 +308,7 @@ SENTRY_TEST(value_object_merge) sentry_value_t b = sentry_value_get_by_key(dst, "b"); sentry_value_t c = sentry_value_get_by_key(dst, "c"); TEST_CHECK_INT_EQUAL(sentry_value_as_int32(a), 1); - TEST_CHECK_INT_EQUAL(sentry_value_as_int32(b), 20); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(b), 2); TEST_CHECK_INT_EQUAL(sentry_value_as_int32(c), 30); sentry_value_decref(dst); @@ -340,7 +340,7 @@ SENTRY_TEST(value_object_merge_nested) sentry_value_t bc = sentry_value_get_by_key(nested, "bc"); TEST_CHECK_INT_EQUAL(sentry_value_as_int32(a), 1); TEST_CHECK_INT_EQUAL(sentry_value_as_int32(ba), 1); - TEST_CHECK_INT_EQUAL(sentry_value_as_int32(bb), 20); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(bb), 2); TEST_CHECK_INT_EQUAL(sentry_value_as_int32(bc), 30); sentry_value_decref(dst); diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 202b448a8..d1d887d53 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -88,6 +88,9 @@ XX(recursive_paths) XX(sampling_before_send) XX(sampling_decision) XX(sampling_transaction) +XX(scope_contexts) +XX(scope_extra) +XX(scope_tags) XX(scoped_txn) XX(sentry__value_span_new_requires_unfinished_parent) XX(serialize_envelope)