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 2022/03/02 08:19:26 UTC
[GitHub] [apisix] shuaijinchao opened a new pull request #6487: test(http-logger): fetch request and response body test case
shuaijinchao opened a new pull request #6487:
URL: https://github.com/apache/apisix/pull/6487
### What this PR does / why we need it:
FIX https://the-asf.slack.com/archives/CUC5MN17A/p1646136677114399
### 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?
* [ ] Have you modified the corresponding document?
* [ ] 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] starsz commented on a change in pull request #6487: test(http-logger): fetch request and response body test case
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #6487:
URL: https://github.com/apache/apisix/pull/6487#discussion_r819200649
##########
File path: t/plugin/http-logger2.t
##########
@@ -32,6 +32,64 @@ add_block_preprocessor(sub {
$block->set_value("no_error_log", "[error]");
}
+ my $http_config = $block->http_config // <<_EOC_;
+ server {
+ listen 12001;
+
+ location /http-logger/test {
+ content_by_lua_block {
+ ngx.say("test-http-logger-response")
+ }
+ }
+
+ location /http-logger/center {
+ content_by_lua_block {
+ local function str_split(str, reps)
+ local str_list = {}
+ string.gsub(str, '[^' .. reps .. ']+', function(w)
+ table.insert(str_list, w)
+ end)
+ return str_list
+ end
+
+ local args = ngx.req.get_uri_args()
+ local query = args.query or nil
+ ngx.req.read_body()
+ local body = ngx.req.get_body_data()
+
+ if query then
+ if type(query) == "string" then
+ query = {query}
+ end
+
+ local data, err = require("cjson").decode(body)
Review comment:
Got it.
--
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 #6487: test(http-logger): fetch request and response body test case
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6487:
URL: https://github.com/apache/apisix/pull/6487#discussion_r817541767
##########
File path: t/lib/server.lua
##########
@@ -46,6 +46,14 @@ local function inject_headers()
end
end
+local function str_split(str, reps)
Review comment:
Let's add the mock API in the test file like:
https://github.com/apache/apisix/blob/4feab8e6bf1cf8ffd16440848b693ce14a792fbc/t/plugin/clickhouse-logger.t#L39
It's not commonly used.
--
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] shuaijinchao commented on a change in pull request #6487: test(http-logger): fetch request and response body test case
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6487:
URL: https://github.com/apache/apisix/pull/6487#discussion_r817593530
##########
File path: t/lib/server.lua
##########
@@ -46,6 +46,14 @@ local function inject_headers()
end
end
+local function str_split(str, reps)
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 #6487: test(http-logger): fetch request and response body test case
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6487:
URL: https://github.com/apache/apisix/pull/6487#discussion_r818233930
##########
File path: t/plugin/http-logger2.t
##########
@@ -32,6 +32,64 @@ add_block_preprocessor(sub {
$block->set_value("no_error_log", "[error]");
}
+ my $http_config = $block->http_config // <<_EOC_;
+ server {
+ listen 12001;
+
+ location /http-logger/test {
+ content_by_lua_block {
+ ngx.say("test-http-logger-response")
+ }
+ }
+
+ location /http-logger/center {
+ content_by_lua_block {
+ local function str_split(str, reps)
+ local str_list = {}
+ string.gsub(str, '[^' .. reps .. ']+', function(w)
+ table.insert(str_list, w)
+ end)
+ return str_list
+ end
+
+ local args = ngx.req.get_uri_args()
+ local query = args.query or nil
+ ngx.req.read_body()
+ local body = ngx.req.get_body_data()
+
+ if query then
+ if type(query) == "string" then
+ query = {query}
+ end
+
+ local data, err = require("cjson").decode(body)
+ if err then
+ ngx.log(ngx.ERR, "logs:", body)
+ end
+
+ for i = 1, #query do
+ local fields = str_split(query[i], ".")
+ local val
+ for j = 1, #fields do
+ local key = fields[j]
+ if j == 1 then
+ val = data[key]
+ else
+ val = val[key]
+ end
+ end
+ ngx.log(ngx.ERR ,query[i], ":", val)
Review comment:
Better to use WARN log level
##########
File path: t/plugin/http-logger2.t
##########
@@ -141,3 +199,86 @@ GET /opentracing
--- no_error_log
removing batch processor stale object
--- wait: 1.5
+
+
+
+=== TEST 6: set fetch request body and response body route
+--- 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,
+ [[{
+ "methods": ["POST"],
+ "plugins": {
+ "http-logger": {
+ "uri": "http://127.0.0.1:12001/http-logger/center?query[]=request.body&query[]=response.body",
+ "batch_max_size": 1,
+ "max_retry_count": 1,
+ "retry_delay": 2,
+ "buffer_duration": 2,
+ "inactive_timeout": 2,
+ "include_req_body": true,
+ "include_resp_body": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:12001": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/http-logger/test"
+ }]],
+ [[{
Review comment:
We should remove the response body check
--
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] starsz commented on a change in pull request #6487: test(http-logger): fetch request and response body test case
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #6487:
URL: https://github.com/apache/apisix/pull/6487#discussion_r817697617
##########
File path: t/plugin/http-logger2.t
##########
@@ -32,6 +32,64 @@ add_block_preprocessor(sub {
$block->set_value("no_error_log", "[error]");
}
+ my $http_config = $block->http_config // <<_EOC_;
+ server {
+ listen 12001;
+
+ location /http-logger/test {
+ content_by_lua_block {
+ ngx.say("test-http-logger-response")
+ }
+ }
+
+ location /http-logger/center {
+ content_by_lua_block {
+ local function str_split(str, reps)
+ local str_list = {}
+ string.gsub(str, '[^' .. reps .. ']+', function(w)
+ table.insert(str_list, w)
+ end)
+ return str_list
+ end
+
+ local args = ngx.req.get_uri_args()
+ local query = args.query or nil
+ ngx.req.read_body()
+ local body = ngx.req.get_body_data()
+
+ if query then
+ if type(query) == "string" then
+ query = {query}
+ end
+
+ local data, err = require("cjson").decode(body)
Review comment:
Hi, Why do we need to decode the body?
--
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] shuaijinchao commented on a change in pull request #6487: test(http-logger): fetch request and response body test case
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6487:
URL: https://github.com/apache/apisix/pull/6487#discussion_r818232007
##########
File path: t/plugin/http-logger2.t
##########
@@ -32,6 +32,64 @@ add_block_preprocessor(sub {
$block->set_value("no_error_log", "[error]");
}
+ my $http_config = $block->http_config // <<_EOC_;
+ server {
+ listen 12001;
+
+ location /http-logger/test {
+ content_by_lua_block {
+ ngx.say("test-http-logger-response")
+ }
+ }
+
+ location /http-logger/center {
+ content_by_lua_block {
+ local function str_split(str, reps)
+ local str_list = {}
+ string.gsub(str, '[^' .. reps .. ']+', function(w)
+ table.insert(str_list, w)
+ end)
+ return str_list
+ end
+
+ local args = ngx.req.get_uri_args()
+ local query = args.query or nil
+ ngx.req.read_body()
+ local body = ngx.req.get_body_data()
+
+ if query then
+ if type(query) == "string" then
+ query = {query}
+ end
+
+ local data, err = require("cjson").decode(body)
Review comment:
The log format data is JSON, and we need to get the information in the structure.
--
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 #6487: test(http-logger): fetch request and response body test case
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #6487:
URL: https://github.com/apache/apisix/pull/6487
--
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