Timeout for auto-generated transactions

[philipphofmann]

SentryTracer.waitForChildren waits for all child spans to finish. The downside of this is that when one of these spans never finishes, the SDK never finishes the transaction. We can solve this by, for example, by starting a timeout for the transaction, which is reset every time a new span is added. When this timeout exceeds, the SentryTracer finishes the transaction.

[philipphofmann]

We decided not to take care of this in the first iteration of automatic performance instrumentation. There is a risk that the SDK keeps multiple references of unfinished transactions, but a timeout might not be the only solution to solve this problem. Adding a timeout brings the risk of ending transactions too early.

[philipphofmann]

Related to getsentry/sentry-java#1811

[brustolin]

I believe we should tie a transaction with its target. If the target get out of scope we finish the transaction.
waitForChildren currently is only used for UI transactions.
We create those transactions at ViewController 'loadView' or 'viewWillAppear'. Let's say we have a child span that never ends, we have only one transaction in memory hanging forever, this will not really cause a memory problem.
When the ViewController disappear we can decide what to do with hanging transactions. And probably use timers here, because a ViewController can go away before a network request started with it finishes.

[kevinrenskers]

Look at idle transaction logic

[marandaneto]

If this is implemented, update the spec that was written in this PR getsentry/develop#766

[kevinrenskers]

@philipphofmann the SentryTracer class already has a bunch of idle timeout logic (dispatchIdleTimeout), but I am not really sure what the use of it is. It's executed by canBeFinished when there is a idleTimeout set and there are no children waiting, then waits for idleTimeout seconds and calls finishInternal.

The default value of idleTimeout is 0 though, so who is calling this with idleTimeout set to a non-zero value, and why? I don't really understand the use of it, especially since it only does something when there are no waiting children which feels like that's exactly the main point?

I can add a bunch of duplicate timeout logic next to it for the children to finish, but that feels wrong. Can we not use the existing logic, or would that break things?

[marandaneto]

The JS SDK has a flag called maxTransactionDuration, which defaults to 600 seconds, otherwise, the transaction is marked as deadline_exceeded.
Maybe we should align this behavior?

[philipphofmann]

I like the idea of maxTransactionDuration

The maximum duration of a transaction, measured in seconds, before it will be marked as "deadline_exceeded". If you never want transactions marked that way, set maxTransactionDuration to 0.

The default is 600.

I'm going to bring this to the TSC tomorrow and get back to you @kevinrenskers.

[kevinrenskers]

@philipphofmann is there any news about this? A decision?

[kevinrenskers]

When we start the transaction we start a timer of 30 seconds, for auto generated transactions. When the timer reaches 0 then we mark the transaction and the unfinished child spans as deadline_exceeded and call finishInternal.

When you finish the transaction, cancel the timer.

[philipphofmann]

Fixed with #2535

[marandaneto]

Fixed with #2535

Does it require any change on the spec? See comment.

[philipphofmann]

Yes, it does. I have it on my list and will update it.