search

tsaikd / gogstash

Logstash like, written in golang
MIT License
644 stars 106 forks source link

Added ip2location filter #165

Closed helgeolav closed 3 years ago

helgeolav commented 3 years ago

ip2location is another geo database, like geoip2 - but another provider. This filter is built using their own official library and is based on the code from the geoip2 filter.

I also added some code for FileWatcher - but should this be placed somewhere else? My intention is to reuse this code other places (geoip2, ip2proxy) when the database is updated.

To be able to test this filter yourself you need to create a free account on https://lite.ip2location.com

tsaikd commented 3 years ago

https://github.com/fsnotify/fsnotify Is it what you want for file watcher?

helgeolav commented 3 years ago

Hi

Here are my answers.

  1. I looked at some fsnotify alternatives, including the one you mentioned. I decided not to use any of them. The reason being that I need to add external modules that are strictly not needed. fsnotify has many open issues and PRs - and it is more work to integrate this as it over time the implementation and its API will probably change. FileWatcher does not provide real-time notifications, but that is acceptable as the underlying file changes at most once per day.
  2. For the handling of error from net.ParseCIDR - it should as it can be user input. The code was copied from geoip2 and I have not changed the part of the code that was already working. (After posting this PR I also found a potential panic situation in the Event-method as the result from net.ParseIP is not validated.)
  3. Not sure why ip2location 8.3 is imported, i was planning on using v9.

Let me know what way to go with filewatcher vs fsnotify. I will then address all the issues above. I will also make the changes back into geoip2 at a later point.

tsaikd commented 3 years ago

For the FileWatcher, my opinion is not try to make a new wheel, but it's not mandatory. It's not an easy task if you need to consider running the codes in different platforms. In your case, integrating should be simple. Just create a background go-routine at init handler function, and write the relevant callback function for detecting files changed. Anyway, it's fine to write your own tools if the other library interface is not good for you.

helgeolav commented 3 years ago

Thanks for your feedback. It took some time to process it all. I think I got all the changes into this (updated) PR.

I moved to fsnotify as I see it is already referenced in the project. As fsnotify behaves differently on different platforms I added a timer to wait for changes to be completed. (On Windows and Mac I get more than one notification when a file is updated.)