Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support file-logger plugin of response body record #8414

Conversation

pixeldin
Copy link
Contributor

Description

Providing some optional setting items for file-logger to control the output log content as requested in #8331.

I refer some options of the http-logger:
include_req_body, include_resp_body and include_resp_body_expr

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)

@pixeldin pixeldin changed the title Supporting file-logger plugin to record response body feat: support file-logger plugin of response body record Nov 28, 2022
@tzssangglass
Copy link
Member

hi @pixeldin , we have error log as attempt to set ngx.status after sending out response headers, we need to avoid call ngx.status = code after ngx.say, try with

diff --git t/plugin/file-logger2.t t/plugin/file-logger2.t
index ccd1d9f5..ad4396c0 100644
--- t/plugin/file-logger2.t
+++ t/plugin/file-logger2.t
@@ -164,7 +164,6 @@ contain with target
 
             if new_msg.response.body == "hello world\n"
             then
-                ngx.status = code
                 ngx.say('contain target body hits with expr')
             end
 
@@ -174,7 +173,6 @@ contain with target
             local new_msg = core.json.decode(msg)
             if new_msg.response.body == nil
             then
-                ngx.status = code
                 ngx.say('skip unconcern body')
             end
         }

tzssangglass
tzssangglass previously approved these changes Dec 6, 2022
t/plugin/file-logger2.t Outdated Show resolved Hide resolved
t/plugin/file-logger2.t Outdated Show resolved Hide resolved
apisix/plugins/file-logger.lua Outdated Show resolved Hide resolved
tzssangglass
tzssangglass previously approved these changes Dec 7, 2022
@pixeldin
Copy link
Contributor Author

pixeldin commented Dec 8, 2022

@tzssangglass please retrigger the CI workflow, the running state seems unstable.

@spacewander
Copy link
Member

@tzssangglass please retrigger the CI workflow, the running state seems unstable.

Done

t/plugin/file-logger2.t Outdated Show resolved Hide resolved
t/plugin/file-logger2.t Outdated Show resolved Hide resolved
t/plugin/file-logger2.t Outdated Show resolved Hide resolved
@spacewander spacewander merged commit c5fc10d into apache:master Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants