Kong / kong

🦍 The Cloud-Native API Gateway and AI Gateway.
https://konghq.com/install/#kong-community
Apache License 2.0
39.22k stars 4.81k forks source link

Log request over http #226

Closed jeevanrd closed 9 years ago

jeevanrd commented 9 years ago

We have a reporting service running and we want to log request over http call to this service. Is it something already available?? Please let me know. Otherwise We would like to send a PR for this change.

thibaultcha commented 9 years ago

Hi,

There is currently no HTTP logging plugin. You can read the TCP/UDP ones to know how they are implemented and how to retrieve the actual data describing a request to send.

subnetmarco commented 9 years ago

@jeevan1986, how would the HTTP request look like? Would it just be a POST to a third-party URL with a json body?

jeevanrd commented 9 years ago

@thefosk yes, it would be a POST to a third-party URL with a json body.

sonicaghi commented 9 years ago

@jeevan1986 you can check https://github.com/Mashape/kong/pull/244

jeevanrd commented 9 years ago

@sinzone Thanks. That's what I am looking for. But also we need the url params in the log message to keep track of some user parameters. So it would be nice if can you can add this. "url_params = ngx.req.get_uri_args()"

Please let us know once it is done.

subnetmarco commented 9 years ago

@jeevan1986 that is already being logged: https://github.com/Mashape/kong/blob/master/kong/kong.lua#L234

thibaultcha commented 9 years ago

uri is actually a plain string of the request URI without the arguments. See: http://wiki.nginx.org/HttpCoreModule#.24uri

request_uri would be a string including the arguments.

But as pointed by @jeevan1986, get_uri_args() returns a table which is more appropriate, and is what the ALF logging plugin is using, for ex.

subnetmarco commented 9 years ago

oh, that's a bug. We definitely want to use request_uri

thibaultcha commented 9 years ago

get_uri_args() would make more sense. That way we have the path, and the querystring as a table.

Or, why not the 3 of them? path, full URI, and the querystring table. The default format is very poor and undocumented, that should change. It should also be properly implemented in a serializer, so we can change it for another serializer easily (ALF...)

subnetmarco commented 9 years ago

For logging tools like Kibana and Splunk having simpler data structures is better than complex log structures like HAR or ALF. It would be a nightmare to write queries against ALF using Splunk.

I agree we need to document the current format too.

I think the requested URL should be logged in plain format anyways, with maybe the addition of parsed arguments on a separate property (what if the request is being made with some values or some chars that are not properly parsed in Lua? Having the raw format would be nice). So the first step is using request_uri instead of uri to fix the bug, and the second step is adding one more property with the parsed arguments if we want to provide that too.

thibaultcha commented 9 years ago

For logging tools like Kibana and Splunk having simpler data structures is better than complex log structures like HAR or ALF. It would be a nightmare to write queries against ALF using Splunk.

We already discussed that and I did not bring this subject into the conversation. Why are you saying this? However, we agreed plugins need to have different serializers. Nothing more, nothing less here.

I would use request_uri and get_uri_args(), as well as uri, eventually.

subnetmarco commented 9 years ago

We already discussed that and I did not bring this subject into the conversation. Why are you saying this?

I was making a point about the current logging format since when we talked about it, it was in a private conversation and that information should be disclosed.

thibaultcha commented 9 years ago

Alright, I still think it can be improved and it needs to be carefully documented tho

jeevanrd commented 9 years ago

@thibaultCha I agree with you, it makes sense to emit all the three (path, full URI, and the querystring table). Then it's only at the consumer end what ever property he want to consume he can use it.

thibaultcha commented 9 years ago

In the meantime, HTTP logging was added by #251 :)

jeevanrd commented 9 years ago

@thibaultCha Any update on emitting the (path, full URI, and the querystring table) in the log message. This is a must required for me to consume the httplog plugin.

thibaultcha commented 9 years ago

Yes, but for that, we should open another issue to discuss improvements on the logging format.