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

Docs are bad. Is extra still supported? #2453

Closed
bmulholland opened this issue Nov 1, 2024 · 14 comments
Closed

Docs are bad. Is extra still supported? #2453

bmulholland opened this issue Nov 1, 2024 · 14 comments
Assignees
Labels

Comments

@bmulholland
Copy link

https://docs.sentry.io/platforms/ruby/enriching-events/context/#additional-data says set_extra is deprecated

Errors can have a sentry_context method, and that will be added to Sentry. I can't find any documentation about it, was was removed/deprecated? If so, I can't find any migration guidance that would indicate what we should do instead. In any case, that method seems to be implemented in a way that uses extra. Does the above mean that sentry_context is deprecated? That would explain the lack of docs... but then again, what's the alternative?

There's event processors, which seems like a good approach. Happy to write one. Except the docs really don't document it well. What's the event object? Okay, well I guess I can guess that it's this one https://github.com/getsentry/sentry-ruby/blob/master/sentry-ruby/lib/sentry/event.rb. Though TBH I shouldn't have to dig through the source to try to figure this stuff out.

Also, all the docs seem to assume that I can just wrap my code in something to add context. But in this case, I want to pull out attributes of the error object itself and send those to Sentry. The docs just don't mention that uses case, although it should be bread-and-butter stuff for Sentry.

Now, I want to just add some basic values to the error, so that I get info about that error in Sentry. How do I find that? Looks like the relevant options are tags contexts extra, from the Event source. Tags are here: https://docs.sentry.io/platforms/ruby/enriching-events/tags/. That's not what I'm looking for, but useful.

That leaves me with contexts and extra. As I say above, I believe extra is deprecated, in favour of context? Okay, well I can use context then. That must be this: https://docs.sentry.io/platforms/ruby/enriching-events/context/.

So now, in an event processor, I want to add context.... how do I do that? What's the format? The docs here have a reference to set_context. But that's operating on a scope. I'm in an event processor, I have hint (oh BTW, what's that? where's the docs for it?). And I have an event.

When working with a system that's supposed to be processing errors, I really shouldn't have to be in a situation where I'm just deploying code and guessing whether the format is even right. But, again, reading the source, I see that set_context just sets values in the hash. So I guess I can just add to the context hash with my own object?

Now I'll write that code and deploy it, and hope that -- when an error does happen -- this all just works. But who knows!

@bmulholland
Copy link
Author

bmulholland commented Nov 1, 2024

Here's the scenario, BTW, which is probably a common enough scenario to be called out specifically: I'm integrating with an external service. When the request fails, I want to log the request_id to Sentry, so I can check logs on the sub-processor and correlate that with the error I'm investigating. The error object has a request_id field.

@sl0thentr0py
Copy link
Member

Extra is still supported but deprecated. We prefer contexts now, so the note here is correct.

I want to log the request_id to Sentry

The canonical way to do this is just add these values on the current scope as the example suggests:

Sentry.configure_scope do |scope|
  scope.set_context(:request:, { request_id: 'XXXX' })
  # or a tag which is indexed and searchable
  scope.set_tag(:request_id, 'XXX')
end

We know this is confusing sometimes, we will make this simpler in the future with top-level Sentry.set_context and Sentry.set_tag methods so no one needs to think of the scope and such. Thanks for the feedback. I will also tell the docs team to try and simplify the messaging.

Event processors are meant for advanced use cases where you need to do more transformation on the final event payload at the end of the pipeline, but you could equally do:

Sentry.configure_scope do |scope|
  scope.add_event_processor do |event, hint|
     event.contexts[:request] = { request_id: 'XXX' }
     event
  end
end

@bmulholland
Copy link
Author

@sl0thentr0py Thanks for the reply. I'm honestly still confused, though, and now a bit frustrated. You suggest I "just" call Sentry.configure_scope. But where would I do that?

Let's say there's a request in a gem that returns an error. The exception is raised, and there's no rescue block for it anywhere. So there's nowhere to put the code you suggest. I'd need something that intercepts that exception and adds the context. That doesn't exist -- especially if you say I shouldn't be using an event processor.

You say something about top-level, but this can't be added at any higher point; the request_id only exists -- by definition! -- after a request has been sent (indeed, it comes from the response to the request).

What am I missing?

@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 4, 2024
@sl0thentr0py
Copy link
Member

Okay, let's get into more detail. Typically in a rack/rails environment, the scope is _isolated_ for the request/response cycle and the exception is captured at the end of the flow.

So, in theory, the following should work (if it doesn't, I will need more info on your setup):

request_id = make_request('https://foobar.com')

Sentry.configure_scope do |scope|
  scope.set_context(:request:, { request_id: request_id })
end

This is because the exception will pick up this scope at the end of the rack request.
This is assuming make_request actually returns the request_id. If that is the code that raises, I will first need to understand where and how you get access to the request_id.

@sl0thentr0py
Copy link
Member

sl0thentr0py commented Nov 4, 2024

While building the SDK, we actually monkeypatch into popular gems and frameworks to closely follow lifecycle so maybe you can also tell me what gem you are using for making requests and I can give you better advice.

The problem here is you are retroactively trying to add state after the exception has happened, which is not that straightforward. The best place to add this state would be exactly where the exception happens and not somewhere up the chain.

@getsantry getsantry bot moved this to Waiting for: Community in GitHub Issues with 👀 3 Nov 4, 2024
@bmulholland
Copy link
Author

bmulholland commented Nov 4, 2024

Honestly I probably want this for unhandled exceptions from several gems. For starters, https://github.com/nylas/nylas-ruby and https://github.com/stytchauth/stytch-ruby. But there's probably others I'll want in future.

Monkeypatching every gem individually feels like something I'd prefer avoiding, though. Maintenance headache!

@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 4, 2024
@bmulholland
Copy link
Author

The problem here is you are retroactively trying to add state after the exception has happened

Well no, I'm not. The exceptions have the state, as a request_id parameter. I'm just trying to send that state to Sentry. Ideally in a way that doesn't couple application code directly to Sentry...

And in any case, this isn't code I directly control.

@sl0thentr0py
Copy link
Member

well we do support capturing all stack variables at the exception call site
https://docs.sentry.io/platforms/ruby/configuration/options/#include_local_variables
but that comes at a performance cost in ruby.

@bmulholland
Copy link
Author

I feel like we're really not on the same page here? That's overkill; these aren't local variables. They're an attribute of the exception itself.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 4, 2024
@sl0thentr0py
Copy link
Member

We can be on the same page if you give me detailed, precise information on your problem with code to understand (and ideally a reproduction) instead of ranting. If you write a wall of text, it's difficult for me to parse what exactly it is you want to solve.

I need to understand:

  • how you're making the request
  • how you're getting the request_id even when the request raises somewhere in the flow
  • a link to the exception class that is thrown so I can see how the class/attributes are set up

It is true that currently we don't serialize and send exception object attributes (because they are not standardized) but we could potentially make this work as an optional feature, just that no one asked for it so far.

@bmulholland
Copy link
Author

bmulholland commented Nov 4, 2024

Hey, sure, a few points:

Firstly, the wall of text was to give you all information about the journey a real person goes through trying to consume this gem, the Sentry product, and the documentation about all of this. As a startup founder, this sort of detailed real-user feedback is what I want from my users, because I want to work through and fix all those things. It's not intended for me, it's intended for you to use this information, to improve your product and docs, to help others who come later. If that isn't something that Sentry cares about, then sure -- next time, I'll provide less information. Let me know.

Secondly, I'm talking at two levels here: a general scenario, and a specific scenario I'm working on right now.

I can give you a very specific example. This Nylas error type is raised when running any number of requests. I've actually already got error handling code for this gem, because I want to translate it to more specific errors. So I've added your configure_scope code and am deploying that now; hopefully that solves my immediate problem.

However, this solution requires adding a rescue to every single place the code might raise an exception. I happen to already be doing that, for this example. However, if I wasn't doing that, it would feel very annoying to have to add a rescue and/or wrapping block for every place, just to do a "take this value from the exception and send it to Sentry." Even if that's actually the intended way to do this, the documentation should probably explicitly call that out. But again, this goes back to my first point -- improving the documentation isn't necessary here, for me, because you've helped me directly.

@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 4, 2024
@sl0thentr0py
Copy link
Member

sl0thentr0py commented Nov 4, 2024

okay I read through your stuff again

The exception is raised, and there's no rescue block for it anywhere.

If that is the case, you can simply add the rescue where you make the request and use Sentry.capture_exception(e) yourself with all the state you require instead of relying on our automatic capturing.

begin
   make_request(url)
rescue => e
   Sentry.capture_exception(e) do |scope|
     scope.set_context(:request:, { request_id: e.request_id })
   end
end

@sl0thentr0py
Copy link
Member

We always appreciate feedback and I've already passed this on to a couple of teams but in the SDK issue stream, we try to keep things technical - so bugs, feature requests etc that require a bit more concreteness.

I made a new feature request for automatically extracting exception attributes.
#2455

@bmulholland
Copy link
Author

bmulholland commented Nov 14, 2024

Thanks, @sl0thentr0py. Understood that this isn't the place for docs feedback. Logged getsentry/sentry-docs#11815

Before your message above, I had no idea that capture_X even took a block -- and I've had to look at the source to confirm that capture_message supports that too. So thanks for that. But yeah, the docs have some pretty big gaps.

I dug in and it appears that the change to contexts happened years ago. I've been using Sentry for 4-5 years though, and never noticed that. I don't see anything about it in the changelog, which I do usually check on major gem upgrades...

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

No branches or pull requests

2 participants