Add Telemetry integration#887
Conversation
solnic
left a comment
solnic
left a comment
There was a problem hiding this comment.
This is indeed a very useful thing to have! Left a couple of comments, otherwise LGTM 🚀
| interpolation_parameters: [inspect(metadata[:handler_id])] | ||
| ) | ||
|
|
||
| Sentry.capture_message("Telemetry handler %s failed", options) |
There was a problem hiding this comment.
Shouldn't this be a warning or even an error?
There was a problem hiding this comment.
This is already an error that gets reported to Sentry. Not sure what you mean here?
There was a problem hiding this comment.
@whatyouhide oh I just assumed that capture_message doesn't send an error by default, and sends an info-level message instead. IIRC Sentry displays and manages these messages differently, especially when it comes to notifications.
There was a problem hiding this comment.
I think you're thinking about logging. These get displayed as errors.
There was a problem hiding this comment.
@whatyouhide messages in Sentry do have levels too, and our SDK supports level option in capture_message. I just checked and luckily the default level is error.
One thing I noticed is that the UI displays "(no error message)" which may be a little confusing for the users.
2 participants
I think this would be a really helpful addition!
Corresponding docs PR.