-
-
Notifications
You must be signed in to change notification settings - Fork 495
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid duplicated sampling on Transaction events #1601
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1601 +/- ##
==========================================
- Coverage 98.47% 98.44% -0.03%
==========================================
Files 130 130
Lines 7223 7223
==========================================
- Hits 7113 7111 -2
- Misses 110 112 +2
Continue to review full report at Codecov.
|
Transport should only care about sending the event object to the right destination with right format. It doesn't need to check if it's allowed to send an event, which should already be done by `Client#capture_event`.
0154afb
to
c623b0e
Compare
c623b0e
to
915a105
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx for the quick fix @st0012!
Looks good to my newbie eyes, just one minor optimization suggestion but can also ignore
Co-authored-by: Neel Shah <neelshah.sa@gmail.com>
As @sl0thentr0py pointed out on discord, having
sample_allowed?
insidesending_allowed?
causes double sampling on transaction events and should be avoided. So this PR fixes the issue and removes an unnecessary sending check from theTransport
class.