Skip to content
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

[Breaking change] Switch logging to ECS-JSON layout by default #114968

Closed
Mpdreamz opened this issue Oct 14, 2021 · 16 comments
Closed

[Breaking change] Switch logging to ECS-JSON layout by default #114968

Mpdreamz opened this issue Oct 14, 2021 · 16 comments
Assignees
Labels
Breaking Change Feature:Logging Feature:Upgrade Assistant impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team

Comments

@Mpdreamz
Copy link
Member

Change description

Which release will ship the breaking change?

TBD, The sooner the better but we'd have to have a deprecation period.

We could potentially evaluate enabling it by default on Cloud as an opaque change to the user in Isolation.

Is this a Kibana or Elasticsearch breaking change?

Kibana

Describe the change. How will it manifest to users?

Switching the default logging format to ECS is a move several of Elastic products are making with 8.0.

Switching to ECS structured logs helps unifying our log collection and aids with log correlation. E.g logs with trace.id can be used to enable quick navigation from Discover to Logs UI to APM Trace overview in any direction.

How many users will be affected?

Anyone manually ingesting Kibana logs.

Users using the kibana module from Filebeat to ingest kibana logs.

Are there any edge cases?

Unknown if any of the open items #60391 influences a decision.

Cross links

elastic/elasticsearch#47105
elastic/apm-server#3829
elastic/beats#15544

@botelastic botelastic bot added the needs-team Issues missing a team label label Oct 14, 2021
@mshustov mshustov added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc and removed needs-team Issues missing a team label labels Oct 14, 2021
@elasticmachine
Copy link
Contributor

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

@mshustov
Copy link
Contributor

mshustov commented Oct 14, 2021

Reading #52226, I believe it should have been done there. But we stopped at just refactoring the JSON format to be ECS-compliant.
I'm not sure we can switch the default right now without a deprecation period (at least via Upgrade Assistant UI).

However, I talked to @pgomulka about how Elasticsearch approached the breaking change.
The Elasticsearch team is going to introduce the breaking change without a deprecation message, the change will be reflected in the migration guide only (elastic/elasticsearch#79146)

Right now, only Kibana and Beats don't use ECS-JSON by default https://github.com/elastic/dev/issues/1323
Beats team is working on the matter in elastic/beats#15544 @andresrc is that planned for v8.0?

@lukeelmers @kobelb What do you think about introducing this breaking change at such a late stage respecting Make it minor initiative?
If we aren't comfortable with adding a breaking change, we can document how to make the ECS-JSON format the default and plan the work for the next major release.

@lukeelmers
Copy link
Member

I'm not sure we can switch the default right now without a deprecation period (at least via Upgrade Assistant UI).

What do you think about introducing this breaking change at such a late stage

If making this the default behavior is the direction the rest of the stack is going (including Elasticsearch), then I think it makes sense for us to do so as well.

I'm not super concerned about introducing the breaking change without a deprecation period if we are able to squeeze it into 8.0 (we should discuss feasibility of this), because:

  1. We are already breaking all of the logging formats in 8.0 anyway (both pattern & json layouts), so this will mostly require updating documentation to make the changes clear to users
  2. As the json layout is already ECS-compliant, it shouldn't be an enormous effort for us to update the default

@kobelb
Copy link
Contributor

kobelb commented Oct 14, 2021

I wish we had a time machine to go back in time and know that when we switch to the new logging format we should've used ECS-JSON intsead of just JSON. Since we don't, I think that you all are proposing is the best course of action given the tough position we're in.

I've asked @lukeelmers to send an email to @mfinkle summarizing the breaking change we're proposing making to get his official approval.

@lukeelmers
Copy link
Member

We've confirmed we're good to go on this (thanks @mfinkle). I'll take a look at it. I think we'll need to split this into a couple of pieces:

  1. PR to 7.x to check for users who are (a) using the new logging configs, and (b) using the default layout, so that we can log a deprecation warning and surface instructions in the Upgrade Assistant.
  2. PR to 8.0/master to switch the default layout to use json instead of pattern.
    • As a part of this I will see if it's possible to keep pattern layout as the default in dev mode, as most devs are working with the default settings & json is much harder to read in the console. If necessary this could also be a separate PR.
  3. PR to add clearer documentation (we discussed the idea of a "logging upgrade guide") to help provide a high-level overview for users. Currently we have lots of documentation on the new logging system, but it is fairly detailed & low-level.

when we switch to the new logging format we should've used ECS-JSON intsead of just JSON

@kobelb To clarify on this, the new JSON format is ECS compliant. The issue is just that it isn't our default layout. (This was done to be consistent with the legacy configuration, where you had to explicitly say logging.json: true). So in this case I think we're mostly concerned with changing that default.

@lukeelmers lukeelmers self-assigned this Oct 14, 2021
@lukeelmers lukeelmers added Feature:Logging impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:small Small Level of Effort labels Oct 14, 2021
@andresrc
Copy link

Right now, only Kibana and Beats don't use ECS-JSON by default elastic/dev#1323 Beats team is working on the matter in elastic/beats#15544 @andresrc is that planned for v8.0?

We are looking at it, at least while running through agent, but we don't know if we will be able to add it yet.

@lukeelmers
Copy link
Member

lukeelmers commented Oct 19, 2021

As I started on the work for this, logging to the console in json by default just started to feel bad; the logs aren't easily human readable in json unless you are piping them through a tool like jq, in which case they are usually pretty huge and difficult to follow.

So I did some more digging into the Elasticsearch implementation linked here (elastic/elasticsearch#47105). I was surprised to find that, after pulling the latest master and running ES from source, my console logs were still in plaintext format.

After discussing the Elasticsearch implementation details with @pgomulka, I realized that the Kibana behavior actually is already consistent with ES:

The main difference is that ES has a concept of "default" configs for each appender, which we do not have yet in Kibana, so with their changes, the default format for logs written to files will change to ECS JSON. However, in Elasticsearch the default format that is used when logging to the console continues to be the plaintext layout (equivalent to Kibana's "pattern" layout).

Since Kibana doesn't have a concept of default appender configurations, our behavior out of the box is to log to the console only. Like ES, these console logs are using the pattern/plaintext layout. But unlike ES, if a user wants to write Kibana logs to a file, there is no default format: they are forced to choose between json and pattern layouts when writing the configuration. And if they choose json, those logs will already be ECS-compliant... starting in 8.0, there is no non-ECS-JSON option in Kibana.

All this is to say, after spending some time working through this, I think we can close this issue as already done, assuming I've understood the feature request correctly. When Kibana's logs are configured to use json format, they will already be ECS-compliant.

@Mpdreamz WDYT, is there anything I'm missing here? If not, I think we can close this issue.

cc @kobelb

PS @pgomulka let me know if there are any details I've missed from our chat 🙂

@kobelb
Copy link
Contributor

kobelb commented Oct 19, 2021

Thanks for doing this additional research, @lukeelmers. What you've communicated here makes total sense to me, and I support your conclusions.

@Mpdreamz
Copy link
Member Author

Knowing this is the default on Cloud, now more prominently in the docs as default appender + deprecating the pattern formatter is a fantastic outcome 👍

Thank you @lukeelmers and everyone on the thread!

@felixbarny
Copy link
Member

What's the default log format in Docker containers for ES an Kibana in 8.0? As logging in Docker is going through stdout, will we also default to the plaintext/pattern formatter? Or do we override that default in the Docker containers?

@mshustov
Copy link
Contributor

Or do we override that default in the Docker containers?

cc @elastic/kibana-operations

@tylersmalley
Copy link
Contributor

We don't override the default. The only override is here, where we set the cGroup overrides.

@felixbarny
Copy link
Member

According to @pgomulka, Elasticsearch is explicitly overriding the logging configuration for Docker so that it logs ECS-JSON. When starting from the IDE, for example, Elasticsearch defaults to plain-text logs.

@tylersmalley Did you consider following this approach and also default to ECS-JSON logs on Docker? This would make log ingestion and parsing for Docker/k8s that much easier.

@pgomulka
Copy link

pgomulka commented Nov 10, 2021

Elasticsearch defaults to plain-text logs.

might be worth noting that console logs are plain-text server logs which are in addition to JSON log files. The intention of those logs is for developers and admins checking on them. JSON logs are meant for machine consumption.
All of the logs - indexing and search slow logs, deprecation logs and server logs - are available in ECS JSON as well. Even when running from IDE

@LeeDr
Copy link

LeeDr commented May 5, 2022

I'm reopening this because our tar.gz archive installs don't log to file in ecs format by default but the rpm/deb package installs do. I'm not sure if/how we can also have the console log be plain text (which seems desired).

@LeeDr LeeDr reopened this May 5, 2022
@jbudz jbudz added the Team:Operations Team label for Operations Team label May 5, 2022
@jbudz
Copy link
Member

jbudz commented Nov 17, 2022

I'm reopening this because our tar.gz archive installs don't log to file in ecs format by default but the rpm/deb package installs do. I'm not sure if/how we can also have the console log be plain text (which seems desired).

@LeeDr I know we discussed this on slack a bit ago, reading though the thread now #114968 (comment) seems like this was considered acceptable. I'm going to re-close this but let me know if I'm missing anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Feature:Logging Feature:Upgrade Assistant impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team
Projects
None yet
Development

No branches or pull requests