feat: file logger plugin support response body in variable by mscb402 · Pull Request #8711 · apache/apisix

mscb402 · 2023-01-19T03:38:21Z

Description

We need to add a new var resp_body to let the user put it in a log format which let the user use the response in the way there want.

Fixes #8705

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
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 first)

spacewander · 2023-01-29T06:55:44Z

@@ -0,0 +1,124 @@
+#


Could we move these tests under https://github.com/apache/apisix/blob/master/t/plugin/file-logger2.t? That file is still small and doesn't have special setup.

Move to file-logger2

spacewander · 2023-01-29T06:58:15Z

    metadata_schema = metadata_schema
 }

+core.ctx.register_var("resp_body", function(ctx)


Would be better to put it in https://github.com/apache/apisix/blob/master/apisix/core/ctx.lua so other loggers can use it too.

I don't think this var should be put into the ctx.lua file. Because the value of resp_body comes from ctx.resp_body, which is set by file-logger when include_resp_body = true.

Not only one logger plugin will support recording the response body. file-logger.lua is not an appropriate place.

spacewander · 2023-01-29T06:59:34Z

+| ---- | ------ | -------- |------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | path | string | True     | Log file path. |
-| include_resp_body      | boolean | False     | When set to `true` includes the response body in the log file.                                                                                                                                                                |
+| include_resp_body      | boolean | False     | When set to `true` includes the response body in the log file. If format is set, you can use the variable $resp_body to get the response body.                                                                                     |


Would be better to put it in https://github.com/apache/apisix/blob/master/docs/en/latest/apisix-variable.md, instead of modifying each logger's doc.
Note that we need to mean that this variable is only for logger and requires the logger to have a special configuration.

spacewander · 2023-01-30T03:41:06Z

        route_name = true,
        service_id = true,
        service_name = true,
+        resp_body = function(ctx)


apisix/apisix/core/ctx.lua

Line 206 in c8d5afb

-- sort in alphabetical

spacewander · 2023-01-30T03:41:46Z

        service_id = true,
        service_name = true,
+        resp_body = function(ctx)
+            -- only work when file-logger set include_resp_body = true


The comment is incorrect. See #8711 (comment)

tokers · 2023-02-06T02:10:52Z

 | service_name        | core       | Name of Service.                                                                    |                |
 | redis_cmd_line      | Redis      | The content of Redis command.                                                       |                |
 | rpc_time            | xRPC       | Time spent at the rpc request level.                                                |                |
+| resp_body           | core       | This variable is only for logger and requires the logger to have a special configuration. |                |


Which special configuration? I'm afraid that such a description is not useful for users to use this variable.

I was update it

spacewander · 2023-02-07T02:18:48Z

        route_id = true,
        route_name = true,
        service_id = true,
-        service_name = true,


Why remove the comma?

I readded it

spacewander · 2023-02-07T02:25:27Z

 | service_name        | core       | APISIX 服务的名称。                                                               |                   |
 | redis_cmd_line      | Redis      | Redis 命令的内容。                                                                |                   |
 | rpc_time            | xRPC       | 在 RPC 请求级别所花费的时间。                                                      |                   |
+| resp_body           | core       | 此变量仅用于 Logger，并要求对应的 Logger 需要具备某些配置。                            |                  |


Please put it in alphabetical order

spacewander · 2023-02-07T02:25:47Z

 | service_name        | core       | Name of Service.                                                                    |                |
 | redis_cmd_line      | Redis      | The content of Redis command.                                                       |                |
 | rpc_time            | xRPC       | Time spent at the rpc request level.                                                |                |
+| resp_body           | core       | This variable is only for logger and requires the logger plugin to have a special configuration like `include_resp_body`. |                |


We should explain what this variable is, and what decides its value

Co-authored-by: 罗泽轩 <[email protected]>

* upstream/master: docs: change the file name to 'create-ssl.py'.If 'ssl.py' is used as … (apache#8623) feat: Body transformer plugin (apache#8766) fix: mocking plugin panic when response_example contain $ (apache#8810) (apache#8816) feat: file logger plugin support response body in variable (apache#8711) docs(getting-started): rewrite the install section (apache#8807) feat: allow each logger to define custom log format in its conf (apache#8806) fix(etcd): reloaded data may be in res.body.node (apache#8736) fix: fix fetch all service info from consul (apache#8651) docs: fix global-rule.md wrong curl address (apache#8793) feat: stream subsystem support consul service discovery (apache#8696) chore(kafka-logger): support configuration `meta_refresh_interval` parameter (apache#8762) feat: ready to release 2.15.2 (apache#8783)

mscb402 added 5 commits January 19, 2023 11:29

chore: code lint fix

3e19f1d

chore: modify test

04f69bc

chore: modify test

667dd96

chore: fix file logger e2e error

e195c71

fix: log file conflicts

1847971

mscb402 marked this pull request as ready for review January 28, 2023 06:19

spacewander reviewed Jan 29, 2023

View reviewed changes

mscb402 added 2 commits January 29, 2023 16:43

fix: move file-logger3.t to file-logger2.t

9a98eed

fix: move var to ctx.lua

3aeb37d

spacewander reviewed Jan 30, 2023

View reviewed changes

mscb402 added 5 commits January 31, 2023 15:25

chore: update doc

823b709

chore: update variable doc

acd458e

Update file-logger.md

27175d9

Update file-logger.md

e24a839

Update apisix-variable.md

61c2f75

mscb402 requested review from spacewander and tokers and removed request for spacewander and tokers February 3, 2023 05:43

tokers reviewed Feb 6, 2023

View reviewed changes

chore: update variable doc

13497ca

mscb402 requested a review from spacewander February 7, 2023 01:17

spacewander reviewed Feb 7, 2023

View reviewed changes

mscb402 added 2 commits February 7, 2023 10:38

chore: update variable doc

cc40731

chore: format doc

b44407c

spacewander reviewed Feb 9, 2023

View reviewed changes

Comment thread docs/en/latest/apisix-variable.md Outdated

soulbird reviewed Feb 9, 2023

View reviewed changes

Comment thread docs/zh/latest/apisix-variable.md Outdated

mscb402 and others added 2 commits February 9, 2023 09:17

Update docs/en/latest/apisix-variable.md

b7d0690

Co-authored-by: 罗泽轩 <[email protected]>

Update apisix-variable.md

2bda6af

soulbird previously approved these changes Feb 9, 2023

View reviewed changes

mscb402 requested a review from tokers February 9, 2023 02:37

Update apisix-variable.md

4d45204

mscb402 dismissed soulbird’s stale review via 4d45204 February 9, 2023 08:45

soulbird approved these changes Feb 9, 2023

View reviewed changes

tokers approved these changes Feb 9, 2023

View reviewed changes

spacewander approved these changes Feb 10, 2023

View reviewed changes

spacewander merged commit 0d41d3a into apache:master Feb 10, 2023

mscb402 deleted the feat/file_logger_support_resp_body_var branch May 16, 2023 08:33

Uh oh!

Conversation

mscb402 commented Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

spacewander Jan 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mscb402 commented Jan 19, 2023 •

edited

Loading

spacewander Jan 29, 2023 •

edited

Loading