twitter-archive / diffy

Find potential bugs in your services with Diffy
https://twitter.com/diffyproject
Apache License 2.0
3.83k stars 368 forks source link

Services that do not return X-Action-Name header have a single "No_controller_reached" endpoint in the UI #6

Closed coderunner closed 9 years ago

coderunner commented 9 years ago

I would suggest to set the default endpoint name (HTTP case) to the requested path (no parameters, replacing all '/' with '_').

If that makes sense, I can issue a PR.

hasaziz commented 9 years ago

please do

puneetkhanduri commented 9 years ago

That approach would work as long as your URLs don't have any path params. Unfortunately, most (or at least enough) services do have path params in their URL structures. This leads to a very bad situation where almost every request ends up creating an endpoint and your UI gets cluttered with multiple endpoints that actually represent the same controller method.

One feature we would love to see is to do some cardinality analysis on the request URL patterns to dynamically identify path params without knowing anything about the target's internal controller routing logic. e.g.

/user/1/name /user/2/name /user/3/name ... might then be automatically collapsed to a single endpoint represented by:

/user/:path_param1/name

Without the ability to collapse endpoints the user experience degenerates very quickly which is why for the time being we require either the request to have a "Canonical-Resource" header or the responses from the targets to have and "X-Action-Name" header.

coderunner commented 9 years ago

I agree that my suggestion was far from ideal.

I suggested it because I thought this was a slightly better default. I think my main concern was that the displayed text No_controller_reached seemed to indicate some errors which was confusing to me. I noticed that the README was updated, but I wonder if No_controller_reached should be renamed to something like Undefined_endpoint or Unnamed_endpoint?

puneetkhanduri commented 9 years ago

Agreed. Undefined_endpoint sounds like a better choice than No_controller_reached. Please feel free to do the honors.