-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
rotatingfile: add basic rotating file plugin #3922
Conversation
What happened with this for the 1.7.0 release? :-( |
|
||
// FilePerm defines the permissions that Writer will use for all | ||
// the files it creates. | ||
var FilePerm = os.FileMode(0666) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about I see this plugin creates the file, in that case, let's change stat
ing the current file/root directory to re-use the permissions when re-creating.FilePerm
s mode to 0644
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I agree. Fixed.
} | ||
|
||
var sampleConfig = ` | ||
## Files to write to, "stdout" is a specially handled file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this comment is copypasta, remove or revise.
Not sure why that build is failing. Did the plugin format change again? It was good before 1.7.0 |
@flexd |
Changed that to the new format. I just looked at the code I have running in production here, and it still has the old metric.Serialize(), even if we are on a 1.6 build. Perhaps I pulled the code just before the change happened :) Last commit I have there is 8c51d62. Let me know if there's anything else I need to do! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add this to the existing file output instead of having it as a standalone plugin?
I think it should be possible to use the existing file
option in place of root
and filename_prefix
, and then just rename max_age
to rotate_max_age
to give it a little more context.
I haven't looked yet at the rotated naming method, could you add a bit about that in the README.md. In my mind it should be something like:
mail.log
mail.log.0
mail.log.1
Or potentially this style which doesn't require multiple renames:
shorewall-init.log
shorewall-init.log-20180617
Sure, we could do that I guess. I figured you would rather want a separate plugin, to separate concerns and make the implementation a bit cleaner. The files currently look like this.
There is a current file that is written to, and at the end of each write if the file is too old it will be closed, renamed to This is to ensure it's unique, and we do not have to track state of what was the last file, which would be gone if we had a restart, and we would have to store that somewhere or discover it from the folder each time. We could change that to just use file, so I think keeping the two plugins separate would be best considering, but it's up to you. Anyway, it's working well. We have used this in production for, I'd say at least 6 months at this point. I use it to export Telegraf output on remote servers and ship the files home with Apache NiFi across VPNs/network boundaries, where they are finally dropped on disk on a host that picks them up and sends them to InfluxDB. This is one of the reasons I want the filenames to be as unique as possible. I could deal with this in NiFi easily, but I figured there is no need to make the plugin more complicated than it needs to be. The files can be sorted chronologically and are all roughly the same size. We could add some logic to rotate the file by file size or bytes written instead of time if someone needs that, but currently this fulfills our need perfectly. |
This should work, later on we could always add support for other naming schemes, if it is needed.
Yes, make this optional (comment it out in the config), and if it is set to a value that is not the "zero value" then enable rotation. |
One more thing. The I'm not sure why you would want to write to multiple files to begin with, but I assume people might be using it now and that we would not want to remove that now? |
Yes, if the Multiple files is rarely used in my experience except during debugging to write to both stdout and a file. We cannot remove it now but I think it shouldn't be a problem for rotation. If a user only wants to rotate one file they can always define the plugin multiple times. |
Sorry this has taken so long, I'll try to shoe-horn in some time to work on this before christmas :-) Lots of stuff to do, and my initial PR works for us. |
I like the idea of rotating. This would make using telegraf on offline IoT devices much easier to collect and ship up metrics once plugged in and connected to a network. |
Sorry this has taken forever! See the related pull-request. :-) Works in my test environment but I have not tried it in production yet. Should have all the suggestions from this PR implemented in the file plugin. |
Required for all PRs:
Hi there! I finally got around to submitting this, as some of us talked about in #1550
This is a output plugin very similar to the normal file output plugin, but with file rotation. A use-case for this is if you want to move metrics across a network boundary and moving files is a lot easier than establishing connections through strict firewalls or any other similar situation.
The filename prefix, max file age and root path are all configurable.
I have been using this for a few months without any issues, other than a time I managed to move the current file while it was in use, which will end up breaking. It might be possible to handle that, but avoiding that situation is a good enough fix for me at this moment.
I hope this is useful for someone else. I will continue to use this for my use-case and it would be great if I could use a packaged version of Telegraf instead of building my own.
PS: This is pretty much a merge of the existing file plugin + a "how to rotate a file in Go" query in a search-engine. It is not very complicated and could probably be improved.