Propose a new way to continue a trace

[cleptric]

This RFC proposes a new way to continue a trace when creating nested transactions.

Rendered RFC

Refs getsentry/develop#690

[marandaneto]

On Java, it's a bit different but similar https://docs.sentry.io/platforms/android/performance/instrumentation/custom-instrumentation/#distributed-tracing
We've not implemented the continueFromHeaders method, reason is here.

[marandaneto]

This would only work if you have access to the transaction instance, would you do that rather than adding spans to the running transaction?
Usually, continuing trace is used for distributed tracing, such as frontend <-> servers.

[cleptric]

The reason for this RFC was a use-case in getsentry/sentry itself. There are cases where we start transactions inside transactions.

Link to our [internal Slack conversation].(https://sentry.slack.com/archives/C043MQLSLTS/p1663598491050169)

[sl0thentr0py]

For completeness, these are the patterns of usage that currently break dynamic sampling.
https://github.com/getsentry/sentry/blob/a10312cf1f10af0a79a07b9895565963429dce24/src/sentry/ingest/ingest_consumer.py#L156-L168

[lobsterkatie]

I'd opt for exposing the option but giving it a reasonable default so that most people don't have to deal with it.

That said, I'd echo Manoel's question: In what circumstances is starting a new transaction in the same process better than just nesting spans?

[cleptric]

See #14 (comment)

[lobsterkatie]

Okay, yes, thanks for that. But I'm still not sure if the issue is what this fixes or the fact that maybe we really shouldn't be nesting transactions...

[Swatinem]

👍🏻 for the general idea. I implemented TransactionContext::continue_from_span in Rust since the very beginning since it felt like the right thing to do.

👎🏻 on a bool option that looks up the current Hub. Unless something changed, we modeled Transactions to be mostly independent from Hubs. There could be multiple concurrent transactions, and the decision which one is bound to the hub is up to the API user.

[sl0thentr0py]

I would ask @beezz, @untitaker etc to also maybe clarify the use case and if this is a workaround for some other problem because it is not entirely clear to many of us why starting a new transaction is necessary instead of just adding spans.
https://github.com/getsentry/sentry/blob/a10312cf1f10af0a79a07b9895565963429dce24/src/sentry/ingest/ingest_consumer.py#L156-L168

[sl0thentr0py]

Regarding preference, I agree with @Swatinem that a new method with explicit semantics is better than another magic bool variable that breaks hub assumptions on the current one.
The tradeoff here is that it won't magically make dynamic sampling work on those who use the current explicit startTransaction, they'll have to migrate their usages first.

[marandaneto]

A similar issue that would be solved by a solution to continue the trace thru the same process.
getsentry/sentry-dart#721
It works with the current workaround but this is semantically wrong because when you think about header, you think about external services (HTTP and so on).

[cleptric]

Thanks again for all your input! 🙂
I'm closing this for the moment and we can revisit it if the need for this will arise again in the future.

[mitsuhiko]

Can we revisit this? We ran into this again with monitors where we need to continue a trace (eg: create transaction context) but not yet create a transaction which I believe part of this RFC covers.

Refs getsentry/sentry#43647

[cleptric]

@mitsuhiko After talking with @evanpurkhiser and @davidenwang, it seems like this would be a different ask than the initial goal of this RFC.
What we had in mind here was to auto-magically continue a trace from startTransaction, the other use-case now being to propagate the trace without being in the context of a transaction.
Does this make sense?

[mitsuhiko]

@cleptric we can probably separate these things down but there are two paths we might want to look at:

1. we continue a trace independent of the transaction (eg: is that what happens if we continue from a cronjob?)
2. we do always continue a transaction, but create a clearer user experience around making a transaction when there is already one ongoing. (is that what we want?)

We can open a new RFC with a clearer set goal but unless we have an initial hunch where we are leaning here we might want to understand this first.

# Does this create a transaction or a trace. What are the consequences of either?
sentry_sdk.continue_trace_from_environment()