sentry-core: add traces_sampler client option#510
Conversation
|
Thanks for doing this!
I think that's fine for now.
I guess you would want to add it to this page. The file to change would be this one in the docs repo.
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 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 Report
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 |
|
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 Draft PR that shows the issue: rust-lang/crates.io#5423 |
|
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. |
This PR adds the
traces_sampleroption toClientOptions, and respects it when making transaction sampling decisions as detailed in the SDK guidelines: https://develop.sentry.dev/sdk/performance/#sdk-configurationThere was some prior work on this topic in #288, however by now it's pretty stale so I started fresh.
Points for review:
Debugfor 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. linkClientOptionsis a breaking change - I presume this necessitates a minor version bump for release?