Skip to content
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

Support transaction (part 1) #1108

Merged
merged 6 commits into from
Nov 20, 2020
Merged

Support transaction (part 1) #1108

merged 6 commits into from
Nov 20, 2020

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented Nov 20, 2020

This PR builds the groundwork for the performance monitoring feature. It can already send the new "transaction" type data to the server now.

This feature can be enabled by setting:

Sentry.init do |config|
  # any float larger than 0.0 to activate the feature
  # but it doesn't do the sampling now and sends every transaction
  config.traces_sample_rate = 1.0
end

Every event happened inside a transaction will now have a new Trace Details section:

截圖 2020-11-20 下午3 48 15


By clicking Search By Trace, users can see all the events associated with that transaction, including the transaction event itself.

截圖 2020-11-20 下午3 48 48


And this is how a transaction event looks like:

截圖 2020-11-20 下午3 49 14

Upcoming Features

  • Sampling mechanism
  • Different tracing events (currently only supports rack request and active record's SQL query)

@st0012 st0012 added this to the 4.0.0 milestone Nov 20, 2020
@st0012 st0012 self-assigned this Nov 20, 2020
@codecov-io
Copy link

codecov-io commented Nov 20, 2020

Codecov Report

Merging #1108 (8fba220) into master (fa1216c) will increase coverage by 0.11%.
The diff coverage is 99.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1108      +/-   ##
==========================================
+ Coverage   97.92%   98.04%   +0.11%     
==========================================
  Files         133      144      +11     
  Lines        5601     6087     +486     
==========================================
+ Hits         5485     5968     +483     
- Misses        116      119       +3     
Impacted Files Coverage Δ
...-ruby/spec/sentry/transport/http_transport_spec.rb 100.00% <ø> (ø)
sentry-ruby/spec/sentry/client_spec.rb 95.94% <94.11%> (-0.16%) ⬇️
...ls/lib/sentry/rails/tracing/abstract_subscriber.rb 94.44% <94.44%> (ø)
sentry-rails/spec/sentry/rails/tracing_spec.rb 97.29% <97.29%> (ø)
sentry-rails/lib/sentry/rails.rb 100.00% <100.00%> (ø)
sentry-rails/lib/sentry/rails/railtie.rb 85.00% <100.00%> (+1.21%) ⬆️
sentry-rails/lib/sentry/rails/tracing.rb 100.00% <100.00%> (ø)
...b/sentry/rails/tracing/active_record_subscriber.rb 100.00% <100.00%> (ø)
sentry-rails/spec/sentry/rails_spec.rb 100.00% <100.00%> (ø)
sentry-rails/spec/support/test_rails_app/app.rb 100.00% <100.00%> (ø)
... and 31 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 fa1216c...8fba220. Read the comment docs.

@st0012 st0012 requested a review from HazAT November 20, 2020 08:06
@st0012 st0012 force-pushed the support-transaction branch 6 times, most recently from a197d25 to 24506b7 Compare November 20, 2020 10:24
@st0012 st0012 force-pushed the support-transaction branch from 24506b7 to 1d50851 Compare November 20, 2020 10:42
@st0012 st0012 added the tracing label Nov 20, 2020
sentry-ruby/lib/sentry-ruby.rb Outdated Show resolved Hide resolved
sentry-ruby/lib/sentry-ruby.rb Outdated Show resolved Hide resolved
sentry-ruby/lib/sentry/hub.rb Outdated Show resolved Hide resolved
sentry-ruby/lib/sentry/hub.rb Outdated Show resolved Hide resolved
sentry-ruby/lib/sentry/span.rb Outdated Show resolved Hide resolved
sentry-ruby/lib/sentry/span.rb Outdated Show resolved Hide resolved
sentry-ruby/lib/sentry/transaction.rb Show resolved Hide resolved
@st0012 st0012 force-pushed the support-transaction branch from 48c9496 to 8fba220 Compare November 20, 2020 14:12
@st0012 st0012 requested a review from HazAT November 20, 2020 14:42
sampled_flag = ""
sampled_flag = @sampled ? 1 : 0 unless @sampled.nil?

"#{@trace_id}-#{@span_id}-#{sampled_flag}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here is a nit, if sampled is nil which is a valid value we would emit #{@trace_id}-#{@span_id}- with a trailing -

we should change that (eventho I think it would still work)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if @samped is nil, sampled_flag will be an empty string, which will add nothing to the string and become

"#{@trace_id}-#{@span_id}-"

so exactly the expected behavior 😄

here's the spec for such case https://github.com/getsentry/sentry-ruby/pull/1108/files#diff-a54e868d59f29d1f5369002784782b73b90c18c6493aab761e0230a51329fb6bR52-R61

@st0012 st0012 merged commit 44f347e into master Nov 20, 2020
@st0012 st0012 deleted the support-transaction branch November 20, 2020 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants