You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/11/14 12:20:29 UTC
[GitHub] [apisix] spacewander commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)
spacewander commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748845464
##########
File path: apisix/utils/log-util.lua
##########
@@ -119,13 +120,32 @@ local function get_full_log(ngx, conf)
}
if conf.include_req_body then
Review comment:
I think we can change the current logic to
```
if include_req_body || eval(include_req_body_expr) {
log req body
}
```
What is your opinion?
##########
File path: apisix/utils/log-util.lua
##########
@@ -119,13 +120,32 @@ local function get_full_log(ngx, conf)
}
if conf.include_req_body then
- local body = req_get_body_data()
- if body then
- log.request.body = body
- else
- local body_file = ngx.req.get_body_file()
- if body_file then
- log.request.body_file = body_file
+
+ local log_request_body = true
+
+ if conf.request_body_expr then
+ local request_expr, err = expr.new(conf.request_body_expr)
Review comment:
We can check `conf.request_expr` before new?
##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,98 @@ GET /t
--- error_log_like eval
qr/create new kafka producer instance, brokers: \[\{"port":9092,"host":"127.0.0.127"}]/
qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
+
+
+
+=== TEST 26: set route(id: 1,include_req_body = true,request_body_expr = array)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "kafka-logger": {
+ "broker_list" :
+ {
+ "127.0.0.1":9092
+ },
+ "kafka_topic" : "test1",
+ "key" : "key1",
+ "timeout" : 1,
+ "include_req_body": true,
+ "request_body_expr": [
+ [
+ "remote_addr",
+ "==",
+ "127.0.0.1"
+ ]
+ ],
+ "batch_max_size": 1
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "kafka-logger": {
+ "broker_list" :
+ {
+ "127.0.0.1":9092
+ },
+ "kafka_topic" : "test1",
+ "key" : "key1",
+ "timeout" : 1,
+ "batch_max_size": 1
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 27: hit route, report log to kafka
+--- request
+POST /hello
+abcdef
+--- response_body
+hello world
+--- no_error_log
+[error]
+--- error_log_like eval
+qr/send data to kafka: \{.*"body":"abcdef"/
+--- wait: 2
+--- wait: 2
Review comment:
Duplicate `--- wait`?
##########
File path: apisix/plugins/kafka-logger.lua
##########
@@ -74,6 +74,15 @@ local schema = {
inactive_timeout = {type = "integer", minimum = 1, default = 5},
batch_max_size = {type = "integer", minimum = 1, default = 1000},
include_req_body = {type = "boolean", default = false},
+ request_body_expr = {
Review comment:
Would be better to name it as `include_req_body_expr`?
##########
File path: apisix/utils/log-util.lua
##########
@@ -119,13 +120,32 @@ local function get_full_log(ngx, conf)
}
if conf.include_req_body then
- local body = req_get_body_data()
- if body then
- log.request.body = body
- else
- local body_file = ngx.req.get_body_file()
- if body_file then
- log.request.body_file = body_file
+
+ local log_request_body = true
+
+ if conf.request_body_expr then
+ local request_expr, err = expr.new(conf.request_body_expr)
+ if not request_expr then
+ core.log.error('generate log expr err ' .. err)
Review comment:
Need to skip the remaining logic. We should not use a nil request_expr.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org