feat: Added log format support in syslog plugin.#8279
Conversation
| [error] | ||
|
|
||
|
|
||
| === TEST 5: add plugin metadata |
There was a problem hiding this comment.
Could you check the logged data format?
You can refer to https://github.com/apache/apisix/pull/7328/files#diff-1108dce1e7823ca0b746c4ba69e692e4b02c6d49117d70859954b56cbb810c29 as an example.
BTW, new test should be added at the end.
There was a problem hiding this comment.
I fixed test reordering problem.
Should I add "access request" test?
And is that enough if is require?
=== TEST 10: access for log format
--- request
GET /hello
--- response_body
hello world
There was a problem hiding this comment.
Let's do it. And we need to inject some code in extra_init_by_lua to check the logged data.
There was a problem hiding this comment.
I think we don't need extra_init_by_lua for these tests.
I added new test cases. Please review it again.
BTW: It was to hard to understand test framework for me. :)
There was a problem hiding this comment.
BTW: It was to hard to understand test framework for me. :)
You can read the doc in https://github.com/apache/apisix/blob/master/docs/en/latest/internal/testing-framework.md
dd52597 to
642c15c
Compare
| --- grep_error_log eval | ||
| qr/sending a batch logs to 127.0.0.1:(\d+)/ | ||
| --- grep_error_log_out | ||
| sending a batch logs to 127.0.0.1:5044 |
There was a problem hiding this comment.
This check only ensures the log is sent to port 5044, which is already checked by other tests.
It is necessary to check the content of the log via injected code like
Line 238 in dc4b354
There was a problem hiding this comment.
I changed syslog server port.
There was a problem hiding this comment.
Simply modifying the port is useless ， we should check the format of the reported logs to ensure that log_format is in effect, see: https://github.com/apache/apisix/blob/master/t/plugin/kafka-logger-log-format.t
There was a problem hiding this comment.
Could you please check again?
| --- no_error_log | ||
| [error] | ||
| --- error_log eval | ||
| qr/syslog-log-format.*\{.*"host":"localhost"/ |
There was a problem hiding this comment.
I run this test locally and it can pass even without setting metadata, because the default log format already contains "host":"localhost" in
apisix/apisix/utils/log-util.lua
Line 128 in ab4fe88
There was a problem hiding this comment.
I added upstream variable into log format.
|
Please make the CI pass, thanks! |
* upstream/master: (48 commits) fix(ai): remove BUILD_ROUTER event when ai module is unloaded (apache#8184) chore: add some comment for make_request_to_vault function (apache#8420) docs: update admin api English doc (apache#8227) ci: use fixed os version of ubuntu (apache#8438) feat: Support store secrets in secrets manager for auth plugin via kms components (apache#8421) feat: interact via gRPC in APISIX Admin API (apache#8411) fix: last_err can be nil when the reconnection is successful (apache#8377) feat: support global data encryption of secret information (apache#8403) refactor(env): rename funtion name (apache#8426) feat(admin): add kms admin api (apache#8394) docs: update consumer and upstream docs (apache#8223) ci: add cron job for GM (apache#8398) docs: add kms env doc (apache#8419) feat: Added log format support in syslog plugin. (apache#8279) feat: add vault common components (apache#8412) docs: update global-rule/plugin-config/plugin/ docs (apache#8262) docs: update consumer-group/router/service/script doc (apache#8332) feat: support store secret in env for auth plugin (apache#8390) docs: update Upgrade Guide CN version (apache#8392) docs: add GM plugin EN doc to make website display normally (apache#8393) ...
3 participants
Fixes #8278