diff --git a/examples/example.c b/examples/example.c index 96b23f907..0acff76d2 100644 --- a/examples/example.c +++ b/examples/example.c @@ -223,8 +223,8 @@ main(int argc, char **argv) sentry_transaction_set_sampled(tx_ctx, 0); } - sentry_value_t tx = sentry_transaction_start(tx_ctx); - sentry_transaction_finish(tx); + sentry_transaction_start(tx_ctx); + sentry_transaction_finish(); } // make sure everything flushes diff --git a/include/sentry.h b/include/sentry.h index f0b6ba622..932595d3a 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1289,14 +1289,13 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_sampled( * from an external integration (i.e. a span from a different SDK) * or manually constructed by a user. */ -SENTRY_EXPERIMENTAL_API sentry_value_t sentry_transaction_start( +SENTRY_EXPERIMENTAL_API void sentry_transaction_start( sentry_value_t transaction); /** - * Finishes and sends a transaction to sentry. + * Finishes and sends the current transaction to sentry. */ -SENTRY_EXPERIMENTAL_API void sentry_transaction_finish( - sentry_value_t transaction); +SENTRY_EXPERIMENTAL_API void sentry_transaction_finish(); #ifdef __cplusplus } diff --git a/src/sentry_core.c b/src/sentry_core.c index f3532e3e8..4b23ca03d 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -506,15 +506,6 @@ sentry__prepare_transaction(const sentry_options_t *options, { sentry_envelope_t *envelope = NULL; - bool should_skip = !sentry_value_is_true( - sentry_value_get_by_key(transaction, "sampled")); - if (should_skip) { - SENTRY_DEBUG("throwing away transaction due to sample rate"); - goto fail; - } - // Field is superfluous, strip so it doesn't leak into the payload - sentry_value_remove_by_key(transaction, "sampled"); - SENTRY_WITH_SCOPE (scope) { SENTRY_TRACE("merging scope into event"); // Don't include debugging info @@ -736,12 +727,11 @@ sentry_set_level(sentry_level_t level) } } -sentry_value_t +void sentry_transaction_start(sentry_value_t tx_cxt) { sentry_value_t tx = sentry_value_new_event(); - // TODO: stuff transaction into the scope bool should_sample = sentry__should_send_transaction(tx_cxt); sentry_value_set_by_key( tx, "sampled", sentry_value_new_bool(should_sample)); @@ -760,22 +750,35 @@ sentry_transaction_start(sentry_value_t tx_cxt) sentry_value_set_by_key( tx, "span_id", sentry__value_new_span_uuid(&span_id)); + sentry__scope_set_span(tx); sentry_value_decref(tx_cxt); - - return tx; } void -sentry_transaction_finish(sentry_value_t tx) +sentry_transaction_finish() { - sentry_value_t sampled = sentry_value_get_by_key(tx, "sampled"); - if (!sentry_value_is_null(sampled) && !sentry_value_is_true(sampled)) { - sentry_value_decref(sampled); - sentry_value_decref(tx); - // TODO: remove from scope + sentry_value_t tx = sentry_value_new_null(); + SENTRY_WITH_SCOPE (scope) { + if (sentry_value_is_null(scope->span)) { + SENTRY_DEBUG("could not find a transaction on the scope to finish"); + return; + } + sentry_value_t sampled + = sentry_value_get_by_key(scope->span, "sampled"); + if (!sentry_value_is_true(sampled)) { + SENTRY_DEBUG("throwing away transaction due to sample rate"); + sentry__scope_remove_span(); + return; + } + tx = sentry__value_clone(scope->span); + } + sentry__scope_remove_span(); + if (sentry_value_is_null(tx)) { + SENTRY_DEBUG("could not find a transaction on the scope to finish"); return; } - sentry_value_decref(sampled); + + sentry_value_remove_by_key(tx, "sampled"); sentry_value_set_by_key(tx, "type", sentry_value_new_string("transaction")); sentry_value_set_by_key(tx, "timestamp", diff --git a/src/sentry_core.h b/src/sentry_core.h index e6f5a1453..d149f1538 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -116,7 +116,7 @@ void sentry__options_unlock(void); for (sentry_options_t *Options = sentry__options_lock(); Options; \ sentry__options_unlock(), Options = NULL) -// these for now are only needed for tests +// these for now are only needed outside of core for tests #ifdef SENTRY_UNITTEST bool sentry__roll_dice(double probability); bool sentry__should_send_transaction(sentry_value_t tx_cxt); diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 3edf49888..99012fa9b 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -230,9 +230,30 @@ sentry__symbolize_stacktrace(sentry_value_t stacktrace) void sentry__scope_set_span(sentry_value_t span) { - // TODO: implement this function and get rid of this line. - (void)span; - return; + SENTRY_WITH_SCOPE_MUT (scope) { + sentry_value_decref(scope->span); + scope->span = span; + } +} + +#ifdef SENTRY_UNITTEST +sentry_value_t +sentry__scope_get_span() +{ + SENTRY_WITH_SCOPE (scope) { + return scope->span; + } + return sentry_value_new_null(); +} +#endif + +void +sentry__scope_remove_span() +{ + SENTRY_WITH_SCOPE_MUT (scope) { + sentry_value_decref(scope->span); + scope->span = sentry_value_new_null(); + } } void diff --git a/src/sentry_scope.h b/src/sentry_scope.h index 393cf349d..a4d0ce9b4 100644 --- a/src/sentry_scope.h +++ b/src/sentry_scope.h @@ -77,10 +77,23 @@ void sentry__scope_apply_to_event(const sentry_scope_t *scope, /** * Sets the span (actually transaction) on the scope. An internal way to pass - * around contextual information needed from a transaction into other events. + * around contextual information needed from a transaction into other events. If + * the scope already contains an unfinished transaction, that transaction will + * be discarded and will not be sent to sentry. + * + * This takes ownership of the span. */ void sentry__scope_set_span(sentry_value_t span); +/** + * Removes the current span (actually transaction) on the scope. If the + * transaction has not yet finished, this does not finish the transaction + * nor does it send it to sentry; The transaction will be discarded. + * + * Remove at your own discretion. + */ +void sentry__scope_remove_span(); + /** * These are convenience macros to automatically lock/unlock a scope inside a * code block. @@ -96,3 +109,8 @@ void sentry__scope_set_span(sentry_value_t span); sentry__scope_unlock(), Scope = NULL) #endif + +// this is only used in unit tests +#ifdef SENTRY_UNITTEST +sentry_value_t sentry__scope_get_span(); +#endif diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index aa8f4fdcb..647ae2ca8 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -1,7 +1,12 @@ +#include "sentry_scope.h" #include "sentry_testsupport.h" #include "sentry_tracing.h" #include "sentry_uuid.h" +#define CHECK_STRING_PROPERTY(Src, Field, Expected) \ + TEST_CHECK_STRING_EQUAL( \ + sentry_value_as_string(sentry_value_get_by_key(Src, Field)), Expected) + SENTRY_TEST(basic_tracing_context) { sentry_value_t span = sentry_value_new_object(); @@ -116,19 +121,19 @@ SENTRY_TEST(basic_function_transport_transaction) sentry_value_t transaction = sentry_value_new_transaction("How could you", "Don't capture this."); - transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_transaction_start(transaction); + sentry_transaction_finish(); sentry_user_consent_give(); transaction = sentry_value_new_transaction("honk", "beep"); - transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_transaction_start(transaction); + sentry_transaction_finish(); sentry_user_consent_revoke(); transaction = sentry_value_new_transaction( "How could you again", "Don't capture this either."); - transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_transaction_start(transaction); + sentry_transaction_finish(); sentry_close(); @@ -153,13 +158,13 @@ SENTRY_TEST(transport_sampling_transactions) for (int i = 0; i < 100; i++) { sentry_value_t transaction = sentry_value_new_transaction("honk", "beep"); - transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_transaction_start(transaction); + sentry_transaction_finish(); } sentry_close(); - // well, its random after all + // exact value is nondeterministic because of rng TEST_CHECK(called_transport > 50 && called_transport < 100); } @@ -191,11 +196,59 @@ SENTRY_TEST(transactions_skip_before_send) sentry_init(options); sentry_value_t transaction = sentry_value_new_transaction("honk", "beep"); - transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_transaction_start(transaction); + sentry_transaction_finish(); sentry_close(); TEST_CHECK_INT_EQUAL(called_transport, 1); TEST_CHECK_INT_EQUAL(called_beforesend, 0); } + +static void +before_transport(sentry_envelope_t *envelope, void *data) +{ + uint64_t *called = data; + *called += 1; + + sentry_envelope_free(envelope); +} + +SENTRY_TEST(multiple_transactions) +{ + uint64_t called_transport = 0; + + sentry_options_t *options = sentry_options_new(); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + + sentry_transport_t *transport = sentry_transport_new(before_transport); + sentry_transport_set_state(transport, &called_transport); + sentry_options_set_transport(options, transport); + + sentry_options_set_traces_sample_rate(options, 1.0); + sentry_init(options); + + sentry_value_t tx_cxt = sentry_value_new_transaction("wow!", NULL); + sentry_transaction_start(tx_cxt); + + sentry_value_t scope_tx = sentry__scope_get_span(); + CHECK_STRING_PROPERTY(scope_tx, "transaction", "wow!"); + + sentry_transaction_finish(); + scope_tx = sentry__scope_get_span(); + TEST_CHECK(sentry_value_is_null(scope_tx)); + + tx_cxt = sentry_value_new_transaction("whoa!", NULL); + sentry_transaction_start(tx_cxt); + tx_cxt = sentry_value_new_transaction("wowee!", NULL); + sentry_transaction_start(tx_cxt); + scope_tx = sentry__scope_get_span(); + CHECK_STRING_PROPERTY(scope_tx, "transaction", "wowee!"); + sentry_transaction_finish(); + + sentry_close(); + + TEST_CHECK_INT_EQUAL(called_transport, 2); +} + +#undef CHECK_STRING_PROPERTY diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 0c54353ed..2e79a18a8 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -29,6 +29,7 @@ XX(module_finder) XX(mpack_newlines) XX(mpack_removed_tags) XX(multiple_inits) +XX(multiple_transactions) XX(os) XX(page_allocator) XX(path_basics)