-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[core.logging] Fix calculating payload bytes for arrays and zlib streams. #92100
Conversation
Pinging @elastic/kibana-core (Team:Core) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
return ( | ||
!isBoom(res) && | ||
res.variety === 'stream' && | ||
res.source === src && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need such a sophisticated check? Maybe duck-typing would be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-evaluating each of the typeguards, there's actually quite a bit of cleanup we can do -- for example, we're already checking isBoom
earlier in the function so we don't need those checks duplicated in the typeguards, and testing reference equality of res.source
and src
isn't strictly necessary.
I went ahead and simplified each of them to take the source
object and the variety
string.
The one for zlib streams is still a little more complex as TS will complain if we just try to check variety === 'stream' && obj.bytesWritten
, as obj
is unknown
, hence the typeof
and null
checks.
@elasticmachine merge upstream |
@elasticmachine merge upstream @restrry This is ready for another look whenever you get the chance. |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @lukeelmers |
Closes #90457
Summary
This PR fixes 2 bugs related to how we calculate payload bytes for the purposes of http response logging which was introduced in #87939:
Example logs with calculated zlib payloads
Reviewing / Testing
It is recommended to preview the diff with "hide whitespace changes" enabled.
You need to enable
http.server.response
logs to test this. Here's the config I've been testing with: