search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.99k stars 8.24k forks source link

[KP] Implement Log rotation appender #56291

Closed mshustov closed 3 years ago

mshustov commented 4 years ago

The operations team shipped built-in log rotations at https://github.com/elastic/kibana/pull/49750. It doesn't make sense to migrate the legacy version since we want to provide log rotation capability in the form of a separate appender.

Requirements

log4j2 version has quite a big API surface https://logging.apache.org/log4j/2.x/manual/appenders.html#RollingFileAppender From the very beginning, we are going to support a limited sub-set of API to cover the Audit Logging use-case. Requirements from @jportner :

The other options that Elasticsearch provides (such as strategy action conditions, file name, etc.) are nice-to-have, but not hard requirements to fulfill for Audit Logging.

LP migration

When we stop maintaining the LP logging system we should cont contact the Cloud team to use the new appender instead of legacy log-rotation.

elasticmachine commented 4 years ago

Pinging @elastic/kibana-platform (Team:Platform)

pgayvallet commented 4 years ago

as an NP plugin or an appender

Did not look at #49750 implementation, but is is technically possible to do this from outside of an appender?

mshustov commented 4 years ago

Did not look at #49750 implementation, but is is technically possible to do this from outside of an appender?

For sure. Although we need to check the current implemented in elasticsearch. I skimmed the docs some time ago and though it provides this functionality as the RollingFile appender https://www.elastic.co/guide/en/elasticsearch/reference/current/logging.html

jportner commented 4 years ago

The new Audit Logging feature (#52125) will need to use log rotation. I would love to be able to use the NP log rotation for this purpose. To do so, we will need to be able to:

These are all currently provided by the Elasticsearch/Log4j log rotation. It would be great if we could use the same configuration keys, too.

The other options that Elasticsearch provides (such as strategy action conditions, file name, etc.) are nice-to-have, but not hard requirements to fulfill for Audit Logging.

legrego commented 4 years ago

Do we have a rough prioritization for this? We would really like this for the new audit logger, especially to facilitate ingestion within ESS.

mshustov commented 4 years ago

@legrego not yet, but we are in the planning phase right now. In what release do you need it?

elasticmachine commented 4 years ago

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

pgayvallet commented 4 years ago

Specify a retention period that is number-based (current) or time-based (not yet implemented)

Correct me if I'm wrong, but as long as we implement a time-based roll policy, even number-based retention would technically be time based, as we would retain X file * retention period.

After looking at the log4j implementation, date-based file patterns/rolling is a pain (need to pattern match the existing files against the date pattern, sort then depending on the parsed date...), so I'm unsure this could be done for 7.11

Would time-based roll policy with number-based file be acceptable for a first version, or is the log-2020-12-30T16:00:00.log format / rolling an absolute necessity?

legrego commented 4 years ago

Would time-based roll policy with number-based file be acceptable for a first version, or is the log-2020-12-30T16:00:00.log format / rolling an absolute necessity?

IMO a number-based file would be fine. @przemek-grzedzielski @s-nel @Kushmaro, will a number-based file be acceptable for Kibana's logs on ESS, or will ESS require the log files to have a timestamp suffix?

Kushmaro commented 4 years ago

@legrego we depend on filebeat, so no real importance to the numbering, just need to have the log file moved(renamed) and not copied and truncated in-place, so that filebeat can keep the handle on the rolled over log.