Skip to content

feat(kafka-logger): add max req/resp body size attributes#11133

Merged
moonming merged 8 commits into
apache:masterfrom
shreemaan-abhishek:fix/kfk-logger-large-body-high-cpu
Apr 16, 2024
Merged

feat(kafka-logger): add max req/resp body size attributes#11133
moonming merged 8 commits into
apache:masterfrom
shreemaan-abhishek:fix/kfk-logger-large-body-high-cpu

Conversation

@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

Description

In cases where the req/response body is very large and when include_req_body/resp_body is enabled this leads to very high CPU usage and can even cause unavailability. To fix such event, two new attributes are introduced:

  • max_resp_body_bytes
  • max_req_body_bytes

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)

@shreemaan-abhishek shreemaan-abhishek changed the title feat(kfk-logger): add max req/resp body size attributes feat(kafka-logger): add max req/resp body size attributes Apr 9, 2024
@shreemaan-abhishek
shreemaan-abhishek marked this pull request as ready for review April 10, 2024 01:10
Comment thread apisix/plugins/kafka-logger.lua Outdated
local math = math
local pairs = pairs
local type = type
local req_body = ngx.req.read_body

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
local req_body = ngx.req.read_body
local req_read_body = ngx.req.read_body

Comment thread apisix/core/response.lua Outdated
ctx._body_buffer[ctx._plugin_name] = nil
return body_buffer
if max_resp_body_bytes and #body_data >= max_resp_body_bytes then
body_data = str_sub(body_data, 1, max_resp_body_bytes)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to sub the body_data again here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in 213, we just sub the chunk this happens in the case when this is the first and last chunk of body_filter phase. (since eof == true and body_buffer is nil).

in 221 we need to sub because it is the last chunk but not the very first one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if it's the first chunk, it will be dealed in line ~202

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm now it makes sense but I think we are calling sub on 213 on chunk because body_buffer is nil. And on 221 we are calling it on body data formed after concat_tab the body_buffer table.

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.

3 participants