getsentry / sentry-ruby

Sentry SDK for Ruby
https://sentry.io/for/ruby
MIT License
927 stars 486 forks source link

Add extra data in examples and/or handle ** args #2299

Closed nhorton closed 3 months ago

nhorton commented 3 months ago

We often get issues in code where somebody had an error condition where they had code like:

Sentry.capture_message("We hit an issue", my_method_arg: my_method_arg)

That is how structured logging is styled, and looks like it makes sense.

Sentry wants that instead to be:

Sentry.capture_message("We hit an issue", extra: {my_method_arg: my_method_arg})

The big problem with this though too is that if the only thing in the error handling was a Sentry capture, then devs often (udnerstandably) don't write a test for that case, and we only see the issue in prod when we get a syntax error on the above.

Couple things that would be nice:

  1. Add examples of extra data in the Ruby docs more. All the examples are the block style stuff with context, and not the more common style above
  2. Take ** args and pass everything you don't recognize as extras
  3. If you really can't do 2 for some reason, at least don't raise exceptions when RAILS_ENV == production for it.
st0012 commented 3 months ago

then devs often (udnerstandably) don't write a test for that case

IMO that's not a good assumption. Like anything else you'd rely on in a production environment, you should definitely cover error reporting logic with tests. For that reason, this SDK actually provides test helper, which you can access to with require "sentry/test_helper".

But at the same time, we didn't do a good job surfacing it to our users. I will open a PR to link API docs in the readme (#2300).

Take ** args and pass everything you don't recognize as extras

Ultimately, this depends on if Sentry want to give special treatments to extra attributes and how it could affect users' behaviour. While it's convenient for some users, it could, for example, lead to contexts being under-utilized? I'll let @sl0thentr0py make the call.

sl0thentr0py commented 3 months ago

Take ** args and pass everything you don't recognize as extras

hmm, we don't do this in other SDKs so I don't really want to change the semantics here, sorry!

I can make it clearer in the docs and also make sure it doesn't throw.

sl0thentr0py commented 3 months ago

actually see this note https://docs.sentry.io/platforms/ruby/enriching-events/context/#additional-data

so yes we do prefer that you send contexts instead of extra.