rfc(decision): Mobile - Tracing Without Performance V2

[philipphofmann]

This RFC aims to find a strategy to update the so traces don’t reference hundreds of unrelated
events.

Rendered RFC

[kahest]

Left some first thoughts. In general I don't see a lot of different options here - IMO it's mostly a combination of screen awareness, foreground/background handling and maybe some timeouts. So pretty much exactly your option 1

[kahest]

IIUC this is in line with the JS implementation, and something we can accept for now. In case we have clear pointers we need to address this (and how) we can iterate

[krystofwoldrich]

A manual API to renew the traceId could help with these edge cases.

[romtsn]

fyi, we have TracingUtils.startNewTrace in Java (should probably be exposed through the static API though)

[kahest]

For JPC there might be ways to make the 80% work with some KCP magic I guess (@markushi @romtsn wdyt?). But SwiftUI is a closed box and we'll need to mitigate this by providing clear ways/instructions on where to e.g. wrap something with a SDK method and/or manual API usage

[markushi]

I think the easiest way to get started is by providing a wrapper ~SentryScreen() { <Composable> }, in a later version we could have a @SentryScreen method annotation which utilizes KCP to auto-wrap it.

[bitsandfoxes]

What about additionally tying it to foreground/background behaviour like sessions? It doesn't have to be only one rule, right? It could be a set of rules and conditions that lead to the creation of a new traceId.

[markushi]

I think the easiest way to get started is by providing a wrapper ~SentryScreen() { <Composable> }, in a later version we could have a @SentryScreen method annotation which utilizes KCP to auto-wrap it.

[buenaflor]

for flutter at the very least we would need the user to use our SentryNavigatorObserver and then set a route name, e.g

navigatorObservers: [
  SentryNavigatorObserver()
],

MaterialPageRoute(
  settings: const RouteSettings(name: 'AutoCloseScreen'),
  builder: (context) => const AutoCloseScreen()),
),

Would the default behaviour remain as it is right now if a user didn't use the navigator observer on flutter?

[philipphofmann]

What's the current default behavior in Flutter, @buenaflor?

[krystofwoldrich]

There is similar issue with React Native. We don't have the screens/routes information without the performance instrumentation (ReactNavigation or ReactNativeNavigation).

We could update the auto instrumentation work without performance (without creating spans). Or get some signal of change from native.

Or having a public API to renew the traceId.

[Lms24]

We were recently made aware that another implication of long-running traces is that it potentially increases transaction/span quota usage. This is because in JS we inherit the sampling decision for the trace in subsequent transactions.

For example:

1. Pageload transaction is sampled by rolling the dice
2. PropagationContext stores positive sampling decision
3. Interaction transaction is started
4. Interaction transaction is sampled because the propagation context already holds a positive sampling decision
5. Repeat for every started transaction until next pageload or navigation

So either we accept this and move on for now by continuing with this behaviour or we break trace consistency by again rolling the dice for new root spans/transactions.

[philipphofmann]

@Lms24, please clarify why this wasn't a problem before. I don't understand how this proposed change here will cause this.

[Lms24]

I might be missing a bit of context around how Tracing without Performance and the PropagationContext is implemented in mobile SDKs.

If the proposed change is purely intended for TwP scenarios and does not affect the overall trace lifetime in case a root span/transaction is started ("tracing with performance") I think we're good. That is because for TwP, we defer the sampling decision to the downstream service (i.e. send sentry-trace headers without a sampled flag).

In JS however, we changed the trace lifetime not just for TwP but in general, leading to scenarios like the one above. To illustrate further, why this is problematic, I'm gonna adjust the example a bit from above

1. Initial Pageload transaction is sampled by rolling the dice
2. PropagationContext stores positive sampling decision
3. application, still on the same page but after the pageload span ended, makes an http request to a downstream service and propagates the sentry-trace header with the positive sampling decision, forcing the downstream service to positively sample their transaction.
4. repeat 3 a lot of times (e.g. an application auto-refreshing some state every 5s) and you have a lot of sampled transactions because one initial transaction was sampled positively in the FE.

So even without an active transaction, we'd still propagate a forced sampled flag to downstream services.

Does this make sense?

[philipphofmann]

Thanks for the detailed explanation. Yes, that makes sense, but I guess in the long run, you should have a roughly equal amount of transactions. It shouldn't matter if you roll the dice once for 10 transactions or every time for each transaction. If you roll the dice often enough, an equal amount of transactions should be captured.

[Lms24]

Not necessarily unfortunately. This would only hold up if the sample rates on client and server were the same. If users have lower sample rates on the server, they would send significantly less server-side transactions with the previous implementation.

I tried verifying this with a small script: https://gist.github.com/Lms24/9a631295aef58cf22fb8f5307953335c

[stefanosiano]

When a span starts, SDKs should use the traceID on the PropagationContext

We should say explicitly to use only the traceId, but not the sampling decision of the PropagationContext.
Regardless of client/server side sampling, i think we would break the sampling in general, as it doesn't apply to the PropagationContext. The sampler function is particularly problematic imho

[stefanosiano]

Wouldn't this break transaction/spans sample rates?
The sampled state of the PropagationContext would overwrite the calculated sampling decision of the span done through tracesSampleRate - or even worse through tracesSampler

[stefanosiano]

or are we just saying to use the traceId of the PropagationContext, retaining the usual sample decision?

[stefanosiano]

continuing below

[philipphofmann]

The RFC is on hold for now. We found some problems with the suggested approach in JS and need to revisit our approach. After we decided on how to continue we are either going to reopen this RFC or open another one.