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.

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

[Discuss] Upstream geoip database changes affecting tests #25159

Closed
andrewkroh opened this issue Apr 19, 2021 · 4 comments · Fixed by #25248
Closed

[Discuss] Upstream geoip database changes affecting tests #25159

andrewkroh opened this issue Apr 19, 2021 · 4 comments · Fixed by #25248
Labels
discuss Issue needs further discussion. Team:Elastic-Agent Label for the Agent team Team:Integrations Label for the Integrations team

Comments

@andrewkroh
Copy link
Member

Changes in the Elasticsearch geoip database have always affected golden file tests in Beats. When the database changes and an IP address moves to a new location our golden file tests break (there are several reasons the tests could break).

Historically we dealt with this by updating the golden file tests with enrichments based on the new geoip database version. This hasn't been too much of a hassle because the geoip database wasn't updated since 2019, but now that burden will grew because of enhancements to Elasticsearch (elastic/elasticsearch#68920) enable the database to automatically update itself.

I see two possible solutions:

  1. Disable database updates via an Elasticsearch property ingest.geoip.downloader.enabled: false. This will cause Elasticsearch to use the 2019 database version that is embedded. This would work for now, but the embedded database will be removed in 8.x IIUC. We might be able to request that infra host a test geoip database that is static, and then we can configure our test ES instances to use the static database URL (idea suggest by @jakelandis).

  2. Update test code to ignore geo IP fields (like add them to

    # source path and agent.version can be different for each run
    other_keys = ["log.file.path", "agent.version"]
    ). The downside here is that we might not catch configuration issues with geoip processors in pipelines.

@andrewkroh andrewkroh added the discuss Issue needs further discussion. label Apr 19, 2021
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 19, 2021
@andrewkroh andrewkroh added Team:Elastic-Agent Label for the Agent team Team:Integrations Label for the Integrations team Team:Security-External Integrations labels Apr 20, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/agent (Team:Agent)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Apr 20, 2021
@leehinman
Copy link
Contributor

I like Option 2 if we can test to make sure the fields are present, but we don't compare the values. This should let us know if the geo processor adds or deletes fields.

We would then be trusting the tests in elastic/elasticsearch#68920 to let us know that lookups work.

andrewkroh added a commit to andrewkroh/beats that referenced this issue Apr 22, 2021
Disable database updates via an Elasticsearch property `ingest.geoip.downloader.enabled: false`. This will cause Elasticsearch to use the circa 2019 maxmind database version that is embedded. This would work for now, but the embedded database will be removed in 8.x IIUC.

This prevents our golden file tests from failing everytime the database changes geo or asn values. It also prevents race conditions in tests that might run before the database has been updated or in cases where the downloader service is unavailable.

Closes: elastic#25159
andrewkroh added a commit to andrewkroh/beats that referenced this issue Apr 23, 2021
Disable database updates via an Elasticsearch property ingest.geoip.downloader.enabled: false. This will cause Elasticsearch to use the circa 2019 maxmind database version that is embedded. This would work for now, but the embedded database will be removed in 8.x IIUC.

This prevents our golden file tests from failing everytime the database changes geo or asn values. It also prevents race conditions in tests that might run before the database has been updated or in cases where the downloader service is unavailable.

Closes: elastic#25159
andrewkroh added a commit that referenced this issue Apr 23, 2021
Disable database updates via an Elasticsearch property `ingest.geoip.downloader.enabled: false`. This will cause Elasticsearch to use the circa 2019 maxmind database version that is embedded. This would work for now, but the embedded database will be removed in 8.x IIUC.

This prevents our golden file tests from failing everytime the database changes geo or asn values. It also prevents race conditions in tests that might run before the database has been updated or in cases where the downloader service is unavailable.

Closes: #25159
andrewkroh added a commit that referenced this issue Apr 23, 2021
Disable database updates via an Elasticsearch property ingest.geoip.downloader.enabled: false. This will cause Elasticsearch to use the circa 2019 maxmind database version that is embedded. This would work for now, but the embedded database will be removed in 8.x IIUC.

This prevents our golden file tests from failing everytime the database changes geo or asn values. It also prevents race conditions in tests that might run before the database has been updated or in cases where the downloader service is unavailable.

Closes: #25159
andrewkroh added a commit to andrewkroh/beats that referenced this issue Apr 23, 2021
Disable database updates via an Elasticsearch property ingest.geoip.downloader.enabled: false. This will cause Elasticsearch to use the circa 2019 maxmind database version that is embedded. This would work for now, but the embedded database will be removed in 8.x IIUC.

This prevents our golden file tests from failing everytime the database changes geo or asn values. It also prevents race conditions in tests that might run before the database has been updated or in cases where the downloader service is unavailable.

Closes: elastic#25159
(cherry picked from commit 3b4b10e)
andrewkroh added a commit that referenced this issue Apr 23, 2021
Disable database updates via an Elasticsearch property ingest.geoip.downloader.enabled: false. This will cause Elasticsearch to use the circa 2019 maxmind database version that is embedded. This would work for now, but the embedded database will be removed in 8.x IIUC.

This prevents our golden file tests from failing everytime the database changes geo or asn values. It also prevents race conditions in tests that might run before the database has been updated or in cases where the downloader service is unavailable.

Closes: #25159
(cherry picked from commit 3b4b10e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issue needs further discussion. Team:Elastic-Agent Label for the Agent team Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants