[core.logging] Fix issue where logs fail to calculate size of gunzip streams.

[lukeelmers]

After #87939 was merged, @jen-huang noticed that the visiting the Fleet > Integrations UI would cause the Kibana server to crash with a message from the logging system:

(node:23021) [DEP0108] DeprecationWarning: zlib.bytesRead is deprecated and will change its meaning in the future. Use zlib.bytesWritten instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Node.js process-warning detected:
DeprecationWarning: zlib.bytesRead is deprecated and will change its meaning in the future. Use zlib.bytesWritten instead.
    at getResponsePayloadBytes (/Users/jen/Projects/kibana/src/core/server/http/logging/get_payload_size.ts:57:30)
    at getEcsResponseLog (/Users/jen/Projects/kibana/src/core/server/http/logging/get_response_log.ts:54:17)
    at Object.handleServerResponseEvent [as listener] (/Users/jen/Projects/kibana/src/core/server/http/http_server.ts:302:36)
    at module.exports.internals.Podium.emit (/Users/jen/Projects/kibana/node_modules/@hapi/podium/lib/index.js:221:110)
    at Request._finalize (/Users/jen/Projects/kibana/node_modules/@hapi/hapi/lib/request.js:511:27)
    at Request._reply (/Users/jen/Projects/kibana/node_modules/@hapi/hapi/lib/request.js:457:14)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
Terminating process...
 server crashed  with status code 1

The issue came from get_response_payload, where we are attempting to calculate the size of the http response payload for logging purposes. In this case the isFsReadStream type guard was incorrectly letting zlib's Gunzip streams through as there wasn't an explicit instanceof ReadStream check. Since gunzip is only used a few places in Kibana (which apparently don't have functional test coverage?), this went unnoticed.

Normally this would be fine since we swallow any payload calculation errors in a try/catch as this is not mission-critical info. However, it just so happens that bytesRead is a deprecated method in zlib which will emit a Node process warning.

And normally that would be fine too, except we intentionally exit the process on any Node warning that isn't explicitly on our allow list. My understanding is that this only happens while in dev mode, so a temporary workaround until this PR is merged would be running node --no-warnings scripts/kibana serve --dev.

While I'd like to incorporate payload size calculation for gunzip streams, I'm opening this PR with an immediate fix in the interim so that Fleet is no longer broken on master:

• Updates typeguards for fs.ReadStream to check instanceof ReadStream
• While reviewing the other typeguards, I noticed the isObject checks were also not strict enough and letting too much through, so I used lodash isPlainObject for those instead
• Updated unit tests to ensure zlib streams return undefined as they are supposed to.

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)

[TinaHeiligers]

Just a thought: Should we add the Node process warning that's causing the issue to out allow list to prevent future issues?

Code and fix LGTM!

[TinaHeiligers]

suggestion but not a blocker: add an inline comment as to why we need to explicitly check the instance type:

Suggested change

	res.source instanceof ReadStream
	res.source instanceof ReadStream // prevent zlib's Gunzip streams through

[jen-huang]

Didn't read code but tested locally and Fleet UI is working normally now 👍🏻 Thanks for the quick fix!

[lukeelmers]

Should we add the Node process warning that's causing the issue to out allow list to prevent future issues?

I had considered that, but this is a fairly specific issue to logging -- AFAICT nowhere else in Kibana is attempting to read bytesRead, and even if they were I would expect they'd discover this issue almost immediately as it crashes the server entirely.

This was a perfect storm of making a change to a low-level part of the platform that touches everything, and having one particular area of Kibana that didn't have functional test coverage which happened to be using a different type of stream (and which I also didn't happen to test manually).

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 8d72406

Metrics [docs]

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[mshustov]

it seems to be a valid use-case when you want to respond with an array of objects. isn't it? @lukeelmers