sentry-core: add traces_sampler client option

[tommilligan]

This PR adds the traces_sampler option to ClientOptions, and respects it when making transaction sampling decisions as detailed in the SDK guidelines: https://develop.sentry.dev/sdk/performance/#sdk-configuration

There was some prior work on this topic in #288, however by now it's pretty stale so I started fresh.

Points for review:

• To implement Debug for this option I just print the function's memory address (if set); I can't think of something simple that would have more value to the user. link
• If accepted, I would like to update the official docs with the new option, could someone point me in the right direction for that?
• Adding a new field to ClientOptions is a breaking change - I presume this necessitates a minor version bump for release?

[loewenheim]

Thanks for doing this!

To implement Debug for this option I just print the function's memory address (if set); I can't think of something simple that would have more value to the user. link

I think that's fine for now.

If accepted, I would like to update the official docs with the new option, could someone point me in the right direction for that?

I guess you would want to add it to this page. The file to change would be this one in the docs repo.

Adding a new field to ClientOptions is a breaking change - I presume this necessitates a minor version bump for release?

According to this guideline, it even requires a major bump because you're adding a new public field, but no private field currently exists.

[tommilligan]

Adding a new field to ClientOptions is a breaking change - I presume this necessitates a minor version bump for release?

According to this guideline, it even requires a major bump because you're adding a new public field, but no private field currently exists.

Right, agreed it's a breaking change - but all the sentry* packages are currently sub-v1, so I think a minor version bump is what's needed. I'll add a note in the changelog of the breakage anyway.

I don't like that adding a new config option breaks this for all users, but I think that's outside the scope of this PR.

[codecov-commenter]

Codecov Report

Merging #510 (b7d1a3e) into master (89b31f8) will increase coverage by 0.17%.
The diff coverage is 86.56%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #510      +/-   ##
==========================================
+ Coverage   78.60%   78.77%   +0.17%     
==========================================
  Files          76       76              
  Lines        9206     9240      +34     
==========================================
+ Hits         7236     7279      +43     
+ Misses       1970     1961       -9     

[Turbo87]

I've tried to make use of this new API, but I fail to understand how this is supposed to work. All fields on the TransactionContext struct are private, so even if the function gets passed such a struct there is still no information available to determine the sample rate for the transaction. Am I missing something? 🤔

Draft PR that shows the issue: rust-lang/crates.io#5423

[tommilligan]

Hi - yes, agreed. This was actually merged by a maintainer before I'd fully tested it, so sorry for the incomplete feature.

I've opened a follow up PR already which supports inspection of the transaction context, plus adding arbitrary user data as supported in e.g. the Python SDK. Comments welcome over there! #512

This isn't a breaking change so should be able to be added with a patch release fairly quickly once merged.