strongloop-archive / strong-gateway

DEPRECATED | Our new gateway product is available at https://github.com/strongloop/microgateway.
Other
112 stars 46 forks source link

Report metrics via strong-express-metrics #31

Closed bajtos closed 9 years ago

bajtos commented 9 years ago

Close #30

/to @raymondfeng please review

bajtos commented 9 years ago

@raymondfeng What is the purpose of server/middleware/metrics.js? AFAICT, the middleware is not included in the server, can we remove it?


ATM, strong-express-metrics reports url values including query string, for example:

/api/notes?access_token=eodfHYn0mp8anxEUSDUpKdMeUEgzvx5T
/oauth/authorize?client_id=123&redirect_uri=https%3A%2F%2Flocalhost%3A3001%2Fclient-side-app.html&response_type=token&scope=demo&state=123

This has two consequences:

  1. The URLs displayed in the API metrics UI will most likely include query string too, which is most likely not what we want.
  2. The metrics are leaking potentially sensitive data like access tokens.

Regarding 1), I see two points of view:

Regarding 2), I don't think there is much we can do, strong-express-metris is not the only component logging URLs. Other proxies or e.g. Common Log include this information too (AFAIK). Clients should send the access token in a header, not as a query parameter.

@altsang @kraman @sam-github thoughts?

raymondfeng commented 9 years ago

LGTM.

bajtos commented 9 years ago

Let's move the discussion about request url, path and query to new issue - https://github.com/strongloop/strong-express-metrics/issues/4.