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

Update GeoIP processor documentation #71211

Merged
merged 14 commits into from
Apr 15, 2021
Merged

Update GeoIP processor documentation #71211

merged 14 commits into from
Apr 15, 2021

Conversation

probakowski
Copy link
Contributor

@probakowski probakowski commented Apr 1, 2021 •
edited
Loading

This PR adds documentation for GeoIPv2 auto-update feature.
It also changes related settings names from geoip.downloader.* to ingest.geoip.downloader to have the same convention as current setting.

Relates to #68920

@probakowski probakowski added >docs General docs changes :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP v8.0.0 v7.13.0 labels Apr 1, 2021
@probakowski probakowski requested a review from martijnvg April 1, 2021 23:38
@elasticmachine elasticmachine added Team:Docs Meta label for docs team Team:Data Management Meta label for data/management team labels Apr 1, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@probakowski probakowski mentioned this pull request Apr 1, 2021
Closed
15 tasks
@probakowski
Copy link
Contributor Author

@elasticmachine update branch

martijnvg
martijnvg reviewed Apr 7, 2021
View reviewed changes
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

Thanks @probakowski!

Maybe we should also explain how databases are selected after the section about using custom databases. (first managed databases, then if missing from config databases and if missing then default shipped databases)

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpDownloader.java
@@ -60,9 +60,9 @@

public static final boolean GEOIP_V2_FEATURE_FLAG_ENABLED = "true".equals(System.getProperty("es.geoip_v2_feature_flag_enabled"));

public static final Setting<TimeValue> POLL_INTERVAL_SETTING = Setting.timeSetting("geoip.downloader.poll.interval",
public static final Setting<TimeValue> POLL_INTERVAL_SETTING = Setting.timeSetting("ingest.geoip.downloader.poll.interval",
Copy link
Member

Choose a reason for hiding this comment

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

👍

docs/reference/ingest/processors/geoip.asciidoc Outdated
How often there will be a check for new databases. Defaults to `3d`, minimum is `1d`

[[ingest-geoip-air-gapped]]
==== Updating GeoIP databases in air-gapped environments
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should both bundle file based updating of databases and using proxy/geoip tool in one section called: 'using custom databases'? Which is for use cases when custom databases are used or Elasticsearch is used in air-gapped environments. This will have 2 sections, one for updating databases via config directory (which replaces the third paragraph of geoip page) and one for setting up a http proxy.

Then the third paragraph in the geoip page can removed, which isn't that important for most folks, since they will be using the Elastic managed databases.

docs/reference/ingest/processors/geoip.asciidoc Outdated
@@ -11,12 +11,15 @@ IPv6 addresses.
The `ingest-geoip` module ships by default with the GeoLite2 City, GeoLite2 Country and GeoLite2 ASN GeoIP2 databases from Maxmind made available
Copy link
Member

Choose a reason for hiding this comment

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

Maybe change this into: The geoip processor runs by default with the GeoLite2 City, GeoLite2 Country and GeoLite2 ASN GeoIP2 databases automatically managed by Elastic from Maxmind under the CCA-ShareAlike 4.0 license.

@martijnvg
Copy link
Member

After we have updated the geoip.asciidoc we should also add a page for the geoip stats api (see #71402).

@jrodewig
Copy link
Contributor

jrodewig commented Apr 7, 2021

After we have updated the geoip.asciidoc we should also add a page for the geoip stats api (see #71402).

Thanks @martijnvg! I'm working on a PR to document that API now. I should have something up shortly.

@jrodewig jrodewig self-requested a review April 7, 2021 14:35
jrodewig
jrodewig reviewed Apr 7, 2021
View reviewed changes
Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

@probakowski @martijnvg I pushed a commit with some edits to these docs. That commit also includes docs for the GeoIP stats API. Please feel free to leave feedback or make direct changes.

docs/reference/ingest/apis/geoip-stats-api.asciidoc

`total_download_time`::
(integer)
Total milliseconds spent downloading databases.
Copy link
Contributor

Choose a reason for hiding this comment

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

Total milliseconds spent downloading databases.

@probakowski Can you confirm this is milliseconds? It looks like it in the source, but I had a hard time teasing it out. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is in milliseconds.

docs/reference/ingest/processors/geoip.asciidoc
@@ -4,21 +4,20 @@
<titleabbrev>GeoIP</titleabbrev>
++++

The `geoip` processor adds information about the geographical location of IP addresses, based on data from the Maxmind databases.
This processor adds this information by default under the `geoip` field. The `geoip` processor can resolve both IPv4 and
Copy link
Contributor

Choose a reason for hiding this comment

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

I removed the sentence about the geoip field. This is covered in the parameter docs below.

@jrodewig
Copy link
Contributor

jrodewig commented Apr 7, 2021

@elasticmachine update branch

@probakowski
Copy link
Contributor Author

@elasticmachine update branch

@probakowski
Copy link
Contributor Author

Thank you very much for the changes @jrodewig, they look great!
@martijnvg do you think we are good to merge here?

@probakowski probakowski requested a review from martijnvg April 8, 2021 21:28
martijnvg
martijnvg approved these changes Apr 11, 2021
View reviewed changes
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM

@probakowski probakowski merged commit 308aee2 into elastic:master Apr 15, 2021
@probakowski probakowski deleted the geoip-docs branch April 15, 2021 11:47
@probakowski probakowski mentioned this pull request Apr 15, 2021
Merged
probakowski added a commit to probakowski/elasticsearch that referenced this pull request Apr 15, 2021
@probakowski @elasticmachine @jrodewig
Update GeoIP processor documentation (elastic#71211)
a318fb7 
This PR adds documentation for GeoIPv2 auto-update feature.
It also changes related settings names from geoip.downloader.* to ingest.geoip.downloader to have the same convention as current setting.

Relates to elastic#68920

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: James Rodewig <[email protected]>
probakowski added a commit that referenced this pull request Apr 15, 2021
@probakowski @elasticmachine @jrodewig
[7.x] Update GeoIP processor documentation (#71211) (#71757)
d7f7cbb 
* Update GeoIP processor documentation (#71211)

This PR adds documentation for GeoIPv2 auto-update feature.
It also changes related settings names from geoip.downloader.* to ingest.geoip.downloader to have the same convention as current setting.

Relates to #68920

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: James Rodewig <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrodewig jrodewig jrodewig left review comments

@martijnvg martijnvg martijnvg approved these changes

Assignees
No one assigned
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >docs General docs changes Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team v7.13.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@probakowski @elasticmachine @martijnvg @jrodewig @jakelandis