Skip to content

Fix sampling decision with sentry-trace and add more tests#1326

Merged
st0012 merged 3 commits intomasterfrom
fix-tracing-sampling
Mar 12, 2021
Merged

Fix sampling decision with sentry-trace and add more tests#1326
st0012 merged 3 commits intomasterfrom
fix-tracing-sampling

Conversation

@st0012
Copy link
Copy Markdown
Contributor

@st0012 st0012 commented Mar 11, 2021 •
edited
Loading

  1. Corrected sentry-trace's sampling bit interpretation.
    • empty bit means let the receiver SDK make the sampling decision
  2. Hub#start_transaction should return nil when tracing is disabled
  3. transaction generated from sentry-trace should still go through Hub#start_transaction
  4. Added more test cases to cover all the combinations described in this picture

trace sampling vs tracing activation

@st0012 st0012 added this to the 4.3.0 milestone Mar 11, 2021
@st0012 st0012 self-assigned this Mar 11, 2021
@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 11, 2021 •
edited
Loading

Codecov Report

Merging #1326 (a0f7d20) into master (7989855) will increase coverage by 0.63%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1326      +/-   ##
==========================================
+ Coverage   98.08%   98.71%   +0.63%     
==========================================
  Files         208      113      -95     
  Lines        9088     5295    -3793     
==========================================
- Hits         8914     5227    -3687     
+ Misses        174       68     -106
Impacted Files Coverage Δ
sentry-ruby/lib/sentry/rack/capture_exceptions.rb 94.87% <100.00%> (ø)
sentry-ruby/lib/sentry/transaction.rb 100.00% <100.00%> (ø)
...y-ruby/spec/sentry/rack/capture_exceptions_spec.rb 100.00% <100.00%> (ø)
...b/raven/integrations/sidekiq/cleanup_middleware.rb
sentry-raven/lib/raven/client.rb
...c/raven/integrations/sidekiq/error_handler_spec.rb
sentry-raven/lib/raven/interfaces/message.rb
sentry-raven/lib/raven/integrations/rack.rb
...raven/integrations/rails/controller_transaction.rb
...ec/raven/processors/sanitizedata_processor_spec.rb
... and 88 more

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 7989855...a0f7d20. Read the comment docs.

@st0012 st0012 requested a review from HazAT March 11, 2021 14:49
@lobsterkatie lobsterkatie mentioned this pull request Mar 12, 2021
Closed
@st0012 st0012 force-pushed the fix-tracing-sampling branch from 774b9e3 to 899dcb0 Compare March 12, 2021 07:58
@st0012 st0012 merged commit c8b9e4c into master Mar 12, 2021
@st0012 st0012 deleted the fix-tracing-sampling branch March 12, 2021 08:09
This was referenced Mar 12, 2021
Merged
Merged
Merged
Merged
Closed
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HazAT HazAT HazAT approved these changes

Assignees

@st0012 st0012

Labels

sentry-ruby Traces

Projects

None yet

Milestone

4.3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@st0012 @codecov-io @HazAT