[core.logging] Request/Response logging cleanup tasks.

[lukeelmers]

In #87939 we merged support for request/response logging to the KP. As that PR was already quite large, there are a few cleanup tasks that we wanted to address separately:

1. Clean up default pattern layout

The response logs contain a lot of meta which clutters up the console quite a bit and makes the logs difficult to scan, as the default log pattern places the meta before the message. Here's an example:

[2021-02-04T22:13:20.382Z][DEBUG][http.server.response] {"ecs":{"version":"1.7.0"},"client":{"ip":"127.0.0.1"},"http":{"request":{"method":"GET","mime_type":null,"referrer":"http://localhost:5601/ahl/app/dashboards","headers":{"connection":"keep-alive","pragma":"no-cache","cache-control":"no-cache","user-agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36","dnt":"1","accept":"*/*","sec-fetch-site":"same-origin","sec-fetch-mode":"no-cors","sec-fetch-dest":"script","referer":"http://localhost:5601/ahl/app/dashboards","accept-encoding":"gzip, deflate, br","accept-language":"en-US,en;q=0.9,ja;q=0.8,gl;q=0.7,es;q=0.6","cookie":"[REDACTED]","x-forwarded-for":"127.0.0.1","x-forwarded-port":"60795","x-forwarded-proto":"http","x-forwarded-host":"localhost:5601","host":"localhost:5601"}},"response":{"body":{"bytes":56765},"status_code":200,"headers":{"content-type":"application/javascript; charset=utf-8","etag":"\"18832936a2c8ab58934beb031ec10bedc394f5fc-/ahl/9007199254740991/bundles/plugin/visTypeTimeseries/-gzip\"","cache-control":"must-revalidate","kbn-name":"kbn-blue","kbn-license-sig":"dbdfb6e0d6b82db37115e70770dcef6a759bdc7c764195552e83d98b76a846ab","vary":"accept-encoding","content-encoding":"gzip"},"responseTime":7}},"url":{"path":"/9007199254740991/bundles/plugin/visTypeTimeseries/visTypeTimeseries.chunk.8.js","query":""},"user_agent":{"original":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36"}} GET /9007199254740991/bundles/plugin/visTypeTimeseries/visTypeTimeseries.chunk.8.js 200 7ms - 55.4KB 

We'd like to clean these up by default, and discussed a few options for how we might do this:

1. Remove meta property from default pattern layout
2. Relocate meta property to the end of default pattern layout (doesn't decrease clutter, but makes it easier to scan the logs as the message appears first)
3. Take an approach similar to Elasticsearch where context-specific defaults are applied via a dedicated config file
4. Provide some recommended defaults in the kibana.yml that we ship with Kibana (would require users to uncomment if they choose to use them)

Overall (3) seems ideal to me because it provides the most flexibility, however creating default appender configurations poses some challenges:

• We want to provide a set of predefined defaults only if the user hasn't already set defaults
• configSchema default settings feels like the natural place to do this
• However the default appenders value is never actually used because we inject the "default" legacy appender, so a value always exists there
• So any additional defaults would, AFAICT, need to be loaded in kbn-config's LegacyObjectToConfigAdapter as well (might be fine as a temporary solution until it is removed in 8.0?)

2. Improve get_payload_bytes

The way we are calculating payload bytes can be continually improved, but there are a few immediate things we should address:

• Ensure object responses that are arrays get counted (in [core.logging] Fix issue where logs fail to calculate size of gunzip streams. #90353 this was changed to fix a bug that was breaking Fleet on master, but we should go back and adjust it)
• Add support for zlib streams which are used in Fleet and SIEM

cc @restrry Any other thoughts?

[elasticmachine]

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

[mshustov]

Elasticsearch has to use (3) because it is a log4j configuration interface.
In my opinion, it's not always optimal since the configuration is scattered across different files.
What if we do (4)? Why they have to be commented out by default?

Remove meta property from default pattern layout

How many places in the codebase rely on this? It can be an option requiring the least amount of effort.

[lukeelmers]

@restrry

What if we do (4)? Why they have to be commented out by default?

I don't see why we couldn't uncomment by default, and of course this would be a really simple solution. I suppose I just hadn't considered that as an option since there isn't any precedent for it. Although this wouldn't work for folks who don't use kibana.yml at all and just start up the server with CLI args.

How many places in the codebase rely on this? It can be an option requiring the least amount of effort.

AFAIK we aren't relying on the meta being in the new logging format anywhere at all -- the only place I can think of is filebeat, but it's still using legacy log fields which wouldn't be affected, so I'm +1 for this idea as well.

I think the larger feature of default/"preset" configs would be easier to address in 8.x anyway once we get rid of the legacy infrastructure, so if we're comfortable with simply removing the meta from the default pattern as an interim solution, I'd be okay with that.

[mshustov]

the only place I can think of is filebeat, but it's still using legacy log fields which wouldn't be affected

How is filebeat going to work from v8.0 without access to meta values? Do we need to start talking to them?

AFAIK we aren't relying on the meta being in the new logging format anywhere at all

In this case, removing looks like the optimal option as it doesn't require significant work.

I don't see why we couldn't uncomment by default, and of course this would be a really simple solution. I suppose I just hadn't considered that as an option since there isn't any precedent for it.

The problem with this solution that users can configure a path to a custom kibana.yml file. This means we cannot rely on config/kibana.yml file content to be read

kibana/packages/kbn-utils/src/path/index.ts

Lines 16 to 28 in e9b9bcc

	const CONFIG_PATHS = [
	process.env.KBN_PATH_CONF && join(process.env.KBN_PATH_CONF, 'kibana.yml'),
	process.env.KIBANA_PATH_CONF && join(process.env.KIBANA_PATH_CONF, 'kibana.yml'),
	process.env.CONFIG_PATH, // deprecated
	join(REPO_ROOT, 'config/kibana.yml'),
	].filter(isString);
	const CONFIG_DIRECTORIES = [
	process.env.KBN_PATH_CONF,
	process.env.KIBANA_PATH_CONF,
	join(REPO_ROOT, 'config'),
	'/etc/kibana',
	].filter(isString);

We'd have to introduce another file dedicated to logging configuration.

[lukeelmers]

How is filebeat going to work from v8.0 without access to meta values? Do we need to start talking to them?

Yeah it is on my list to raise this with them, I plan to open an issue and will ping the team.

The problem with this solution that users can configure a path to a custom kibana.yml file

Good point, hadn't considered that. So that's two strikes against this idea:

• Users might not be using the yml anyway if they pass args via CLI alone
• And users might be pointing to a custom yml file at a different path

In this case, removing looks like the optimal option as it doesn't require significant work.

++ Agreed, this feels like the best choice at this point. And I think if we wanted to, we could revisit option (3) in 8.0 once the legacy system is removed. I will move that to a separate issue.

[lukeelmers]

Opened two related issues:

• Filebeat issue highlighting the need to prepare for breaking changes in 8.0 [filebeat] Upgrade Kibana module to prepare for 8.0 breaking changes beats#24136
• Logging enhancement for supporting logger-specific defaults [core.logging] Provide mechanism for configuring default appenders for specific loggers #92082

To summarize the remaining scope of this issue:

• Remove meta from default pattern layout [core.logging] Remove meta from default pattern layout. #92104
• Make adjustments to getPayloadBytes to properly track arrays & zlib streams [core.logging] Fix calculating payload bytes for arrays and zlib streams. #92100