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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to