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

rotation - possibility of log rotations of 2,3,4,5,x minutes or hours #11602

Closed
wants to merge 7 commits into from
Closed

rotation - possibility of log rotations of 2,3,4,5,x minutes or hours #11602

wants to merge 7 commits into from

Conversation

leopucci
Copy link
Contributor

@leopucci leopucci commented Apr 2, 2019

No description provided.

leopucci and others added 7 commits March 25, 2019 16:12
After: Log name based on file change
Before: Log name based on rotation timestamp.

This patch solves the problem of filenames containing 00-00-00 and adds the possibility of log rotations of 2,3,4,5 m or hours (not only 1m,1h)
Important to mention that log rotation is based on logging.metrics.period. So if you put a time of 10s but logging.metrics.period default is 30s, it will rotate logs each 30s.
t.Format calls AppendFormat that breaks time into year/month/day/hour..etc.. 
Now yearly rotation will rotate with fixed format filebeat-2019-1
@leopucci leopucci requested a review from a team as a code owner April 2, 2019 13:21
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@leopucci
Copy link
Contributor Author

leopucci commented Apr 2, 2019

@michalpristas I have correted the tests.

There is a difference that before, you had fixed times and for all other time values an arbitrary time function.
No you can use 1,2,3,4,5 sec/min/hour/etc to rotate the time. As a consequence of being allowed to choose time, you do not have arbitrary time anymore. So I removed the arbitrary tests and the code of it.
Please take a look

@leopucci
Copy link
Contributor Author

leopucci commented Apr 2, 2019

Will stash now

@@ -61,38 +62,31 @@ func newIntervalRotator(interval time.Duration) (*intervalRotator, error) {
func (r *intervalRotator) initialize() error {
r.clock = realClock{}

switch r.interval {
case time.Second:
if r.interval < time.Minute {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: better use switch statements instead of if-elseif-else...

switch {
case r.interval < time.Minute:
   ...
case r.interval < time.Hour:
   ...
...
default:
    ...
}

How about defining some constants like const day = 24 * time.Hour?

Moving this selection into an extra function makes it testable (often it's the most simplest code that contains the bug :) ).

@kvch
Copy link
Contributor

kvch commented Feb 28, 2020

@leopucci Could you please add a Changelog entry and update the PR as requested?

@ph ph added the Team:Services (Deprecated) Label for the former Integrations-Services team label Feb 28, 2020
@botelastic
Copy link

botelastic bot commented Jul 17, 2020

Hi!
We just realized that we haven't looked into this PR in a while. We're sorry!

We're labeling this issue as Stale to make it hit our filters and make sure we get back to it in as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1.
Thank you for your contribution!

@botelastic botelastic bot added the Stalled label Jul 17, 2020
@botelastic
Copy link

botelastic bot commented Aug 16, 2020

Hi!
This PR has been stale for a while and we're going to close it as part of our cleanup procedure.
We appreciate your contribution and would like to apologize if we have not been able to review it, due to the current heavy load of the team.
Feel free to re-open this PR if you think it should stay open and is worth rebasing.
Thank you for your contribution!

@botelastic botelastic bot closed this Aug 16, 2020
@zube zube bot removed the [zube]: Done label Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stalled Team:Services (Deprecated) Label for the former Integrations-Services team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants