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

[Logging] Add date-pattern file naming during log-rotation #132400

Open
Tracked by #134169
afharo opened this issue May 18, 2022 · 4 comments
Open
Tracked by #134169

[Logging] Add date-pattern file naming during log-rotation #132400

afharo opened this issue May 18, 2022 · 4 comments
Labels
enhancement New value added to drive a business result Feature:Logging Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@afharo
Copy link
Member

afharo commented May 18, 2022

Currently, Kibana only supports one Rolling strategy: numeric

We could extend this behaviour with an additional Rolling strategy: date, or maybe support the pattern to declare dates (pattern: %d{yyyy-MM-dd})?

It could be useful for folks using the Time-interval triggering policy to understand what are the times each log file contains just by looking at the file names.

@afharo afharo added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc enhancement New value added to drive a business result Feature:Logging labels May 18, 2022
@elasticmachine
Copy link
Contributor

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

@pgayvallet
Copy link
Contributor

When I initially implemented the rolling file strategy, I wanted to implement a Date rolling strategy in the initial version. The problem is, given the rolling strategy (HOW to name/move/delete the files) and policies (WHEN to roll the files) are dissociated, is was way more complex than anticipated to handle the max number of files to keep when rolling files with non-deterministic (date-based) patterns, especially given you can specify any arbitrary interval for the time-based rolling, and as rolling can happen at any time with size-based rolling.

We could have such date-based strategy not have a max number of file parameters, but instead be time-based (similar to what log4j does), but that means implementing a similar pattern-matching based file removal, as we can't just assume that all files in the log folders are log files.

Also, we need some kind of 'communication' between the policy and strategy for the date rolling strategy to have more knowledge of the rolling interval, at least for the case of the time based rolling. E.g if the rolling is 24h, but rolls because of log offset at 00:00:02, we still want to use the previous day's date. FWIW log4j itself had to implement a lot of spaghetti code for this policy-to-strategy context.

So basically I'm all in for the feature, but I don't have a clear idea how to technically implement it.

@afharo
Copy link
Member Author

afharo commented Jul 25, 2024

This issue has come up again in a support case.

I wonder if we should try to unblock the challenges mentioned in the previous comment. IIUC, there are 3 main points:

  1. Dependency in frequency and pattern
  2. Pattern-matching on deletion
  3. We currently trigger a roll on a new log entry, not time-based.

Is my understanding correct? Did I miss anything?

@pgayvallet
Copy link
Contributor

pgayvallet commented Jul 25, 2024

I think implementing this issue would make sense yes, it's a reasonnable enhancement that makes sense and could benefit more than a few customers ihmo.

Is my understanding correct? Did I miss anything?

I think that you listed everything. A few remarks:

  1. Dependency in frequency and pattern

For the communication between the various components of the rolling file appender, I used the context as a bridge in #182346,

#orderedRolledFileFn?: GetOrderedRolledFileFn;
public async getOrderedRolledFiles(): Promise<string[]> {
if (this.#orderedRolledFileFn) {
return this.#orderedRolledFileFn();
}
throw new Error('orderedRolledFileFn not registered on the context');
}
public setOrderedRolledFileFn(fn: GetOrderedRolledFileFn) {
this.#orderedRolledFileFn = fn;
}

And even if the setter/getter pattern is quite ugly, it does the job, and I think we could use the same pattern for any additional communication we need.

  1. Pattern-matching on deletion

Here again, something was started in #182346, as the rolling strategy is now in charge of listing the file for the retention to decide which ones should be deleted. So I think the main concern here is to be able to properly pattern match which file belongs to our appender (so yeah, matching on format), which may be a little challenging but hopefully could be inferred from the file format

  1. We currently trigger a roll on a new log entry, not time-based.

Now that I think again about it, I don't think this is an issue on its own. It's mostly about being able to keep track of which file we will be rolling / what should be the next rolling file.

Rolling on fixed time instead of by-event don't really solve this by itself (we've already fixed similar issues when rolling during daily saving time change, and rolling at fixed time wouldn't have helped), so it's really about being able to predict which file we're supposed to roll to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Logging Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

3 participants