dd-trace-rb icon indicating copy to clipboard operation
dd-trace-rb copied to clipboard

Enhancement Request: Allow Manual Setting of Resource Name for Requests Failing in before_action

Open ricksuggs opened this issue 1 year ago • 2 comments

Hello ddtrace team,

I'm experiencing an issue with resource naming in traces when requests fail during a before_action callback in a Rails controller. Specifically, if an authentication fails in before_action, the trace's resource is captured as "GET 401" instead of "UserController#show". This behavior prevents us from grouping error responses (401, 403, 500, etc.) by the actual controller action that was called.

Here's a simplified version of my controller setup:

class UserController < ApplicationController
  before_action :authenticate

  def show
    user = User.find_by(token: token)
    ...
  end

  private

  def authenticate
    raise TokenExpiredException if token.expired?
  end
end

Upon debugging, I found that:

The resource name for successful requests is captured in Datadog::Tracing::Contrib::ActionPack::ActionController::Instrumentation#start_processing. If an exception is raised in the before_action, start_processing is never called. The default resource name is then set in Datadog::Tracing::Contrib::Rack::TraceMiddleware#set_request_tags! to a generic "GET 401". Given this behavior, I propose an enhancement where developers can manually set the trace's resource name earlier in the request lifecycle, perhaps through a custom middleware or directly within the before_action. This would provide greater flexibility in handling traces and allow for more accurate monitoring and debugging of applications.

Would this be considered for a future enhancement? Or is there an existing workaround that I might not be aware of?

ricksuggs avatar Apr 24 '24 23:04 ricksuggs

Might also be useful for responses which go to a rack middleware and never call the app (i.e. rack-cors)

nateberkopec avatar Aug 30 '24 03:08 nateberkopec

While not directly related to this issue, but just FYI: We are working currently on adding http.route to the tracer (PR). Having http.route on the trace might be helpful in such situations, since the route will be set earlier in the request lifecycle (we set it when the route is resolved).

y9v avatar Sep 09 '24 15:09 y9v