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

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

Closed
nhorton opened this issue Apr 18, 2024 · 3 comments · Fixed by #2301
Closed

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

nhorton opened this issue Apr 18, 2024 · 3 comments · Fixed by #2301
Assignees

Comments

@nhorton
Copy link

nhorton commented Apr 18, 2024

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
Copy link
Collaborator

st0012 commented Apr 20, 2024

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
Copy link
Member

sl0thentr0py commented Apr 23, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants