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 03:47:53 UTC

[GitHub] [apisix] windyrjc opened a new pull request #5501: feat(kafka-logger) kafka logger supports logging request body (#5343)

windyrjc opened a new pull request #5501:
URL: https://github.com/apache/apisix/pull/5501


   ### What this PR does / why we need it:
   kafka logger supports logging request body 
   feat: #5343 
   ### Pre-submission checklist:
   
   * [x] Did you explain what problem does this PR solve? Or what new features have been added?
   * [x] Have you added corresponding test cases?
   * [x] Have you modified the corresponding document?
   * [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
   


-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748847942



##########
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:
       In my opinion, i think `include_req_body` like a main switch, `include_req_body_expr` like a switch in condition. Close  `include_req_body`  will not log any request body data, and `include_req_body`'s semantic is compatible with  the previous version of kafka-logger, we are just only add a condition 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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749027979



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1208,3 +1208,107 @@ hello world
 --- error_log_like eval
 qr/send data to kafka: \{.*"body":"abcdef"/
 --- wait: 2
+
+
+
+=== TEST 28: hit route, not trigger request_body_expr rule
+--- request
+GET /hello

Review comment:
       Ok, i just want to prove that if there was no request body, it still work normally. There is already has a eval false case below and i'm delete this case for good 

##########
File path: t/plugin/kafka-logger.t
##########
@@ -1135,7 +1135,7 @@ qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
                                 "key" : "key1",
                                 "timeout" : 1,
                                 "include_req_body": true,
-                                "request_body_expr": [
+                                "include_req_body_expr": [
                                     [
                                       "remote_addr",

Review comment:
       I'm not understand what this means 




-- 
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



[GitHub] [apisix] spacewander commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748982201



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,201 @@ 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,include_req_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" : "test2",
+                                "key" : "key1",
+                                "timeout" : 1,
+                                "include_req_body": true,
+                                "include_req_body_expr": [
+                                    [
+                                      "remote_addr",
+                                      "==",
+                                      "127.0.0.1"
+                                    ]
+                                ],
+                                "batch_max_size": 1
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                }]],
+                [[{

Review comment:
       Please remove the part which is not relative to the test, from L1156 to L1182




-- 
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



[GitHub] [apisix] spacewander commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r750061433



##########
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},
+        include_req_body_expr = {

Review comment:
       Before we can merge it, could you add a check for the expr in the `check_schema`? Like:
   https://github.com/apache/apisix/blob/9b355c6f13194c00eeb491542ee0727467988877/apisix/plugins/response-rewrite.lua#L111




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r750120071



##########
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},
+        include_req_body_expr = {

Review comment:
       done




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748977696



##########
File path: docs/en/latest/plugins/kafka-logger.md
##########
@@ -58,6 +58,7 @@ For more info on Batch-Processor in Apache APISIX please refer.
 | retry_delay      | integer | optional    | 1              | [0,...] | Number of seconds the process execution should be delayed if the execution fails.        |
 | include_req_body | boolean | optional    | false          | [false, true] | Whether to include the request body. false: indicates that the requested body is not included; true: indicates that the requested body is included. |
 | cluster_name     | integer | optional    | 1              | [0,...] | the name of the cluster. When there are two or more kafka clusters, you can specify different names. And this only works with async producer_type.|
+| request_body_expr  | object  | optional    |                |         | Whether to logging request body,based on [lua-resty-expr](https://github.com/api7/lua-resty-expr).            |

Review comment:
       done

##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,97 @@ 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" : "test2",
+                                "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" : "test2",
+                                    "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

Review comment:
       done

##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,97 @@ 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" : "test2",
+                                "key" : "key1",
+                                "timeout" : 1,
+                                "include_req_body": true,
+                                "request_body_expr": [
+                                    [
+                                      "remote_addr",

Review comment:
       done




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749028012



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1135,7 +1135,7 @@ qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
                                 "key" : "key1",
                                 "timeout" : 1,
                                 "include_req_body": true,
-                                "request_body_expr": [
+                                "include_req_body_expr": [
                                     [
                                       "remote_addr",

Review comment:
       I'm not understand what this means, please be more specific?, thank you.  😁




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748852290



##########
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:
       done 




-- 
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



[GitHub] [apisix] tzssangglass commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749970911



##########
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},
+        include_req_body_expr = {
+            type = "array",
+            items = {
+                type = "array",
+                items = {
+                    type = "string"
+                }
+            }
+        },

Review comment:
       ```suggestion
           include_req_body_expr = {
               type = "array",
               minItems = 1,
               items = {
                   type = "array",
                   items = {
                       type = "string"
                   }
               }
           },
   ```




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749243422



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1135,7 +1135,7 @@ qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
                                 "key" : "key1",
                                 "timeout" : 1,
                                 "include_req_body": true,
-                                "request_body_expr": [
+                                "include_req_body_expr": [
                                     [
                                       "remote_addr",

Review comment:
       I'm using `ctx.var` to eval the expr and `ctx.var` doesn't directly hold args. it only has args table. If you want to achieve this, i need to eval the expr by `ctx.var.args`




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748851922



##########
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:
       done

##########
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:
       done




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749243422



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1135,7 +1135,7 @@ qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
                                 "key" : "key1",
                                 "timeout" : 1,
                                 "include_req_body": true,
-                                "request_body_expr": [
+                                "include_req_body_expr": [
                                     [
                                       "remote_addr",

Review comment:
       I'm using ctx.var to eval the expr and ctx.var doesn't directly hold args. it only has args table. If you want to achieve this, i need to eval the expr by ctx.var.args




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749948007



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,80 @@ 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,include_req_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" : "test2",
+                                "key" : "key1",
+                                "timeout" : 1,
+                                "include_req_body": true,
+                                "include_req_body_expr": [
+                                    [
+                                      "arg_name",
+                                      "==",
+                                      "qwerty"
+                                    ]
+                                ],
+                                "batch_max_size": 1
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                }]]
+                )
+            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, expr eval success

Review comment:
       i forgot lint before push, sorry~




-- 
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



[GitHub] [apisix] tzssangglass commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749939229



##########
File path: docs/zh/latest/plugins/kafka-logger.md
##########
@@ -57,6 +57,7 @@ title: kafka-logger
 | max_retry_count  | integer | 可选   | 0              | [0,...] | 从处理管道中移除之前的最大重试次数。             |
 | retry_delay      | integer | 可选   | 1              | [0,...] | 如果执行失败,则应延迟执行流程的秒数。           |
 | include_req_body | boolean | 可选   | false          | [false, true] | 是否包括请求 body。false: 表示不包含请求的 body ; true: 表示包含请求的 body 。|
+| include_req_body_expr | array  | 可选    |           |         | 是否采集请求body,基于[lua-resty-expr](https://github.com/api7/lua-resty-expr)。 该选项需要开启 `include_req_body`|

Review comment:
       ```suggestion
   | include_req_body_expr | array  | 可选    |           |         | 是否采集请求body,基于 [lua-resty-expr](https://github.com/api7/lua-resty-expr)。 该选项需要开启 `include_req_body`|
   ```

##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,80 @@ 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,include_req_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" : "test2",
+                                "key" : "key1",
+                                "timeout" : 1,
+                                "include_req_body": true,
+                                "include_req_body_expr": [
+                                    [
+                                      "arg_name",
+                                      "==",
+                                      "qwerty"
+                                    ]
+                                ],
+                                "batch_max_size": 1
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                }]]
+                )
+            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, expr eval success
+--- request
+POST /hello?name=qwerty
+abcdef
+--- response_body
+hello world
+--- no_error_log
+[error]
+--- error_log eval
+qr/send data to kafka: \{.*"body":"abcdef"/
+--- wait: 2
+
+
+=== TEST 28: hit route,expr eval fail

Review comment:
       ```suggestion
   --- wait: 2
   
   
   
   === TEST 28: hit route,expr eval fail
   ```

##########
File path: docs/en/latest/plugins/kafka-logger.md
##########
@@ -57,6 +57,7 @@ For more info on Batch-Processor in Apache APISIX please refer.
 | max_retry_count  | integer | optional    | 0              | [0,...] | Maximum number of retries before removing from the processing pipe line.                 |
 | retry_delay      | integer | optional    | 1              | [0,...] | Number of seconds the process execution should be delayed if the execution fails.        |
 | include_req_body | boolean | optional    | false          | [false, true] | Whether to include the request body. false: indicates that the requested body is not included; true: indicates that the requested body is included. |
+| include_req_body_expr  | array  | optional    |          |         | Whether to logging request body,based on [lua-resty-expr](https://github.com/api7/lua-resty-expr), this option require to turn on `include_req_body` option.             |

Review comment:
       ```suggestion
   | include_req_body_expr  | array  | optional    |          |         | Whether to logging request body, based on [lua-resty-expr](https://github.com/api7/lua-resty-expr), this option require to turn on `include_req_body` option.             |
   ```

##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,80 @@ 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,include_req_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" : "test2",
+                                "key" : "key1",
+                                "timeout" : 1,
+                                "include_req_body": true,
+                                "include_req_body_expr": [
+                                    [
+                                      "arg_name",
+                                      "==",
+                                      "qwerty"
+                                    ]
+                                ],
+                                "batch_max_size": 1
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                }]]
+                )
+            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, expr eval success

Review comment:
       ```suggestion
   [error]
   
   
   
   === TEST 27: hit route, expr eval success
   ```




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749136068



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1135,7 +1135,7 @@ qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
                                 "key" : "key1",
                                 "timeout" : 1,
                                 "include_req_body": true,
-                                "request_body_expr": [
+                                "include_req_body_expr": [
                                     [
                                       "remote_addr",

Review comment:
       > Use `arg_blah == 1` in the expression, and hit it with "POST /hello?blah=1" and "POST /hello?blah=2".
   
   I'm using `ctx.var` to eval the expr and  `ctx.var` don't have request param, it only has request urI.If you want to achieve this, i need to eval the expr by `ngx.req.get_uri_args`




-- 
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



[GitHub] [apisix] spacewander commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749839423



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1135,7 +1135,7 @@ qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
                                 "key" : "key1",
                                 "timeout" : 1,
                                 "include_req_body": true,
-                                "request_body_expr": [
+                                "include_req_body_expr": [
                                     [
                                       "remote_addr",

Review comment:
       > I mean to use `$arg_` variable. You can refer to
   > 
   > https://github.com/apache/apisix/blob/cc43b9fc1bf9be14c05d36415f83cdd189d0a7f5/t/plugin/traffic-split4.t#L110
   
   done ,thank you very much




-- 
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



[GitHub] [apisix] spacewander commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r750803972



##########
File path: apisix/plugins/kafka-logger.lua
##########
@@ -107,6 +109,14 @@ local _M = {
 
 
 function _M.check_schema(conf, schema_type)
+
+    if conf.vars then

Review comment:
       Should be `include_req_body_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



[GitHub] [apisix] spacewander commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749085852



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1135,7 +1135,7 @@ qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
                                 "key" : "key1",
                                 "timeout" : 1,
                                 "include_req_body": true,
-                                "request_body_expr": [
+                                "include_req_body_expr": [
                                     [
                                       "remote_addr",

Review comment:
       Use `arg_blah == 1` in the expression, and hit it with "POST /hello?blah=1" and "POST /hello?blah=2".




-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748851957



##########
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:
       done




-- 
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



[GitHub] [apisix] spacewander commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748954119



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,97 @@ 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" : "test2",
+                                "key" : "key1",
+                                "timeout" : 1,
+                                "include_req_body": true,
+                                "request_body_expr": [
+                                    [
+                                      "remote_addr",

Review comment:
       Better to use a test case that can eval to false.

##########
File path: docs/en/latest/plugins/kafka-logger.md
##########
@@ -58,6 +58,7 @@ For more info on Batch-Processor in Apache APISIX please refer.
 | retry_delay      | integer | optional    | 1              | [0,...] | Number of seconds the process execution should be delayed if the execution fails.        |
 | include_req_body | boolean | optional    | false          | [false, true] | Whether to include the request body. false: indicates that the requested body is not included; true: indicates that the requested body is included. |
 | cluster_name     | integer | optional    | 1              | [0,...] | the name of the cluster. When there are two or more kafka clusters, you can specify different names. And this only works with async producer_type.|
+| request_body_expr  | object  | optional    |                |         | Whether to logging request body,based on [lua-resty-expr](https://github.com/api7/lua-resty-expr).            |

Review comment:
       Better to move it under include_req_body and explain its relation to include_req_body.

##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,97 @@ 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" : "test2",
+                                "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" : "test2",
+                                    "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

Review comment:
       Let's add a test that expr is evaluated to false




-- 
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



[GitHub] [apisix] spacewander commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748978680



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1135,7 +1135,7 @@ qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
                                 "key" : "key1",
                                 "timeout" : 1,
                                 "include_req_body": true,
-                                "request_body_expr": [
+                                "include_req_body_expr": [
                                     [
                                       "remote_addr",

Review comment:
       We can do it with `arg_xx`. There is no need to use a separate rule.

##########
File path: t/plugin/kafka-logger.t
##########
@@ -1208,3 +1208,107 @@ hello world
 --- error_log_like eval
 qr/send data to kafka: \{.*"body":"abcdef"/
 --- wait: 2
+
+
+
+=== TEST 28: hit route, not trigger request_body_expr rule
+--- request
+GET /hello

Review comment:
       The test will pass whether the request_body_expr is correct or not




-- 
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



[GitHub] [apisix] spacewander merged pull request #5501: feat(kafka-logger): supports logging request body

Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5501:
URL: https://github.com/apache/apisix/pull/5501


   


-- 
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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r748847942



##########
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:
       In my opinion, i think `include_req_body` is like a main switch and `include_req_body_expr` like a switch in condition. Close  `include_req_body`  will not log any request body data, and `include_req_body`'s semantic is compatible with  the previous version of kafka-logger, we just only add a condition 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



[GitHub] [apisix] windyrjc commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
windyrjc commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749848750



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,201 @@ 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,include_req_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" : "test2",
+                                "key" : "key1",
+                                "timeout" : 1,
+                                "include_req_body": true,
+                                "include_req_body_expr": [
+                                    [
+                                      "remote_addr",
+                                      "==",
+                                      "127.0.0.1"
+                                    ]
+                                ],
+                                "batch_max_size": 1
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                }]],
+                [[{

Review comment:
       done




-- 
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



[GitHub] [apisix] tzssangglass commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749971103



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,82 @@ 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,include_req_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,
+                 [[{

Review comment:
       ```suggestion
                    [=[{
   ```

##########
File path: t/plugin/kafka-logger.t
##########
@@ -1114,3 +1114,82 @@ 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,include_req_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" : "test2",
+                                "key" : "key1",
+                                "timeout" : 1,
+                                "include_req_body": true,
+                                "include_req_body_expr": [
+                                    [
+                                      "arg_name",
+                                      "==",
+                                      "qwerty"
+                                    ]
+                                ],
+                                "batch_max_size": 1
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                }]]

Review comment:
       ```suggestion
                   }]=]
   ```




-- 
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



[GitHub] [apisix] spacewander commented on a change in pull request #5501: feat: kafka logger supports logging request body (#5343)

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5501:
URL: https://github.com/apache/apisix/pull/5501#discussion_r749821366



##########
File path: t/plugin/kafka-logger.t
##########
@@ -1135,7 +1135,7 @@ qr/failed to send data to Kafka topic: .*, brokers: \{"127.0.0.127":9092}/
                                 "key" : "key1",
                                 "timeout" : 1,
                                 "include_req_body": true,
-                                "request_body_expr": [
+                                "include_req_body_expr": [
                                     [
                                       "remote_addr",

Review comment:
       I mean to use `$arg_` variable. You can refer to https://github.com/apache/apisix/blob/cc43b9fc1bf9be14c05d36415f83cdd189d0a7f5/t/plugin/traffic-split4.t#L110




-- 
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