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/10/30 13:17:42 UTC
[GitHub] [apisix] rustyb opened a new pull request, #8205: Patch 2
rustyb opened a new pull request, #8205:
URL: https://github.com/apache/apisix/pull/8205
### Description
Small PR to optionally add the request body to the `google-cloud-logging` plugin as requested in https://github.com/apache/apisix/issues/8101.
Setting `include_req_body` to `true` in the plugin config will add the request payload to the jsonPayload part of the cloud logging message.
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
Fixes #8101
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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] rustyb commented on a diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
rustyb commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1016712762
##########
t/plugin/google-cloud-logging.t:
##########
@@ -31,6 +31,61 @@ add_block_preprocessor(sub {
if (!defined $block->request) {
$block->set_value("request", "GET /t");
}
+ my $http_config = $block->http_config // <<_EOC_;
+server {
+ listen 12001;
+
+ location /google-cloud-logging/test {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local data = ngx.req.get_body_data()
+ ngx.log(ngx.WARN, "google cloud logging request body: ", data)
+ ngx.log(ngx.ERR, data)
+ ngx.say('test-http-logger-response')
+ ngx.log(ngx.WARN, "loggingBody:", data)
+ }
+ }
+
+
+ location /google/logging/entries {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local body = ngx.req.get_body_data()
+
+ local data, err = require("cjson").decode(body)
+ if err then
+ ngx.log(ngx.WARN, "loggingBody", body)
+ end
+
+ ngx.log(ngx.WARN, "loggingBody", body)
+ ngx.say("ok")
+ ngx.print(body)
+ }
+ }
+
+ location /google/logging/test1 {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local json_decode = require("toolkit.json").decode
+ local json_encode = require("toolkit.json").encode
+ local data = ngx.req.get_body_data()
+ local decoded_data = json_decode(data)
+
+ ngx.log(ngx.WARN,"gcp logs body entries: ", json_encode(decoded_data["entries"][1]["jsonPayload"]))
+ ngx.say("ok")
+ }
+ }
Review Comment:
Yup that would do the job indeed. Updated in https://github.com/apache/apisix/pull/8205/commits/06bd36aa20198eae5bc140fa4bffb0c878797a03
--
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] github-actions[bot] commented on pull request #8205: feat: add request body to google-cloud-logging
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1488306600
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.
--
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 diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1055118788
##########
apisix/plugins/google-cloud-logging.lua:
##########
@@ -178,6 +180,7 @@ local function get_logger_entry(conf, ctx, oauth)
jsonPayload = {
route_id = entry.route_id,
service_id = entry.service_id,
+ request_body = core.json.decode(entry.request.body),
Review Comment:
We don't need to decode request body which is already a single 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] rustyb commented on pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
rustyb commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1302796035
@tzssangglass I think i understood what you mean unfortunately I just cannot get the right format on the regex in test 29 - can you help me there?
--
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] tokers commented on a diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1015205569
##########
t/plugin/google-cloud-logging.t:
##########
@@ -793,9 +850,90 @@ passed
-=== TEST 27: test route (set include_req_body = true on route succeeds)
+=== TEST 28: set fetch request body and response body route
+--- config
+ location /t {
+ content_by_lua_block {
+ local config = {
+ uri = "/google-cloud-logging/test",
+ method = 'POST',
+ upstream = {
+ type = "roundrobin",
+ nodes = {
+ ["127.0.0.1:12001"] = 1
+ }
+ },
+ plugins = {
+ ["google-cloud-logging"] = {
+ auth_file = "t/plugin/google-cloud-logging/config.json",
+ inactive_timeout = 1,
+ batch_max_size = 1,
+ include_req_body = true,
+ }
+ }
+ }
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, config)
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 28: set fetch request body and response body route
Review Comment:
The sequence number is wrong? It should be `29`.
--
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] rustyb commented on a diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
rustyb commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1016263059
##########
t/plugin/google-cloud-logging.t:
##########
@@ -793,9 +850,90 @@ passed
-=== TEST 27: test route (set include_req_body = true on route succeeds)
+=== TEST 28: set fetch request body and response body route
+--- config
+ location /t {
+ content_by_lua_block {
+ local config = {
+ uri = "/google-cloud-logging/test",
+ method = 'POST',
+ upstream = {
+ type = "roundrobin",
+ nodes = {
+ ["127.0.0.1:12001"] = 1
+ }
+ },
+ plugins = {
+ ["google-cloud-logging"] = {
+ auth_file = "t/plugin/google-cloud-logging/config.json",
+ inactive_timeout = 1,
+ batch_max_size = 1,
+ include_req_body = true,
+ }
+ }
+ }
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, config)
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 28: set fetch request body and response body route
Review Comment:
@tokers I removed the test 26 & 27 I added in an initial commit while trying to get tests working and the test 28 & 29 referenced above become 26 & 27.
--
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 diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1009997161
##########
t/plugin/google-cloud-logging.t:
##########
@@ -745,7 +746,54 @@ passed
-=== TEST 25: test route(https file configuration SSL authentication succeed: ssl_verify = false)
+=== TEST 25: test route (https file configuration SSL authentication succeed: ssl_verify = false)
+--- request
+GET /hello
+--- wait: 2
+--- response_body
+hello world
+
+=== TEST 26: set include_req_body = true on route succeeds
Review Comment:
3 blank lines are required between test cases
--
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] rustyb commented on pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
rustyb commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1297302469
@tzssangglass I think i've got the correct test added here now however i'm having trouble to get the tests working locally. Is it possible to get the CI enabled to verify the tests are correct?
--
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 diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1055128624
##########
t/plugin/google-cloud-logging.t:
##########
@@ -31,6 +31,43 @@ add_block_preprocessor(sub {
if (!defined $block->request) {
$block->set_value("request", "GET /t");
}
+ my $http_config = $block->http_config // <<_EOC_;
+server {
+ listen 12001;
+
+ location /google-cloud-logging/test {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local data = ngx.req.get_body_data()
+ ngx.log(ngx.WARN, "google cloud logging request body: ", data)
+ ngx.log(ngx.ERR, data)
+ ngx.say('test-http-logger-response')
+ ngx.log(ngx.WARN, "loggingBody:", data)
Review Comment:
The logging of the data is a bit confusing. The same data here is logged by three times.
##########
t/plugin/google-cloud-logging.t:
##########
@@ -745,9 +783,58 @@ passed
-=== TEST 25: test route(https file configuration SSL authentication succeed: ssl_verify = false)
+=== TEST 25: test route (https file configuration SSL authentication succeed: ssl_verify = false)
--- request
GET /hello
--- wait: 2
--- response_body
hello world
+
+
+
+=== TEST 26: set fetch request body true in configuration
+--- config
+ location /t {
+ content_by_lua_block {
+ local config = {
+ uri = "/google-cloud-logging/test",
+ method = 'POST',
+ upstream = {
+ type = "roundrobin",
+ nodes = {
+ ["127.0.0.1:12001"] = 1
+ }
+ },
+ plugins = {
+ ["google-cloud-logging"] = {
+ auth_file = "t/plugin/google-cloud-logging/config.json",
+ inactive_timeout = 1,
+ batch_max_size = 1,
+ include_req_body = true,
+ }
+ }
+ }
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, config)
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 27: check request body included in log entry
+--- request
+POST /google-cloud-logging/test
+{"bodyItem": "something"}
+--- error_log eval
+qr/gcp logs body entries/
Review Comment:
Could we also check if the logged body is exactly the same as `{"bodyItem": "something"}`?
--
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 diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1016634339
##########
t/plugin/google-cloud-logging/config.json:
##########
@@ -5,5 +5,5 @@
"scopes": [
"https://apisix.apache.org/logs:admin"
],
- "entries_uri": "http://127.0.0.1:1980/google/logging/entries"
+ "entries_uri": "http://127.0.0.1:12001/google/logging/test1"
Review Comment:
```suggestion
"entries_uri": "http://127.0.0.1:12001/google/logging/entries"
```
##########
t/plugin/google-cloud-logging.t:
##########
@@ -31,6 +31,61 @@ add_block_preprocessor(sub {
if (!defined $block->request) {
$block->set_value("request", "GET /t");
}
+ my $http_config = $block->http_config // <<_EOC_;
+server {
+ listen 12001;
+
+ location /google-cloud-logging/test {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local data = ngx.req.get_body_data()
+ ngx.log(ngx.WARN, "google cloud logging request body: ", data)
+ ngx.log(ngx.ERR, data)
+ ngx.say('test-http-logger-response')
+ ngx.log(ngx.WARN, "loggingBody:", data)
+ }
+ }
+
+
+ location /google/logging/entries {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local body = ngx.req.get_body_data()
+
+ local data, err = require("cjson").decode(body)
+ if err then
+ ngx.log(ngx.WARN, "loggingBody", body)
+ end
+
+ ngx.log(ngx.WARN, "loggingBody", body)
+ ngx.say("ok")
+ ngx.print(body)
+ }
+ }
+
+ location /google/logging/test1 {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local json_decode = require("toolkit.json").decode
+ local json_encode = require("toolkit.json").encode
+ local data = ngx.req.get_body_data()
+ local decoded_data = json_decode(data)
+
+ ngx.log(ngx.WARN,"gcp logs body entries: ", json_encode(decoded_data["entries"][1]["jsonPayload"]))
+ ngx.say("ok")
+ }
+ }
Review Comment:
```suggestion
location /google-cloud-logging/test {
content_by_lua_block {
ngx.req.read_body()
local data = ngx.req.get_body_data()
ngx.log(ngx.WARN, "google cloud logging request body: ", data)
ngx.log(ngx.ERR, data)
ngx.say('test-http-logger-response')
ngx.log(ngx.WARN, "loggingBody:", data)
}
}
location /google/logging/entries {
content_by_lua_block {
ngx.req.read_body()
local json_decode = require("toolkit.json").decode
local json_encode = require("toolkit.json").encode
local data = ngx.req.get_body_data()
local decoded_data = json_decode(data)
ngx.log(ngx.WARN,"gcp logs body entries: ", json_encode(decoded_data["entries"][1]["jsonPayload"]))
ngx.say("ok")
}
}
```
would this work around with ` "entries_uri": "http://127.0.0.1:12001/google/logging/entries"` in `config.json`
--
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 diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1016639185
##########
t/plugin/google-cloud-logging.t:
##########
@@ -31,6 +31,61 @@ add_block_preprocessor(sub {
if (!defined $block->request) {
$block->set_value("request", "GET /t");
}
+ my $http_config = $block->http_config // <<_EOC_;
+server {
+ listen 12001;
+
+ location /google-cloud-logging/test {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local data = ngx.req.get_body_data()
+ ngx.log(ngx.WARN, "google cloud logging request body: ", data)
+ ngx.log(ngx.ERR, data)
+ ngx.say('test-http-logger-response')
+ ngx.log(ngx.WARN, "loggingBody:", data)
+ }
+ }
+
+
+ location /google/logging/entries {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local body = ngx.req.get_body_data()
+
+ local data, err = require("cjson").decode(body)
+ if err then
+ ngx.log(ngx.WARN, "loggingBody", body)
+ end
+
+ ngx.log(ngx.WARN, "loggingBody", body)
+ ngx.say("ok")
+ ngx.print(body)
+ }
+ }
+
+ location /google/logging/test1 {
+ content_by_lua_block {
+ ngx.req.read_body()
+ local json_decode = require("toolkit.json").decode
+ local json_encode = require("toolkit.json").encode
+ local data = ngx.req.get_body_data()
+ local decoded_data = json_decode(data)
+
+ ngx.log(ngx.WARN,"gcp logs body entries: ", json_encode(decoded_data["entries"][1]["jsonPayload"]))
+ ngx.say("ok")
+ }
+ }
Review Comment:
Because it seems that only `/google/logging/entries` and `/google-cloud-logging/test` are used. `/google/logging/test1` is not necessary.
--
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] rustyb commented on pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
rustyb commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1297133966
@tzssangglass do you have a guide on how you add new test cases?
--
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 pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1297903541
> I think i've got the correct test added here now however i'm having trouble to get the tests working locally. Is it possible to get the CI enabled to verify the tests are correct?
When you fix the comments, I will perform CI manually first (github restricts the user who submits for the first time to be confirmed by the maintainer before performing CI)
Your test case does not cover the function you added, you can refer to: https://github.com/apache/apisix/blob/024e0ef70b5e403e11eb7a2f4abb59675a657081/t/plugin/http-logger2.t#L212-L262
condition: `"include_req_body": true` -->
verify:
`--- error_log
request.body:test-http-logger-request`
--
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 diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1014947771
##########
t/plugin/google-cloud-logging.t:
##########
@@ -745,9 +801,137 @@ passed
-=== TEST 25: test route(https file configuration SSL authentication succeed: ssl_verify = false)
+=== TEST 25: test route (https file configuration SSL authentication succeed: ssl_verify = false)
--- request
GET /hello
--- wait: 2
--- response_body
hello world
+
+
+
+=== TEST 26: set include_req_body = true on route succeeds
+--- config
+ location /t {
+ content_by_lua_block {
+
+ local config = {
+ uri = "/hello",
+ upstream = {
+ type = "roundrobin",
+ nodes = {
+ ["127.0.0.1:1980"] = 1
+ }
+ },
+ plugins = {
+ ["google-cloud-logging"] = {
+ auth_file = "t/plugin/google-cloud-logging/config-https-ip.json",
+ inactive_timeout = 1,
+ batch_max_size = 1,
+ ssl_verify = false,
+ include_req_body = true,
+ }
+ }
+ }
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, config)
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 27: set fetch request body and response body route
+--- config
+ location /t {
+ content_by_lua_block {
+ local config = {
+ uri = "/google-cloud-logging/test",
+ method = 'POST',
+ upstream = {
+ type = "roundrobin",
+ nodes = {
+ ["127.0.0.1:12001"] = 1
+ }
+ },
+ plugins = {
+ ["google-cloud-logging"] = {
+ auth_file = "t/plugin/google-cloud-logging/config.json",
+ inactive_timeout = 1,
+ batch_max_size = 1,
+ include_req_body = true,
+ }
+ }
+ }
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, config)
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
Review Comment:
It appears that TEST 28 and 29 are duplicates of TEST 26 and 27. As long as TEST 28 and 29 run successfully, it can be verified, right?
--
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 diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1014947771
##########
t/plugin/google-cloud-logging.t:
##########
@@ -745,9 +801,137 @@ passed
-=== TEST 25: test route(https file configuration SSL authentication succeed: ssl_verify = false)
+=== TEST 25: test route (https file configuration SSL authentication succeed: ssl_verify = false)
--- request
GET /hello
--- wait: 2
--- response_body
hello world
+
+
+
+=== TEST 26: set include_req_body = true on route succeeds
+--- config
+ location /t {
+ content_by_lua_block {
+
+ local config = {
+ uri = "/hello",
+ upstream = {
+ type = "roundrobin",
+ nodes = {
+ ["127.0.0.1:1980"] = 1
+ }
+ },
+ plugins = {
+ ["google-cloud-logging"] = {
+ auth_file = "t/plugin/google-cloud-logging/config-https-ip.json",
+ inactive_timeout = 1,
+ batch_max_size = 1,
+ ssl_verify = false,
+ include_req_body = true,
+ }
+ }
+ }
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, config)
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 27: set fetch request body and response body route
+--- config
+ location /t {
+ content_by_lua_block {
+ local config = {
+ uri = "/google-cloud-logging/test",
+ method = 'POST',
+ upstream = {
+ type = "roundrobin",
+ nodes = {
+ ["127.0.0.1:12001"] = 1
+ }
+ },
+ plugins = {
+ ["google-cloud-logging"] = {
+ auth_file = "t/plugin/google-cloud-logging/config.json",
+ inactive_timeout = 1,
+ batch_max_size = 1,
+ include_req_body = true,
+ }
+ }
+ }
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, config)
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
Review Comment:
It appears that TEST 28 and 29 is a duplicate of TEST 26 and 27. As long as TEST 28 and 29 run successfully, it can be verified, right?
--
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 pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1302858203
> @tzssangglass I think i understood what you mean unfortunately I just cannot get the right format on the regex in test 29 - can you help me there?
we can try
```
--- error_log eval
qr/gcp logs body entries/
```
--
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 pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1296874039
hi @rustyb , we need to add test cases to cover this function.
--
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 diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1009997401
##########
t/plugin/google-cloud-logging.t:
##########
@@ -745,7 +746,54 @@ passed
-=== TEST 25: test route(https file configuration SSL authentication succeed: ssl_verify = false)
+=== TEST 25: test route (https file configuration SSL authentication succeed: ssl_verify = false)
+--- request
+GET /hello
+--- wait: 2
+--- response_body
+hello world
+
+=== TEST 26: set include_req_body = true on route succeeds
+--- config
+ location /t {
+ content_by_lua_block {
+
Review Comment:
code style: delete this blank line
--
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 pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1361001216
> @tzssangglass is this PR ready for merge?
Approval. CI errors should not be related to this PR.
--
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] github-actions[bot] closed pull request #8205: feat: add request body to google-cloud-logging
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #8205: feat: add request body to google-cloud-logging
URL: https://github.com/apache/apisix/pull/8205
--
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] github-actions[bot] commented on pull request #8205: feat: add request body to google-cloud-logging
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1528742383
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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] Arztz commented on pull request #8205: feat: add request body to google-cloud-logging
Posted by "Arztz (via GitHub)" <gi...@apache.org>.
Arztz commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1564182827
Is this one still working?
I still find the way to use this one
--
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] moonming commented on pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
moonming commented on PR #8205:
URL: https://github.com/apache/apisix/pull/8205#issuecomment-1360672313
@tzssangglass is this PR ready for merge?
--
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] rustyb commented on a diff in pull request #8205: feat: add request body to google-cloud-logging
Posted by GitBox <gi...@apache.org>.
rustyb commented on code in PR #8205:
URL: https://github.com/apache/apisix/pull/8205#discussion_r1015136784
##########
t/plugin/google-cloud-logging.t:
##########
@@ -745,9 +801,137 @@ passed
-=== TEST 25: test route(https file configuration SSL authentication succeed: ssl_verify = false)
+=== TEST 25: test route (https file configuration SSL authentication succeed: ssl_verify = false)
--- request
GET /hello
--- wait: 2
--- response_body
hello world
+
+
+
+=== TEST 26: set include_req_body = true on route succeeds
+--- config
+ location /t {
+ content_by_lua_block {
+
+ local config = {
+ uri = "/hello",
+ upstream = {
+ type = "roundrobin",
+ nodes = {
+ ["127.0.0.1:1980"] = 1
+ }
+ },
+ plugins = {
+ ["google-cloud-logging"] = {
+ auth_file = "t/plugin/google-cloud-logging/config-https-ip.json",
+ inactive_timeout = 1,
+ batch_max_size = 1,
+ ssl_verify = false,
+ include_req_body = true,
+ }
+ }
+ }
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, config)
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 27: set fetch request body and response body route
+--- config
+ location /t {
+ content_by_lua_block {
+ local config = {
+ uri = "/google-cloud-logging/test",
+ method = 'POST',
+ upstream = {
+ type = "roundrobin",
+ nodes = {
+ ["127.0.0.1:12001"] = 1
+ }
+ },
+ plugins = {
+ ["google-cloud-logging"] = {
+ auth_file = "t/plugin/google-cloud-logging/config.json",
+ inactive_timeout = 1,
+ batch_max_size = 1,
+ include_req_body = true,
+ }
+ }
+ }
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, config)
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
Review Comment:
Yup your right 👍 - i've removed the duplicate tests in https://github.com/apache/apisix/pull/8205/commits/8884e1c49e2ad4a9df77802372601f4ec6389d41
--
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