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

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

Open
ricksuggs opened this issue Apr 24, 2024 · 2 comments
Labels
community Was opened by a community member feature-request A request for a new feature or change to an existing one

Comments

@ricksuggs
Copy link

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 ricksuggs added community Was opened by a community member feature-request A request for a new feature or change to an existing one labels Apr 24, 2024
@nateberkopec
Copy link

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

@y9v
Copy link
Member

y9v commented Sep 9, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Was opened by a community member feature-request A request for a new feature or change to an existing one
Projects
None yet
Development

No branches or pull requests

3 participants