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

Allow option to disable resource.name 404 rewriting #458

Closed
rymndhng opened this issue Aug 24, 2018 · 5 comments
Closed

Allow option to disable resource.name 404 rewriting #458

rymndhng opened this issue Aug 24, 2018 · 5 comments

Comments

@rymndhng
Copy link

The datadog documention recommends to reduce the cardinality of the resource.name, and most of the instrumentation is cognizant of this. What's the scenario where the Status404Decorator is useful?

The decorator here rewrites 404 responses with a custom resource.name. This was very confusing for me because I was expecting to look at the ratio of 404s to non-404s for my resource.

Can we add an option to turn this decorator off? I would like to have the ability to see my 404 responses with their original resource.name.

@realark
Copy link
Contributor

realark commented Aug 24, 2018

@rymndhng Is this instrumentation for a server? If so how would you know which resource to associate the 404 with?

For example, if you had two endpoints /my/service and /other/service, where would you expect a request /a/bad/request to appear?

@rymndhng
Copy link
Author

@realark I think I see where you're coming from. In the case where there is no tag resource.name set, it would be valuable to set resource.name to 404.

The current implementation always sets the resource.name to 404 when the response status is 404. I think it would be more reasonable to set the resource.name if it is not already set. This would users to distinguish between 404s from mismatched routes v.s. 404s returned by your application.

@tylerbenson
Copy link
Contributor

@rymndhng can you explain the circumstance where the 404 being returned by the application is useful to not group together?

@rymndhng
Copy link
Author

rymndhng commented Oct 18, 2018

The cirumstance where the 404s are not useful to be grouped together is because applications may implement generic routes with a different permission scheme. i.e. if I am seeing a huge number of 404s on the url pattern /foo/:id/bar/:id, I want to see the spans with 404s grouped under the same resource name /foo/:id/bar/:id instead of a catch-all 404.

@randomanderson
Copy link
Contributor

Resolved with #1152

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

No branches or pull requests

4 participants