Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Provide set_trace #1137

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

feat: Provide set_trace #1137

wants to merge 10 commits into from

Conversation

bitsandfoxes
Copy link
Contributor

Motivation

Looking at it from one of the upstream SDKs (i.e. Android SDK, Unity SDK) there is currently no way to connect errors and events from their environment with the events sent by sentry-native. The idea is to provide the basic Tracing without Performance functionality and allow the SDK to continue a trace. The trace ID can then be used to link those events.

Copy link

github-actions bot commented Jan 30, 2025

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Provide `set_trace` ([#1137](https://github.com/getsentry/sentry-native/pull/1137))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 04453cc

src/sentry_core.c Outdated Show resolved Hide resolved
@bitsandfoxes bitsandfoxes changed the title feat: Provide set_trace_id to continue trace feat: Provide set_trace_id Feb 6, 2025
@bitsandfoxes bitsandfoxes changed the title feat: Provide set_trace_id feat: Provide set_trace Feb 6, 2025
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 82.51%. Comparing base (03143a8) to head (04453cc).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1137      +/-   ##
==========================================
- Coverage   82.66%   82.51%   -0.16%     
==========================================
  Files          53       53              
  Lines        7927     7942      +15     
  Branches     1239     1240       +1     
==========================================
  Hits         6553     6553              
- Misses       1263     1278      +15     
  Partials      111      111              

@bitsandfoxes bitsandfoxes requested review from supervacuus and markushi and removed request for supervacuus February 7, 2025 10:24
Copy link
Collaborator

@supervacuus supervacuus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides a changelog, I missed a basic test for the interaction with transactions. Something like this in test_tracing.c:

void
apply_scope_and_check_trace_context(
    sentry_options_t *options, const char *trace_id, const char *parent_span_id)
{
    // simulate scope application onto an event
    sentry_value_t event = sentry_value_new_object();
    SENTRY_WITH_SCOPE (scope) {
        sentry__scope_apply_to_event(scope, options, event, SENTRY_SCOPE_NONE);
    }

    // check that the event has a trace context
    sentry_value_t event_contexts = sentry_value_get_by_key(event, "contexts");
    TEST_CHECK(!sentry_value_is_null(event_contexts));
    TEST_CHECK(
        sentry_value_get_type(event_contexts) == SENTRY_VALUE_TYPE_OBJECT);

    sentry_value_t event_trace_context
        = sentry_value_get_by_key(event_contexts, "trace");
    TEST_CHECK(!sentry_value_is_null(event_trace_context));
    TEST_CHECK(
        sentry_value_get_type(event_trace_context) == SENTRY_VALUE_TYPE_OBJECT);

    // check trace context content
    const char *event_trace_id = sentry_value_as_string(
        sentry_value_get_by_key(event_trace_context, "trace_id"));
    TEST_CHECK_STRING_EQUAL(event_trace_id, trace_id);

    const char *event_trace_parent_span_id = sentry_value_as_string(
        sentry_value_get_by_key(event_trace_context, "parent_span_id"));
    TEST_CHECK_STRING_EQUAL(event_trace_parent_span_id, parent_span_id);

    sentry_uuid_t event_trace_span_id = sentry__value_as_uuid(
        sentry_value_get_by_key(event_trace_context, "span_id"));
    TEST_CHECK(!sentry_uuid_is_nil(&event_trace_span_id));

    sentry_value_decref(event);
}

SENTRY_TEST(set_trace_id_with_txn)
{
    // initialize SDK so we have a scope
    sentry_options_t *options = sentry_options_new();
    sentry_options_set_traces_sample_rate(options, 1.0);
    sentry_options_set_sample_rate(options, 1.0);
    sentry_init(options);

    // inject a trace via trace-header into a transaction
    const char *trace_header
        = "2674eb52d5874b13b560236d6c79ce8a-a0f9fdf04f1a63df-1";
    const char *txn_trace_id = "2674eb52d5874b13b560236d6c79ce8a";
    const char *txn_parent_span_id = "a0f9fdf04f1a63df";
    sentry_transaction_context_t *tx_ctx
        = sentry_transaction_context_new("wow!", NULL);
    sentry_transaction_context_update_from_header(
        tx_ctx, "sentry-trace", trace_header);
    sentry_transaction_t *tx
        = sentry_transaction_start(tx_ctx, sentry_value_new_null());

    // set the direct trace first
    const char *direct_trace_id = "aaaabbbbccccddddeeeeffff00001111";
    const char *direct_parent_span_id = "f0f0f0f0f0f0f0f0";
    sentry_set_trace(direct_trace_id, direct_parent_span_id);

    // events should get that trace applied
    apply_scope_and_check_trace_context(
        options, direct_trace_id, direct_parent_span_id);

    // now set a scoped transaction
    sentry_set_transaction_object(tx);

    // events should get the transaction's trace applied
    apply_scope_and_check_trace_context(
        options, txn_trace_id, txn_parent_span_id);

    sentry_transaction_finish(tx);

    // after finishing the transaction, the direct trace should hit again
    apply_scope_and_check_trace_context(
        options, direct_trace_id, direct_parent_span_id);

    sentry_close();
}

Comment on lines +1496 to +1498
/**
* Sets the trace ID.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is now part of the public API and could be used by all clients, not only downstream SDKs, it would be great to have some context here. Just basic information on what it is used for, what clients shouldn't use it for, and a link to conceptual docs (not developer docs), so clients understand the usage scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants