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

request help: kafka logger supports logging request body #5343

Closed
spacewander opened this issue Oct 27, 2021 · 12 comments · Fixed by #5501
Closed

request help: kafka logger supports logging request body #5343

spacewander opened this issue Oct 27, 2021 · 12 comments · Fixed by #5501
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@spacewander
Copy link
Member

Issue description

Add a request_body switch to the schema, and each body can be used by expr to decide whether to log or not. Without this switch, the body is not logged.

"kafka-logger": {
   "broker_list":{
       "127.0.0.1":9092
    },
   "kafka_topic" : "test2",
   "request_body": {
       "expr": [
          ["request_length", "<", "1024"],
       ]
   },
   "key" : "key1",
   "batch_max_size": 1,
   "name": "kafka logger"
}

expr can be evaluated by lua-resty-expr. request body can be fetched by core.request.get_body.

Environment

  • apisix version (cmd: apisix version): 2.10.1
  • OS (cmd: uname -a):
  • OpenResty / Nginx version (cmd: nginx -V or openresty -V):
  • etcd version, if have (cmd: run curl http://127.0.0.1:9090/v1/server_info to get the info from server-info API):
  • apisix-dashboard version, if have:
  • the plugin runner version, if the issue is about a plugin runner (cmd: depended on the kind of runner):
  • luarocks version, if the issue is about installation (cmd: luarocks --version):
@windyrjc
Copy link
Contributor

core.request.get_body and core.response.hold_body_chunk can't �be used in 'log_by_lua' phase,it seems that we can logging log it in other phase or set request_body and response_body in ctx. Does it have a better way?

@tokers
Copy link
Contributor

tokers commented Nov 11, 2021

core.request.get_body and core.response.hold_body_chunk can't �be used in 'log_by_lua' phase,it seems that we can logging log it in other phase or set request_body and response_body in ctx. Does it have a better way?

Do this in an early phase, say, access phase.

@windyrjc
Copy link
Contributor

Other phase like filter_by_lua may rewrite request and response by other plugin. If we get request_body and response_body in filter_by_lua and set it in ctx,it may cause performance issue

@tzssangglass
Copy link
Member

get request body in access_by_lua phase, and get response body in log_by_lua phase?

@spacewander
Copy link
Member Author

it may cause performance issue

Yes. The log of body is expensive. So we should only log it if need.

Other phase like filter_by_lua may rewrite request and response by other plugin.

In practice, it is not a problem. The gzip filter runs after lua filter, so there is no way to log the gzipped response body in the Lua. However, very few people want to log the gzipped body instead of the original one. If people need a 100% real body sent to the client, they should do it in another proxy or a traffic capture sidecar.

@windyrjc
Copy link
Contributor

windyrjc commented Nov 12, 2021

In practice, it is not a problem. The gzip filter runs after lua filter

Other plugin like response_rewrite plugin will also change the response.So do you means I should change all the kafka-logger code to the filter_by_lua phase and set the kafka-logger priority after all the other plugin except gzip ?

@spacewander
Copy link
Member Author

Since the kafka-logger's priority is low enough, we can just keep it untouched.
We are engineers, not mathematicians. We need to know when to trade off.

BTW, the gzip plugin doesn't do the gzip directly, it just controls the behavior of gzip.

@spacewander
Copy link
Member Author

BTW, we are not expected to discuss the logging response body in an issue about the request body. It's off-topic. 😄

@windyrjc
Copy link
Contributor

OK,thanks bro~

@SomalianIvan
Copy link

SomalianIvan commented Nov 13, 2021

@spacewander can I give this a try?

@windyrjc
Copy link
Contributor

windyrjc commented Nov 14, 2021

@spacewander can I give this a try?

I'm already submit a PR😁 #5501

@membphis
Copy link
Member

@spacewander can I give this a try?

you can can try to fix other issues: https://github.com/apache/apisix/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants