logstash-plugins / logstash-filter-geoip

Apache License 2.0
64 stars 82 forks source link

[Doc] GeoIP database service #177

Closed kaisecheng closed 3 years ago

kaisecheng commented 3 years ago

This PR explains GeoIP database service auto-update feature

Related issues https://github.com/elastic/logstash/issues/12560

karenzone commented 3 years ago

Hi Kaise. You did a nice job describing the licensing changes and behaviors. Thank you.

I'm looking at the database option and considering how it impacts the database behavior. It's a bit confusing because we say there's no default setting, but yet we talk about default behavior.

I considered something like this:

  * Value type is <<path,path>>
  * There is no default value for this setting.
  * If not specified, the database defaults to the GeoLite2 City database that ships
  with Logstash.

Could we add a default setting that preserves the default behavior? I see it's expecting a path, so that might not work.

If there's not a more user-friendly solution, perhaps we could just delete the line that says, "There is no default value for this setting" and add the line "If not specified, the database defaults to the GeoLite2 City database that ships with Logstash."

I have some other minor wording suggestions to share, too. But it seems to make sense to resolve this first. Please let me know what you think.

kaisecheng commented 3 years ago

good catch on the default setting and default behavior. database is a path that varies on different systems, so you are right, there is no easy expression to show the default value. So, I took your suggestion.