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.

Sign up for GitHub

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

Jump to bottom

Add support for MaxMind GeoIP2 Enterprise and Anonymous-IP databases #223

Merged
merged 6 commits into from
May 22, 2024
Merged

Add support for MaxMind GeoIP2 Enterprise and Anonymous-IP databases #223

merged 6 commits into from
May 22, 2024

Conversation

edmocosta
Copy link
Contributor

@edmocosta edmocosta commented May 15, 2024 •
edited
Loading

  • Added support for MaxMind GeoIP2 Enterprise and Anonymous-IP databases.
  • Changed the database type detection to use the same strategy as com.maxmind.geoip2.DatabaseReader#getDatabaseType, so it doesn't rely on specific database types.
  • Updated MaxMind dependencies.
  • Added unit tests for the Java classes, including the verification of the plugin's basic operations against all supported databases.
  • The gem version is now read from the VERSION file.

Closes: #153
Closes: #212

@edmocosta edmocosta changed the title [WIP] Add support for MaxMind GeoIP2 Enterprise and Anonymous-IP databases Add support for MaxMind GeoIP2 Enterprise and Anonymous-IP databases May 15, 2024
@edmocosta edmocosta marked this pull request as ready for review May 15, 2024 16:56
kaisecheng
kaisecheng reviewed May 16, 2024
View reviewed changes
Copy link
Contributor

@kaisecheng kaisecheng left a comment

Choose a reason for hiding this comment

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

woohoo! A lot good stuffs in this PR. Thanks for adding test databases 🎉 , fixing the missing close() and updating many legacy parts in this plugin.

My comments are all minor about log and naming. The two new databases run as expected

lib/logstash/filters/geoip.rb Show resolved Hide resolved
src/main/java/org/logstash/filters/geoip/Databases.java Outdated Show resolved Hide resolved
src/main/java/org/logstash/filters/geoip/Databases.java Outdated Show resolved Hide resolved
src/main/java/org/logstash/filters/geoip/Databases.java Outdated Show resolved Hide resolved
src/main/java/org/logstash/filters/geoip/Fields.java Outdated Show resolved Hide resolved
@edmocosta edmocosta force-pushed the add-enterprise-anonymous-ip-dbs-support branch from 29426f0 to 5b1a400 Compare May 21, 2024 15:26
@edmocosta edmocosta requested a review from kaisecheng May 22, 2024 09:50
kaisecheng
kaisecheng approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@kaisecheng kaisecheng left a comment

Choose a reason for hiding this comment

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

🚀 LGTM. good catch on supporting jvm 8.

@edmocosta edmocosta force-pushed the add-enterprise-anonymous-ip-dbs-support branch from bb6a48e to 9eac8ed Compare May 22, 2024 11:28
@edmocosta edmocosta merged commit a53f34b into logstash-plugins:main May 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaisecheng kaisecheng kaisecheng approved these changes

@yaauie yaauie Awaiting requested review from yaauie

Assignees

@edmocosta edmocosta

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@edmocosta @kaisecheng @roaksoax