Support transaction waiting for children to finish.

[maciejwalkowiak]

📜 Description

Adds an option to create transaction that finishes only when it is itself finished and all its children are finished.

💡 Motivation and Context

Fixes #1523

💚 How did you test it?

Unit tests.

📝 Checklist

• I reviewed the submitted code
• I added tests to verify the changes
• I updated the docs if needed
• No breaking changes

🔮 Next steps

Reference docs.

[maciejwalkowiak]

@marandaneto as we do not want to make it a part of public API, but rather use it for Android integration, what API method on the IHub would you expect?

[marandaneto]

@maciejwalkowiak we probably still need a timeout feature, for spans that never finish or such other corner cases, iOS right now does not do it either, @philipphofmann @brustolin do you recall if we've decided to add on iOS too?

[marandaneto]

@marandaneto as we do not want to make it a part of public API, but rather use it for Android integration, what API method on the IHub would you expect?

to be honest, I think this would be an issue for Java Desktop Apps too, don't you? we can start exposing it to Hub with @ApiStatus.Internal, I'm not sure how we could do it otherwise, even if using SentryAndroid, looks like we still need something available in the Hub

[maciejwalkowiak]

Sure for desktop too, but you I thought you have exact use case in mind. Then I'll just add another variant of startTransaction.

[philipphofmann]

@maciejwalkowiak we probably still need a timeout feature, for spans that never finish or such other corner cases, iOS right now does not do it either, @philipphofmann @brustolin do you recall if we've decided to add on iOS too?

We didn't add it yet on iOS. I created an issue to discuss this today at grooming getsentry/sentry-cocoa#1173.

[marandaneto]

I have the impression that there's a bug.

val tr = startTranasction(x)
val span tr.startChild(x)
span.finish()
// at this point, tr has a single child and its finished, tr is gonna be finished automatically, it should not.
// `tr` should only auto finish once all children are finished and once somebody called `tr.finish(x)`

can you confirm my use case? thanks

[marandaneto]

@brustolin and @philipphofmann please review since this is done on iOS too

[philipphofmann]

I currently only have one comment to add.

[codecov-commenter]

Codecov Report

Merging #1535 (f4d2a2c) into main (1cdf98b) will increase coverage by 0.00%.
The diff coverage is 80.95%.

@@            Coverage Diff            @@
##               main    #1535   +/-   ##
=========================================
  Coverage     76.01%   76.01%           
- Complexity     1944     1953    +9     
=========================================
  Files           192      192           
  Lines          6725     6759   +34     
  Branches        670      676    +6     
=========================================
+ Hits           5112     5138   +26     
- Misses         1287     1294    +7     
- Partials        326      327    +1     

Impacted Files	Coverage Δ
sentry/src/main/java/io/sentry/HubAdapter.java	5.08% <0.00%> (-0.09%)	⬇️
sentry/src/main/java/io/sentry/IHub.java	90.47% <ø> (ø)
sentry/src/main/java/io/sentry/NoOpHub.java	47.50% <0.00%> (-1.22%)	⬇️
sentry/src/main/java/io/sentry/Sentry.java	41.49% <0.00%> (-0.58%)	⬇️
sentry/src/main/java/io/sentry/Hub.java	73.00% <66.66%> (-0.21%)	⬇️
sentry/src/main/java/io/sentry/SentryTracer.java	92.66% <89.65%> (-1.39%)	⬇️
sentry/src/main/java/io/sentry/Span.java	91.22% <100.00%> (+0.66%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cdf98b...f4d2a2c. Read the comment docs.

[maciejwalkowiak]

@marandaneto @bruno-garcia is there anything more do you think we should change here?

[marandaneto]

@marandaneto @bruno-garcia is there anything more do you think we should change here?

probably #1535 (comment) and #1528 (comment)

I'll discuss both this afternoon with the team, maybe the later could be already done

[bruno-garcia]

@maciejwalkowiak we probably still need a timeout feature, for spans that never finish or such other corner cases, iOS right now does not do it either, @philipphofmann @brustolin do you recall if we've decided to add on iOS too?

We could merge as-is and add the timeout later? It would unblock Android.

@maciejwalkowiak I am branched from your branch gh-1523, can we make an easy overload for such option? that allows name, op and waitForChildren, with or without startTimestamp

Could the overload be added to the other branch instead and we just merge this?