geoip system tests

[rubvs]

Closes #17603 & fixes #18313

This PR contains some confusion. Mostly, originating from how ES handles GeoIp pipelines. The GeoIP processor operates in two different phases:

1. When the pipelines are created (think compile time)
2. When data are ingest into the pipelines (think runtime)

If the GeoIp database is unavailable on creation, all following ingested data will be tagged, see Code

If the database was available on creation - but is currently unavailable on data ingestion (runtime) - the incoming data will only be tagged if ignore_missing = false, see Code. We have ignore_missing: true in our geoip pipeline for APM Server, see Code.

Therefore, the system tests are testing for 3 things:

1. Compile Time: The database is unavailable on start (compile time). We have to restart the container without downloading - in which case, the tags field will be present the response.
2. Runtime: Enrichment succeeded due to the GeoIp database being available (downloaded) when the container starts.
3. Runtime: Enrichment fails due to missing ip address or the database was removed after initial creation (runtime). In this case, there should be no tags, but the client.geo fields in the response should be missing.

[github-actions]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

• run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

[mergify]

This pull request does not have a backport label. Could you fix it @rubvs? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-7.17 is the label to automatically backport to the 7.17 branch.
• backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
• backport-9./d is the label to automatically backport to the 9./d branch. /d is the digit.
• backport-active-all is the label that automatically backports to all active branches.
• backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
• backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

[rubvs]

unused code. cleanup.

[kruskall]

Thanks for looking into this and digging into the flaky geoip download! 🙇

I'm afraid this has diverged significantly from what the initial ask/issue was about and increased in scope to a point which I'm unsure we should care in apm-server.

[simitt]

Here's how I understand this to work: The apm-data plugin defines download_database_on_pipeline_creation: false. This means that the geoip database is not downloaded when the apm assets are installed, but only on first actual need, so when the first document with the client.ip field set, arrives.
The question is why the behaviour is flaky when the database is not pre-downloaded. In previous conversations @rubvs indicated that this is not a timing issue, but I am not sure there is yet an alternative explanation for it.

[masseyke]

Is the problem just that it takes a little bit of time for elasticsearch to download and install the database? It does not block requests while this is happening, so some will be tagged as if no geoip database exists.

[rubvs]

Thanks @masseyke, the async parts cleared up some confusion. I have updated the system tests accordingly. @simitt the updated test will trigger a lazy download on the first request, which will then poll for the absence of tags for up to 3 minutes, while the database is being downloaded. On success, the resulting hits will be asserted with the approvals.

[rubvs]

Mmh, don't know why the fips unit tests are failing though.

[rubvs]

• Removed downloading the db on startup.
• When required, download the geoip db lazily.
• The approvals already check for tags. I tested this manually to make sure.

Moreover, from an abstraction level this PR reintroduces locality: The test that requires the geoip db, is responsible for downloading the db.

The GeoIpLazyDownload sends an event containing client.ip to trigger the download, then waits until finished. The datastreams have to cleared, since they will contain noisy tagged items.

[isaacaflores2]

Thanks for sticking with this issue and sharing the geoIP database details you learned about ElasticSearch.

It looks like test requested in #17603 were already present. Like you mentioned the approvals.json files do not contain a tags field which will assert the absence of tags given that ElasticSearch has been properly setup with the geoip databases.

[simitt]

🎉

[simitt]

@kruskall please re-review as you had requested changes, so this can be brought over the finish line