Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

METRON-680: GeoLiteDatabase incorrectly using country geoname_id instead of city #433

Closed
wants to merge 2 commits into from

Conversation

justinleet
Copy link
Contributor

Swaps out country's geoname_id for city's geoname_id. Both ids are the same in format (they're the ids for the geonames.org dataset).

The only direct effect I know of from this is the Kibana dashboard uses this field for unique locations (so changing what feeds this changes the dashboard output). The main wrinkle is that city can be unpopulated, even though a country exists (e.g. if an IP is just assigned to the U.S., it may not have a city). This isn't always true that (at least one IP range for Japan appears to fill in Japan as the city's geoname id).

I'm opening the PR with just the direct change, but it's definitely open to discussion if we want to adjust.

@cestella
Copy link
Member

cestella commented Feb 2, 2017

Can you reopen with the right JIRA prefix: METRON-680 ?
EDIT: Nevermind, evidently the ASF bot is smart enough to link the two with this name. Sorry! Retracted.

@justinleet justinleet changed the title Metron 680: GeoLiteDatabase incorrectly using country geoname_id instead of city METRON-680: GeoLiteDatabase incorrectly using country geoname_id instead of city Feb 2, 2017
@justinleet
Copy link
Contributor Author

@cestella My mistake, fixed.

@cestella
Copy link
Member

cestella commented Feb 2, 2017

I don't mind this change, but could we add another field for "countryLocId" so that, if so desired, people could reconfigure that instead?

@justinleet
Copy link
Contributor Author

@cestella We definitely could. METRON-679 has the details about all the other fields we could be passing, including the current country geoname id being passed. If we want to start passing additional fields, I'd rather just do it one as part of that ticket.

For this, we could also just change the box to do a unique count of one of the other existing fields, and just ignore locId entirely. To the best of my knowledge it only gets used in that one Kibana visualization. Depending on how granular we want the box to be (at least by default), we could always make it a unique count on city or a unique count on location_point (or whatever the actual field for lat,long) is.

@cestella
Copy link
Member

cestella commented Feb 2, 2017

Ok, sounds good to me. +1 by inspection

@asfgit asfgit closed this in ddca4d8 Feb 3, 2017
@justinleet justinleet deleted the METRON-680 branch April 4, 2017 12:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants