apisix icon indicating copy to clipboard operation
apisix copied to clipboard

feat(log): add nested log format support for logger plugins

Open TaeyeongKwak opened this issue 2 months ago • 9 comments

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)

TaeyeongKwak avatar Oct 24 '25 04:10 TaeyeongKwak

Hi @TaeyeongKwak, do we need to limit the maximum nesting level? In addition, you need to add the corresponding documentation content.

Baoyuantop avatar Oct 28 '25 03:10 Baoyuantop

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?

TaeyeongKwak avatar Oct 28 '25 23:10 TaeyeongKwak

Hi @TaeyeongKwak, I think creating a test for just one plugin is sufficient.

Baoyuantop avatar Nov 10 '25 07:11 Baoyuantop

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?

TaeyeongKwak avatar Nov 10 '25 22:11 TaeyeongKwak

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/

Baoyuantop avatar Nov 13 '25 06:11 Baoyuantop

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?

TaeyeongKwak avatar Nov 14 '25 02:11 TaeyeongKwak

Hi @TaeyeongKwak, we've made some test fixes in the main branch; you can merge the code from the main branch.

Baoyuantop avatar Nov 21 '25 03:11 Baoyuantop

The failed CI was unrelated to the changes in this PR.

Baoyuantop avatar Dec 09 '25 01:12 Baoyuantop

  • CI / build (ubuntu-latest, linux_openresty, lua-resty-worker-events, t/plugin/[l-z]*) (pull_request)

two ways we can make a try:

  1. rerun the failed test case
  2. merge the latest master branch

membphis avatar Dec 09 '25 05:12 membphis