fluent-plugins-nursery / fluent-plugin-systemd

This is a fluentd input plugin. It reads logs from the systemd journal.
Apache License 2.0
153 stars 43 forks source link

Use storage plugin helper to replace pos_file #31

Closed cosmo0920 closed 7 years ago

cosmo0920 commented 7 years ago

Hi, I've found that storage plugin, which is designed for storing plugin states, usage in this plugin.

tested on the below conf:

<source>
  @type systemd
  path /var/log/journal
  tag kube-proxy
  read_from_head true
  @id in-systemd
  <storage>
    @type local
    persistent true
    path ./tmp/storage.json
  </storage>
</source>
<match kube-proxy>
  @type stdout
</match>

pos_file implementation should be replaced with well-tested storage_local plugin. How do you think about this patches?

Thanks.

reevoo-samuel commented 7 years ago

Er, hello?

This Pull Request is too damn old! Merge or close this, sucka.

cosmo0920 commented 7 years ago

What do you mean "too damn old"? Should I rebase this PR??

errm commented 7 years ago

Hey @cosmo0920 sorry about @reevoo-samuel, he is an unfriendly bot we run against our organisation to encorage people to merge things more quckly...

I don't wan't to merge this just yet until I have got a working fix for journal rotation out... one thing at a time :)

Is this being used by any of the core plugins? When I wrote this plugin the pos file stuff was loosely based on in_tail I think...

cosmo0920 commented 7 years ago

Yep, local storage plugin is used in in_dummy core plugin and will be used in fluent-plugin-windows-eventlog. And I created fluent-plugin-storage-redis and fluent-plugin-storage-mongo. We can store position information into these KVS when using storage plugin.

cosmo0920 commented 7 years ago

And this PR is loosely based on https://github.com/fluent/fluent-plugin-windows-eventlog/pull/5.

errm commented 7 years ago

Hey @cosmo0920 I have done a little refactoring on this to push most of the complexity back into the PosWriter class, doing so also allowed me to add some code to copy the cursor from the old file to the new storage.

cosmo0920 commented 7 years ago

Awsome! Your fix is really what I'd wanted to do. :smile:

errm commented 7 years ago

Great!

I have rebased this on the fixes for journal rotation, I am going to release the journal rotation stuff tomorrow, this needs some documentation... so should go out a day or two after, perhaps Wednesday if I am not too busy.

errm commented 7 years ago

Thanks for your contribution @cosmo0920 I have just released a new version including this change https://github.com/reevoo/fluent-plugin-systemd/releases/tag/v0.2.0