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

[KP] Implement Log rotation appender #56291

Closed
mshustov opened this issue Jan 29, 2020 · 10 comments · Fixed by #84735
Closed

[KP] Implement Log rotation appender #56291

mshustov opened this issue Jan 29, 2020 · 10 comments · Fixed by #84735
Assignees
Labels
Feature:Logging Feature:New Platform NeededFor:Security Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@mshustov
Copy link
Contributor

mshustov commented Jan 29, 2020

The operations team shipped built-in log rotations at #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 :

  • Define different log rotation config for each appender (e.g., one for stdout and a separate one for audit logging)
  • Specify a rolling policy that is size-based or time-based
  • For time-based rolling, specify whether or not to modulate (align rolls on the day boundary as opposed to every twenty-four hours)
  • Specify a retention period that is number-based or time-based

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.

@mshustov mshustov added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Jan 29, 2020
@elasticmachine
Copy link
Contributor

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

@pgayvallet
Copy link
Contributor

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
Copy link
Contributor Author

mshustov commented Feb 2, 2020

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
Copy link
Contributor

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:

  • Define different log rotation config for each appender (e.g., one for stdout and a separate one for audit logging)
  • Specify a roll policy that is size-based (current) or time-based (not yet implemented)
  • For time-based rolling, specify whether or not to modulate (align rolls on the day boundary as opposed to every twenty-four hours)
  • Specify a retention period that is number-based (current) or time-based (not yet implemented)

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.

@mshustov mshustov added Feature:Logging and removed Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Feb 26, 2020
@joshdover joshdover mentioned this issue Mar 17, 2020
30 tasks
@jportner jportner mentioned this issue Jul 1, 2020
12 tasks
@mshustov mshustov changed the title [NP] Migrate Log rotation [KP] Implement Log rotation appender Jul 1, 2020
@legrego
Copy link
Member

legrego commented Nov 16, 2020

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
Copy link
Contributor Author

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

@alexh97 alexh97 added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Nov 20, 2020
@elasticmachine
Copy link
Contributor

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

@pgayvallet pgayvallet self-assigned this Dec 1, 2020
@pgayvallet
Copy link
Contributor

pgayvallet commented Dec 1, 2020

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
Copy link
Member

legrego commented Dec 1, 2020

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
Copy link

Kushmaro commented Dec 3, 2020

@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.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Logging Feature:New Platform NeededFor:Security Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants