-
Notifications
You must be signed in to change notification settings - Fork 372
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
Error rescued with rescue_from still marked as error #1155
Comments
It is mentioned in #1124:
If I remember correctly, |
Hey @iain , thanks for reaching out about this. I think the goal of #1124 was to provide more visibility for user's top level spans, not significantly change current behavior around what is / is not marked an error. You bring up a valuable and reasonable use case. Looking into First, at present our instrumentation of rail's controllers simply is not accounting for
In this scenario, our instrumentation would record the error as being Second is that the most recent work in #1124 makes the first point more obvious. IIRC, previously rack spans were naively marked errors/not-error based on the response being a 500 or not, but now they directly inherit the rails controller errors. Since So this is a long and winding road of saying that we want to address this. I'll discuss with the team and keep this issue updated. In the meantime, if you need a workaround here I can suggest adding a Processor to the flushing pipeline that will adjust these 404 rack spans to not be reflected as errors
|
I see the Rack fixees in #1162, but I realized there is also a |
@iain So, for ActionMailer, we actually don't support it at the moment afaik, but if we did I think it would still exist as a child span of Rack, so the same fix would apply. And for ActiveJob, I believe our instrumentation exists only in the context of other libraries, like Sidekiq. So i'd have to see where the sidekiq middleware hooks are here, if they're a level above active job than it would probably be handled appropriately. We're still discussing internally long term whether there's a robust way we can instrument these hooks, in this case |
Makes sense. Thanks! |
Just upgraded to 0.40.0, and we were able to remove the workarounds mentioned above. Looking good, thanks! 👍 |
@iain very much appreciate the patience and confirmation here. We'll try to update here as we have more info regarding longer term work measuring rescue_from and before/after hooks |
We recently tried to upgrade from ddtrace 0.37.0 to 0.39.0 and we found that errors in a Rails controller are still counted as errors in APM.
For example:
APM would show that requests made to this endpoint as errors, while they are responding with 404 status.
We are using Rails 5.0.7.2, which admittedly is pretty old.
The text was updated successfully, but these errors were encountered: