Nordstrom / xrpc

Simple, production ready Java API server built on top of functional composition.
Apache License 2.0
17 stars 17 forks source link

Xrpc should log the request details along with response status code and time taken for request processing #181

Open shine17 opened 6 years ago

shine17 commented 6 years ago

Xrpc should stdout the incoming request details along with response status code and time taken for processing the request. The output format should be json which helps in kibana indexing. Below is a sample format This helps in kibana queries.

{ "json": { "responseTimeInMSec": 20, "requestUri": "/v2/authorize/8e97701c40f741d8a73d9ccf69358956", "requestProtocol": "HTTP/1.1", "requestHostName": "secure.nordstrom.com", "origin":"secure.nordstrom.com", "requestMethod": "GET", "requestUriQuery": "verifier=3S7741VKlb5Ck5IHUKJxYYSdum2VbGWatAU9WN4tKDt2I1kumHjXuETV7QlG52D3&code=52D8m-BeJFp7MWpckl8TNSYOHSR_lnbVfnNDMI6d1Q84v7BgYGgO87-WPaiwhv70Ed5fFtUeWA30S6Sv4P7M9g==", "applicationId": "authorize-service", "requestContext": "unique guid for the http request", "responseStatusCode": "200", "requestHeaders":{ "X-Nor-Clientid":"something", "Accept":"application/json" }, "responseHeaders":{ "Content-Type": "application/json" }, "timestamp": "2018-04-23T04:22:37.150Z", "environmentVariables": { "environmentName": "prod", "podName":"something", "imageName":"something" } } } origin : if origin header present in the request then log it requestHostName: if host header present in the log, then log it. requestContext - xrpc should generate a guid for each request which uniquely identify that request. This should be exposed as a global getter so that users can use this value along with error log messages , upstream time taken logs etc. this should be also part for the request log. applicationId - should be configurable value in xrpc config environmentVariables - xrpc should have config definition which allows users to specify which all the environment values xrpc needs to read and include to json log output for request. requestHeaders - should display all the request headers in the above format. users should be given the choice to specify which all request header values to be masked(eg: authorization header , apikey header , also mask last few characters). Users can define for those headers which needs to be masked in the xrpc config requestUriQuery - same as request headers, masking capability needed.

jkinkead commented 6 years ago

Other frameworks log this to a dedicated request log appender. We should do the same - and make request logging optional. This isn't going to be desirable outside of enterprise, and won't be wanted if there's a logging proxy in place (like nginx).

We could also allow the client to provide a log appender as a configuration item, and only log if it's present.

xjdr commented 6 years ago

Agreed. This should be configurable and not the default. Also, stdout is probably not desirable for production applications (k8s excluded). Kinesis, Kafka, etc, should be configurable appenders for this type of logging.

andyday commented 6 years ago

elastic beanstalk defaults to logging to stdout. many of the non-serverless and non-k8s applications are using this. the format (json or not) should be configurable ala logback config, but in order to allow for such json, the log writing needs to be structured enough to convert to json. logrus does this nicely for Go. I still haven't delved deep enough to find something similar in the Java space.

agreed that is should be completely configurable. we should also provide some meaningful defaults for this. relying on nginx logs is often not sufficient since it does not provide headers and can also not be aggregated on routes vs paths. route aggregation is key. with route aggregation, specific headers, and json, you can do a ton of analysis on traffic patterns with kibana.