[core.logging] Remove logging config validation that requires default appender to be present. #97323

lukeelmers · 2021-04-15T21:05:09Z

cc @mshustov @TinaHeiligers @joshdover As discussed offline, let's use this PR to discuss whether we want to get this change into 7.13, or wait.

Updates the logging config to remove the validation which requires that users of the KP logging system specify a default appender for the root context.

The default appender is responsible for forwarding all logs to the legacy logging system to preserve BWC with legacy log formats. The original rationale for enforcing the default appender is present was to ensure that folks don't accidentally break legacy logs when using the new system. The downside of the default appender, however, is that it creates duplicate log entries for every record: one in the legacy format, one in the new format.

Now that all of the legacy logging features are available in the new system, we think it would be okay to lift this requirement. Folks who want to opt-in to the new logging system may also opt-out of the legacy one by excluding the default appender. And those who need to preserve the legacy formats can keep it in place.

One important thing to note here is that filebeat's kibana module still relies on the legacy logging format, and will break if users relying on it remove the default appender. The impact here is that the new log format uses different field names, so for filebeat users who have set up dashboards/filters/etc based on these fields, those will no longer work if default is removed. This situation also applies to anyone who isn't using filebeat but is still relying on the legacy log format for ingesting Kibana logs. I've updated the docs to reflect this.

Here's a screengrab showing an example of how the fields differ (These are two different log entries, but you get the idea. Legacy format on the left, new format on the right.)

Plugin API Changes

Kibana's core logging service maintains a default logging appender to provide backwards compatibility with our legacy logging format, which will be removed in 8.0. Previously, including the default appender in your root context was a requirement:

logging:
  root:
    appenders: [default, console] # previously `default` was required
    level: info

The downside of this approach was that you would get duplicate entries in your Kibana logs: one in legacy format (via default appender), and one in the new format.

We have now lifted this requirement for folks who want to fully opt-in to the new system.

logging:
  root:
    appenders: [console] # no `default` necessary
    level: info

Please note, however, that removing the default appender may break existing logging integrations you have which rely on the legacy Kibana logging format. In particular, if you use the Filebeat Kibana module, we do not recommend removing this setting as it will cause your Kibana logs to be indexed under different field names. The same recommendation applies for folks who have set up other custom integrations with the Kibana logs: keeping the default in place ensures that your logs will not break in 7.x.

… present.

elasticmachine · 2021-04-15T21:19:50Z

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

lukeelmers · 2021-04-15T21:21:23Z

docs/settings/logging-settings.asciidoc

@@ -36,7 +44,7 @@ logging:
 ==== Log in json format

 Log the default log format to json layout instead of pattern (the default).
-With `json` layout log messages will be formatted as JSON strings in https://www.elastic.co/guide/en/ecs/current/ecs-reference.html[ECS format] that includes a timestamp, log level, logger, message text and any other metadata that may be associated with the log message itself
+With `json` layout log messages will be formatted as JSON strings in https://www.elastic.co/guide/en/ecs/1.9/ecs-reference.html[ECS format] that includes a timestamp, log level, logger, message text and any other metadata that may be associated with the log message itself


Unrelated to the topic at hand, but figured I would update this while I'm touching the file.

TinaHeiligers

Please note, however, that removing the default appender may break existing logging integrations you have which rely on the legacy Kibana logging format. In particular, if you use the Filebeat Kibana module, we do not recommend removing this setting as it will cause your Kibana logs to be indexed under different field names. The same recommendation applies for folks who have set up other custom integrations with the Kibana logs: keeping the default in place ensures that your logs will not break in 7.x.

IMHO, These are very important caveats and I'd be hesitant to go forward with this until we have approval from Product and the Beats teams to do so. I'd prefer not to rush until we have timeframes from the Beats team and logging use cases that they are taking the necessary steps to migrate to the Kibana logging system configuration.

lukeelmers · 2021-04-15T22:50:37Z

@TinaHeiligers One alternative here would be to wait on this and try to update the filebeat module to have backwards compatibility with the affected fields (the tags & the log level are the main offenders). If we took an approach like this, we could then release the filebeat update at the same time as this change. I'm not sure how exactly we'd populate the tags, so we'd need to investigate this further to see what's even feasible, but thought I'd raise it as another idea. Still wouldn't solve the issue for folks with a custom integration, though.

I don't have strong feelings one way or another on this decision, however I do think it's worth noting that this would still be opt-in, in that by default nobody would have legacy logs taken away. So action would be required before they would get into this situation... today nobody uses the new logging system as we haven't announced it (and I mean literally nobody, according to the telemetry). So it's a question of whether someone is going to take the initiative to update their config in 7.x without testing it first.

kibanamachine · 2021-04-15T23:12:38Z

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

API count

id	before	after	diff
`actions`	-	113	+113
`advancedSettings`	-	18	+18
`alerting`	-	200	+200
`apm`	-	35	+35
`apmOss`	-	38	+38
`banners`	-	9	+9
`beatsManagement`	-	2	+2
`bfetch`	-	58	+58
`canvas`	-	4	+4
`cases`	-	2	+2
`charts`	-	157	+157
`cloud`	-	19	+19
`console`	-	2	+2
`core`	-	2168	+2168
`dashboard`	-	138	+138
`dashboardEnhanced`	-	51	+51
`dashboardMode`	-	11	+11
`data`	-	3436	+3436
`dataEnhanced`	-	52	+52
`devTools`	-	9	+9
`discover`	-	61	+61
`discoverEnhanced`	-	39	+39
`embeddable`	-	426	+426
`embeddableEnhanced`	-	14	+14
`encryptedSavedObjects`	-	20	+20
`enterpriseSearch`	-	2	+2
`esUiShared`	-	84	+84
`eventLog`	-	70	+70
`expressions`	-	1755	+1755
`features`	-	199	+199
`fileUpload`	-	175	+175
`fleet`	-	1075	+1075
`globalSearch`	-	67	+67
`home`	-	91	+91
`indexLifecycleManagement`	-	5	+5
`indexManagement`	-	162	+162
`indexPatternFieldEditor`	-	29	+29
`indexPatternManagement`	-	46	+46
`infra`	-	14	+14
`ingestPipelines`	-	9	+9
`inspector`	-	97	+97
`kibanaLegacy`	-	92	+92
`kibanaReact`	-	232	+232
`kibanaUtils`	-	465	+465
`lens`	-	160	+160
`licenseManagement`	-	3	+3
`licensing`	-	116	+116
`lists`	-	251	+251
`management`	-	35	+35
`maps`	-	182	+182
`mapsEms`	-	69	+69
`ml`	-	343	+343
`monitoring`	-	9	+9
`navigation`	-	29	+29
`newsfeed`	-	17	+17
`observability`	-	176	+176
`osquery`	-	8	+8
`presentationUtil`	-	69	+69
`remoteClusters`	-	4	+4
`reporting`	-	127	+127
`rollup`	-	20	+20
`ruleRegistry`	-	52	+52
`runtimeFields`	-	22	+22
`savedObjects`	-	173	+173
`savedObjectsManagement`	-	91	+91
`savedObjectsTagging`	-	54	+54
`savedObjectsTaggingOss`	-	81	+81
`security`	-	79	+79
`securityOss`	-	11	+11
`securitySolution`	-	99	+99
`share`	-	66	+66
`snapshotRestore`	-	22	+22
`spaces`	-	82	+82
`spacesOss`	-	64	+64
`stackAlerts`	-	4	+4
`taskManager`	-	41	+41
`telemetry`	-	41	+41
`telemetryCollectionManager`	-	24	+24
`telemetryCollectionXpack`	-	1	+1
`telemetryManagementSection`	-	13	+13
`timelines`	-	6	+6
`triggersActionsUi`	-	221	+221
`uiActions`	-	127	+127
`uiActionsEnhanced`	-	185	+185
`uptime`	-	5	+5
`urlForwarding`	-	14	+14
`usageCollection`	-	53	+53
`visTypeTimeseries`	-	7	+7
`visualizations`	-	212	+212
total			+15219

API count missing comments

id	before	after	diff
`actions`	-	113	+113
`advancedSettings`	-	17	+17
`alerting`	-	200	+200
`apm`	-	35	+35
`apmOss`	-	38	+38
`banners`	-	9	+9
`beatsManagement`	-	2	+2
`bfetch`	-	47	+47
`canvas`	-	3	+3
`cases`	-	2	+2
`charts`	-	132	+132
`cloud`	-	19	+19
`console`	-	2	+2
`core`	-	989	+989
`dashboard`	-	126	+126
`dashboardEnhanced`	-	50	+50
`dashboardMode`	-	11	+11
`data`	-	2944	+2944
`dataEnhanced`	-	34	+34
`devTools`	-	8	+8
`discover`	-	48	+48
`discoverEnhanced`	-	37	+37
`embeddable`	-	361	+361
`embeddableEnhanced`	-	14	+14
`encryptedSavedObjects`	-	18	+18
`enterpriseSearch`	-	2	+2
`esUiShared`	-	82	+82
`eventLog`	-	70	+70
`expressions`	-	1364	+1364
`features`	-	89	+89
`fileUpload`	-	175	+175
`fleet`	-	985	+985
`globalSearch`	-	13	+13
`home`	-	67	+67
`indexLifecycleManagement`	-	5	+5
`indexManagement`	-	157	+157
`indexPatternFieldEditor`	-	27	+27
`indexPatternManagement`	-	46	+46
`infra`	-	11	+11
`ingestPipelines`	-	9	+9
`inspector`	-	77	+77
`kibanaLegacy`	-	84	+84
`kibanaReact`	-	207	+207
`kibanaUtils`	-	301	+301
`lens`	-	149	+149
`licenseManagement`	-	3	+3
`licensing`	-	40	+40
`lists`	-	234	+234
`management`	-	35	+35
`maps`	-	181	+181
`mapsEms`	-	69	+69
`ml`	-	339	+339
`monitoring`	-	9	+9
`navigation`	-	29	+29
`newsfeed`	-	17	+17
`observability`	-	176	+176
`osquery`	-	8	+8
`presentationUtil`	-	67	+67
`remoteClusters`	-	4	+4
`reporting`	-	126	+126
`rollup`	-	20	+20
`ruleRegistry`	-	52	+52
`runtimeFields`	-	17	+17
`savedObjects`	-	159	+159
`savedObjectsManagement`	-	80	+80
`savedObjectsTagging`	-	50	+50
`savedObjectsTaggingOss`	-	42	+42
`security`	-	38	+38
`securityOss`	-	8	+8
`securitySolution`	-	90	+90
`share`	-	60	+60
`snapshotRestore`	-	22	+22
`spaces`	-	65	+65
`spacesOss`	-	29	+29
`stackAlerts`	-	4	+4
`taskManager`	-	16	+16
`telemetry`	-	37	+37
`telemetryCollectionManager`	-	24	+24
`telemetryCollectionXpack`	-	1	+1
`telemetryManagementSection`	-	12	+12
`timelines`	-	6	+6
`triggersActionsUi`	-	212	+212
`uiActions`	-	88	+88
`uiActionsEnhanced`	-	135	+135
`uptime`	-	5	+5
`urlForwarding`	-	14	+14
`usageCollection`	-	46	+46
`visTypeTimeseries`	-	7	+7
`visualizations`	-	194	+194
total			+12049

API count with any type

id	before	after	diff
`bfetch`	-	1	+1
`charts`	-	2	+2
`core`	-	147	+147
`dashboard`	-	1	+1
`data`	-	81	+81
`embeddable`	-	2	+2
`esUiShared`	-	4	+4
`expressions`	-	44	+44
`fileUpload`	-	6	+6
`fleet`	-	16	+16
`indexManagement`	-	12	+12
`inspector`	-	6	+6
`kibanaLegacy`	-	3	+3
`kibanaUtils`	-	3	+3
`maps`	-	2	+2
`mapsEms`	-	1	+1
`ml`	-	14	+14
`reporting`	-	1	+1
`ruleRegistry`	-	1	+1
`savedObjects`	-	3	+3
`share`	-	1	+1
`snapshotRestore`	-	1	+1
`triggersActionsUi`	-	1	+1
`visualizations`	-	11	+11
total			+364

Non-exported public API item count

id	before	after	diff
`actions`	-	6	+6
`advancedSettings`	-	1	+1
`alerting`	-	9	+9
`apm`	-	8	+8
`bfetch`	-	2	+2
`canvas`	-	2	+2
`cases`	-	1	+1
`charts`	-	2	+2
`core`	-	29	+29
`dashboard`	-	9	+9
`data`	-	76	+76
`dataEnhanced`	-	2	+2
`devTools`	-	2	+2
`discover`	-	6	+6
`discoverEnhanced`	-	2	+2
`embeddable`	-	3	+3
`encryptedSavedObjects`	-	4	+4
`esUiShared`	-	4	+4
`eventLog`	-	4	+4
`expressions`	-	7	+7
`features`	-	1	+1
`fleet`	-	10	+10
`globalSearch`	-	6	+6
`home`	-	8	+8
`indexManagement`	-	4	+4
`indexPatternFieldEditor`	-	4	+4
`indexPatternManagement`	-	4	+4
`infra`	-	2	+2
`ingestPipelines`	-	4	+4
`inspector`	-	4	+4
`kibanaLegacy`	-	1	+1
`kibanaReact`	-	3	+3
`kibanaUtils`	-	8	+8
`lens`	-	12	+12
`licensing`	-	10	+10
`lists`	-	60	+60
`management`	-	4	+4
`maps`	-	16	+16
`ml`	-	34	+34
`navigation`	-	3	+3
`observability`	-	7	+7
`presentationUtil`	-	4	+4
`reporting`	-	18	+18
`ruleRegistry`	-	4	+4
`runtimeFields`	-	2	+2
`savedObjects`	-	5	+5
`savedObjectsManagement`	-	1	+1
`security`	-	10	+10
`securityOss`	-	3	+3
`securitySolution`	-	7	+7
`share`	-	3	+3
`snapshotRestore`	-	1	+1
`spaces`	-	3	+3
`taskManager`	-	3	+3
`telemetry`	-	3	+3
`telemetryCollectionManager`	-	4	+4
`timelines`	-	1	+1
`triggersActionsUi`	-	28	+28
`uiActions`	-	9	+9
`uiActionsEnhanced`	-	10	+10
`uptime`	-	3	+3
`usageCollection`	-	7	+7
`visTypeTimeseries`	-	2	+2
`visualizations`	-	6	+6
total			+521

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

cc @lukeelmers

mshustov · 2021-04-16T12:07:31Z

We're announcing the new logging config in the upcoming 7.13 release. The current implementation enforcing the default appended doesn't provide the best UX for the default case. Because most of our external users are not filebeat users who have set up dashboards/filters/etc based on these fields. Those who need to ingest Kibana logs can continue using the legacy system or opt-in deafult appender.

One alternative here would be to wait on this and try to update the filebeat module to have backwards compatibility with the affected fields (the tags & the log level are the main offenders).

I'm okay to wait. Do you have any idea when it will be fixed? The sooner users learn about the changes the more time they have to upgrade their log ingestion pipeline.

joshdover · 2021-04-19T14:01:59Z

One alternative here would be to wait on this and try to update the filebeat module to have backwards compatibility with the affected fields (the tags & the log level are the main offenders).

+1 on waiting for filebeat support. Our products should work together without caveats like this. I'm ok with not having compatibility with the tags field in filebeat, but I think the log level is too critical. Ideally, filebeat ships with support for v7 and v8 log format and we lift this requirement in the same version.

lukeelmers · 2021-04-19T14:12:04Z

Do you have any idea when it will be fixed? The sooner users learn about the changes the more time they have to upgrade their log ingestion pipeline.

The issue tracking this is elastic/beats#24136 -- I'll check in to see if there are any timing updates.

Ideally, filebeat ships with support for v7 and v8 log format and we lift this requirement in the same version.

This makes sense to me. Since it seems we're all in agreement to wait for filebeat, I'll go ahead and move this back into draft and we can revisit when the time comes.

lukeelmers added 2 commits April 15, 2021 14:52

Remove logging config validation that requires default appender to be…

6e98962

… present.

Add warnings about default appender removal to the docs.

c220086

lukeelmers added release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Feature:Logging labels Apr 15, 2021

lukeelmers self-assigned this Apr 15, 2021

lukeelmers added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.13.0 v8.0.0 labels Apr 15, 2021

lukeelmers marked this pull request as ready for review April 15, 2021 21:19

lukeelmers requested a review from a team as a code owner April 15, 2021 21:19

lukeelmers commented Apr 15, 2021

View reviewed changes

TinaHeiligers reviewed Apr 15, 2021

View reviewed changes

lukeelmers marked this pull request as draft April 19, 2021 14:12

lukeelmers removed v7.13.0 v8.0.0 labels Apr 19, 2021

This was referenced Apr 21, 2021

[Meta] URL Service #87304

Closed

Introduce Locators in share plugin #98104

Closed

lukeelmers mentioned this pull request Apr 23, 2021

[filebeat] Upgrade Kibana module to prepare for 8.0 breaking changes elastic/beats#24136

Open

lukeelmers closed this Nov 15, 2021

lukeelmers deleted the remove-default-appender-validation branch November 15, 2021 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core.logging] Remove logging config validation that requires default appender to be present. #97323

[core.logging] Remove logging config validation that requires default appender to be present. #97323

lukeelmers commented Apr 15, 2021 •

edited

Loading

elasticmachine commented Apr 15, 2021

lukeelmers Apr 15, 2021

TinaHeiligers left a comment

lukeelmers commented Apr 15, 2021

kibanamachine commented Apr 15, 2021

API count

API count missing comments

API count with any type

Non-exported public API item count

mshustov commented Apr 16, 2021

joshdover commented Apr 19, 2021

lukeelmers commented Apr 19, 2021

[core.logging] Remove logging config validation that requires default appender to be present. #97323

[core.logging] Remove logging config validation that requires default appender to be present. #97323

Conversation

lukeelmers commented Apr 15, 2021 • edited Loading

Plugin API Changes

elasticmachine commented Apr 15, 2021

lukeelmers Apr 15, 2021

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

lukeelmers commented Apr 15, 2021

kibanamachine commented Apr 15, 2021

💚 Build Succeeded

Metrics [docs]

API count

API count missing comments

API count with any type

Non-exported public API item count

mshustov commented Apr 16, 2021

joshdover commented Apr 19, 2021

lukeelmers commented Apr 19, 2021

lukeelmers commented Apr 15, 2021 •

edited

Loading