feat(log): add nested log format support for logger plugins
Description
This PR adds support for nested log format in logger plugins. Previously, the log_format configuration only supported flat key-value structures. This feature allows users to more efficiently organize and group log fields by defining hierarchical/overlapping log structures.
For example, user can set log_format as below:
{
"log_format": {
"host": "$host",
"client_ip": "$remote_addr",
"request": {
"method": "$request_method",
"uri": "$request_uri",
"headers": {
"user_agent": "$http_user_agent"
}
},
"response": {
"status": "$status"
}
}
}
Output Example:
{
"response": {
"status": 200
},
"route_id": "1",
"client_ip": "127.0.0.1",
"request": {
"method": "GET",
"uri": "/",
"headers": {
"user_agent": "curl/8.5.0"
}
},
"host": "127.0.0.1"
}
Which issue(s) this PR fixes:
Fixes #
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
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)
Hi @TaeyeongKwak, do we need to limit the maximum nesting level? In addition, you need to add the corresponding documentation content.
Hi @TaeyeongKwak, do we need to limit the maximum nesting level? In addition, you need to add the corresponding documentation content.
Hi @Baoyuantop, Thank you for your comment. Come to think of it, I agree that we should set a maximum depth as you mentioned. Considering typical use cases, I think allowing a maximum depth of five levels would be sufficient. I'll apply this change and update the documentation accordingly.
And currently, I've only added test cases to some plugins. Should I add test cases for the other plugins too?
Hi @TaeyeongKwak, I think creating a test for just one plugin is sufficient.
Hi @TaeyeongKwak, I think creating a test for just one plugin is sufficient.
@Baoyuantop
Got it. I’ll keep only the tests I’ve written so far.
I notice some CI checks are currently failing, but they don’t seem related to the changes in this PR. Is there anything I might be missing?
Hi @TaeyeongKwak, I saw some errors related to sls-logger. Could you run a test locally to check? You can refer to http://apisix.apache.org/docs/apisix/build-apisix-dev-environment-devcontainers/
Hi @TaeyeongKwak, I saw some errors related to sls-logger. Could you run a test locally to check? You can refer to http://apisix.apache.org/docs/apisix/build-apisix-dev-environment-devcontainers/
Hi @Baoyuantop,
Thank you for the suggestion. I've tested sls-logger locally following the guide you provided, and the tests passed successfully:
root@a1bd1377d5dd:/workspace# git branch --show-current
feat-support-nested-log-format
root@a1bd1377d5dd:/workspace# FLUSH_ETCD=1 prove -Itest-nginx/lib -I. -r t/plugin/sls-logger.t
t/plugin/sls-logger.t .. ok
All tests successful.
Files=1, Tests=49, 32 wallclock secs ( 0.02 usr 0.00 sys + 1.00 cusr 0.91 csys = 1.93 CPU)
Result: PASS
Is there anything I might have missed while performing the test?
Hi @TaeyeongKwak, we've made some test fixes in the main branch; you can merge the code from the main branch.
The failed CI was unrelated to the changes in this PR.
- CI / build (ubuntu-latest, linux_openresty, lua-resty-worker-events, t/plugin/[l-z]*) (pull_request)
two ways we can make a try:
- rerun the failed test case
- merge the latest
masterbranch