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

sentry-ruby (a.k.a. Ruby SDK 4.0) #1029

Closed
5 of 7 tasks
st0012 opened this issue Sep 17, 2020 · 10 comments
Closed
5 of 7 tasks

sentry-ruby (a.k.a. Ruby SDK 4.0) #1029

st0012 opened this issue Sep 17, 2020 · 10 comments
Assignees
Labels
Projects
Milestone

Comments

@st0012
Copy link
Collaborator

st0012 commented Sep 17, 2020

sentry-ruby (a.k.a. Ruby SDK 4.0)

The new sentry-ruby (a.k.a. Ruby SDK 4.0) will be a completely new gem. Its structure and main interfaces will follow the SDK guideline. This issue is to give the community a look at the future plan of the Ruby SDK and gather feedback from the users.

Please notice that things can change after the work begins. So nothing is guaranteed at this stage.

Things that may be different

  • The top-level namespace will be Sentry instead of Raven
  • This gem will have a different structure than the old one (see the New Structure section)
    • Some old API/components will be dropped/renamed
    • Some new concepts/components will be added
  • Configuration will be cascading
config.rails.report_rescued_exceptions = true
# instead of 
config.rails_report_rescued_exceptions = true
  • New APIs will be type-checked by sorbet
  • Ruby 2.3, 2.4 and Rails 4.2 might not be supported
  • Integrations like Rails or Sidekiq will be extracted to standalone gems like sentry-rails or sentry-sidekiq

Things that may remain the same

  • Most integrations should work in the same way (but can have different configuration option)
  • Release/environment detection

New Structure

# Usages
Sentry.init do |config|
  config.dsn = 'http://public@example.com/project-1'
end

Sentry.capture_message(message)
Sentry.capture_exception(exception)
event_id = Sentry.last_event_id

# the above is equivalant to
hub = Sentry.current_hub
hub.capture_message(message)
event_id = hub.last_event_id

# one of the biggest differences will be the new concept of Hub
# a Hub consists of a Client and 1 or many Scope
hub = Sentry::Hub.new(client, scope)

# and you can have multiple hubs with different clients/scopes at the same time
# for example:
client2 = Client.new
client2.configuration.dsn = 'http://public@example.com/project-2'
hub2 = Sentry::Hub.new(client2, scope2)
# this hub will send events to the project-2
hub2.capture_message(message)

Todos

  • send_default_pii option

send_default_pii is opt-in (default: false)
and if it's set to false we should not send any PII info with the SDK. usually this is request body, cookies, user IP address
only if the option is true we send those info
since we removed all the sanitze code in 4.0 we need at least that

@st0012 st0012 added this to the 4.0.0 milestone Sep 17, 2020
@st0012 st0012 added this to To do in 4.x via automation Sep 17, 2020
@st0012 st0012 self-assigned this Sep 17, 2020
@HazAT
Copy link
Member

HazAT commented Sep 17, 2020

Great job! This is already really good.

A few things:

@st0012
Copy link
Collaborator Author

st0012 commented Sep 17, 2020

4.0 should already only send envelopes (to prepare for Performance and Session). So send_event in the Transport should create an Envelope + there should be send_envelope. https://develop.sentry.dev/sdk/envelopes/

So every event should be wrapped in an envelope in 4.0? Does that mean we won't be using the event endpoint (api/#{project_id}/store/) anymore?

We need to add category based RateLimiting https://github.com/getsentry/sentry-electron/blob/master/src/main/transports/net.ts#L128-L165

If this is not urgent, I'd like to add it after the initial 4.0 is stabilized?

@HazAT
Copy link
Member

HazAT commented Sep 17, 2020

Yes, that means no more sending to /store/

and yes, we can ship it after the first version.

@stas
Copy link

stas commented Oct 15, 2020

@st0012 is there a fork/branch we could follow for updates?

@st0012
Copy link
Collaborator Author

st0012 commented Oct 16, 2020

@stas all the updates can be seen on the master branch, just under different folders. for example, the new sdk core is now located under the sentry-ruby folder

@st0012 st0012 added the plan label Oct 23, 2020
@st0012 st0012 unpinned this issue Nov 30, 2020
@HazAT
Copy link
Member

HazAT commented Dec 1, 2020

  • we should add a method called add_breadcrumb (instead of breadcrumbs.record) to add breadcrumbs
  • current_environment should just be called environment
  • Add toplevel functions for Sentry.setTags Sentry.setExtra Sentry.setUser so we don't have to use configure_scope for simple tasks
  • should_capture can be removed (I think) because we have before_send
  • can we rename environments to something more verbose like: enable_in_environments

@st0012
Copy link
Collaborator Author

st0012 commented Dec 1, 2020

Add toplevel functions for Sentry.setTags Sentry.setExtra Sentry.setUser so we don't have to use configure_scope for simple tasks

That makes sense, but we don't seem to have these functions in other SDKs AFAIK? I thought having configure_scope is to make users explicitly aware that they're adding data to a Scope. Are we now hiding the scope concept from the users?

should_capture can be removed (I think) because we have before_send

should_capture works by examining the raw value of the message or exception and it's run before the event is created, which can save the later computation. so I think this option still provides a different advantage than the before_send one.

I don't have a strong opinion on keeping/removing it though, just pointing out the difference.

@HazAT
Copy link
Member

HazAT commented Dec 1, 2020

The top level setX functions exist in JS and Python but you are right now everywhere.
We decided this later down the road so people who really only use the main functions, tags, breadcrumbs etc. don't need to understand the concept of the scope.

wrt should_capture before_send also takes a second argument called hint which can contain the raw exception or anything useful for deciding to discard the event or not.

@st0012
Copy link
Collaborator Author

st0012 commented Dec 3, 2020

should_capture runs right after the message/exception is passed into a client, before initializing an Event, while before_send runs like its name, just before sending the event. so one can argue that it saves a bit of computation.

but with that being said, I'll remove should_capture and just add hints to the before_send.

@st0012
Copy link
Collaborator Author

st0012 commented Dec 3, 2020

@HazAT the points you mentioned (except should_capture) are addressed in #1123
and here's the PR to add event hint support #1122
after it's merged I will drop the should_capture option

@st0012 st0012 closed this as completed Dec 10, 2020
4.x automation moved this from To do to Done Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
4.x
  
Done
Development

No branches or pull requests

3 participants