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 rails error msg,stack,type to rack spans #1124

Merged
merged 4 commits into from
Jul 29, 2020

Conversation

ericmustin
Copy link
Contributor

This PR adds instrumentation to propagate Rails partial errors such as ActionView::Template::Error to the rack span. Currently the error details from this error will get set on the relevant Rails ActionController and ActionView spans, but not on the rack span which is the root of the trace.

Here's an example of this behavior:

image
image

The rack span gets set to error:true only because the status response code is >= 500, but lacks the relevant error info.

The underlying reason why these errors don't propagate to the Rails and Rack Middleware that dd-trace-rb instrumentation inserts is due to the default production behavior of the rails config setting action_dispatch.show_exceptions and the ActionDispatch::ShowExceptions middleware that the tracing middleware is inserted after

link

    def call(env)
      request = ActionDispatch::Request.new env
      @app.call(env)
    rescue Exception => exception
      if request.show_exceptions?
        render_exception(request, exception)
      else
        raise exception
      end
    end

Since action_dispatch.show_exceptions in practice defaults to true in production (technically it's unset in prod, but show_exceptions? explicitly checks if it's false.), The error gets swallowed before the Contrib::Rails::ErrorMiddlleware or the higher level Rack instrumentation can catch/re-raise it.

By propagating the error to the Rack span from the ActionController, we work around this constraint.

One tradeoff here is that, this might mean that if there's custom error handling middleware inserted in the Action Dispatcher middleware stack, we might not be honoring it by pre-emptively setting the rack span to be the same error as the actioncontroller.

@ericmustin ericmustin requested a review from a team July 27, 2020 23:55
Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

↗️↗️↗️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Involves a product feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants