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

Log request over http #226

Closed
jeevanrd opened this issue May 12, 2015 · 17 comments
Closed

Log request over http #226

jeevanrd opened this issue May 12, 2015 · 17 comments
Assignees
Milestone

Comments

@jeevanrd
Copy link

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
Copy link
Member

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
Copy link
Member

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

@jeevanrd
Copy link
Author

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

@subnetmarco subnetmarco added this to the 0.3.0 milestone May 14, 2015
@sonicaghi
Copy link
Member

@jeevan1986 you can check #244

@jeevanrd
Copy link
Author

@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
Copy link
Member

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

@thibaultcha
Copy link
Member

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
Copy link
Member

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

@thibaultcha
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

@jeevanrd
Copy link
Author

@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
Copy link
Member

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

@jeevanrd
Copy link
Author

@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
Copy link
Member

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

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

No branches or pull requests

5 participants