Add an overload for startTransaction that sets the created transaction to the Scope

[maciejwalkowiak]

📜 Description

Add an overload for startTransaction that sets the created transaction to the Scope

💡 Motivation and Context

Fixes #1252

💚 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

Breaking change: methods that previously did not set transaction on the scope, now do.

[maciejwalkowiak]

The number of startTransaction methods is getting dangerously long. We provided overloaded methods to simplify users life, but now with 10 methods the getting started experience may be a bit confusing. I think we should consider having separate method name for bound transactions or passing a builder-like method parameter where all the combinations could be set.

[codecov-io]

Codecov Report

Merging #1313 (6ad2f4f) into main (6af0a42) will increase coverage by 0.00%.
The diff coverage is 62.50%.

@@            Coverage Diff            @@
##               main    #1313   +/-   ##
=========================================
  Coverage     75.81%   75.82%           
- Complexity     1796     1803    +7     
=========================================
  Files           183      183           
  Lines          6227     6233    +6     
  Branches        622      623    +1     
=========================================
+ Hits           4721     4726    +5     
- Misses         1227     1229    +2     
+ Partials        279      278    -1     

Impacted Files	Coverage Δ	Complexity Δ
...sentry/spring/tracing/SentryTransactionAdvice.java	87.50% <ø> (-0.38%)	8.00 <0.00> (ø)
sentry/src/main/java/io/sentry/HubAdapter.java	8.77% <0.00%> (+0.15%)	4.00 <0.00> (ø)
sentry/src/main/java/io/sentry/NoOpHub.java	59.45% <ø> (+1.56%)	21.00 <0.00> (ø)
sentry/src/main/java/io/sentry/Sentry.java	43.16% <33.33%> (-0.87%)	20.00 <2.00> (+1.00)	⬇️
sentry/src/main/java/io/sentry/IHub.java	80.00% <83.33%> (ø)	12.00 <5.00> (+4.00)
.../io/sentry/spring/tracing/SentryTracingFilter.java	73.17% <100.00%> (-1.25%)	5.00 <0.00> (ø)
sentry/src/main/java/io/sentry/Hub.java	75.72% <100.00%> (+0.07%)	78.00 <1.00> (+1.00)
...src/main/java/io/sentry/transport/RateLimiter.java	78.30% <0.00%> (+0.94%)	28.00% <0.00%> (+1.00%)
.../main/java/io/sentry/transport/HttpConnection.java	79.59% <0.00%> (+1.02%)	21.00% <0.00%> (ø%)

---

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 6af0a42...6d990d8. Read the comment docs.

[marandaneto]

we always use the wording "bound to the scope" or "bind it to the scope", because of that, should we we call it bindToScope or smt similar? maybe @bruno-garcia has opinions

[maciejwalkowiak]

bindToScope sounds good. Thanks for suggestion!

[marandaneto]

The number of startTransaction methods is getting dangerously long. We provided overloaded methods to simplify users life, but now with 10 methods the getting started experience may be a bit confusing. I think we should consider having separate method name for bound transactions or passing a builder-like method parameter where all the combinations could be set.

I'd love Kotlin default params here, I generally agree but I don't see any more field coming to play, I'd merge as it is right now and if we get to add more, then yeah.

[marandaneto]

left a comment about the param naming, other than that, LGTM

[marandaneto]

@maciejwalkowiak too late but I guess we could have migrated the Android bits too, using bindToScope instead of doing it manually right