feat: rocketmq logger#5653
Conversation
5b1819d to
d99eb83
Compare
0f5e698 to
0a58971
Compare
|
Pls keep going:-) I'd like to see this feature ready for both community cooperation. |
|
|
||
| repeat_each(1); | ||
| no_long_string(); | ||
| no_root_location(); |
There was a problem hiding this comment.
LGTM. We can improve the test in a separate PR:
The newly added test file should use a preprocessor to reduce duplicate --- request & --- no_error_log section. You can take a look at:
And please split it into rocketmq-logger.t & rocketmq-logger2.t. Each test file should be smaller than 800L.
| return false, "failed to send data to rocketmq topic: " .. err .. | ||
| ", nameserver_list: " .. core.json.encode(conf.nameserver_list) |
There was a problem hiding this comment.
Can we get the nameserver for the current error?
There was a problem hiding this comment.
Sorry, I dont understand what you mean. the nameserver is add to err and test case will verify the error log if send fail. such as https://github.com/yuz10/apisix/blob/master/t/plugin/rocketmq-logger.t#L862
There was a problem hiding this comment.
if we have more nameservers in nameserver_list, one of them is unavailable and this message is sent to the unavailable nameserver, can the log here only print out the unavailable nameserver?
Ok, if this is not convenient to do in this PR, we can optimise it later.
There was a problem hiding this comment.
Which nameserver is error can be returned from the lua-resty-rocketmq library, We can create an issue there.
|
Thank you very much for your outstanding contribution @yuz10 |
8 participants
What this PR does / why we need it:
feature: add rocketmq-logger plugin
which send log to rocketmq.
Pre-submission checklist: