GeoIP2 implementation as a plugin #12699

sgiehl · 2018-04-05T20:43:12Z

As using GeoIP2 requires a schema change on log tables to be able to track ISO region codes, GeoIP2 will be included as a new plugin. This way we can avoid an automatic schema change while updating Matomo.

This plugin will be automatically enabled for new installs. Instances updating to the new version will have the plugin included disabled. Enabling the plugin will then trigger the schema change.

As long as the GeoIP2 plugin is disabled nothing will change for Geolocation configuration, tracked data and API.

As soon as the GeoIP2 plugin is enabled some stuff changes:

Autoupdating of GeoIP databases
Autoupdating feature for databases will switch to configure updates for GeoIP2 databases.
For existing installs with configured updates for GeoIP, it will still show those settings as read only. This settings will be automatically removed as soon as GeoIP2 updates are configured

Tracking
As soon as a GeoIP2 provider is chosen region codes will be stored as ISO codes. [GeoIP Legacy used FIPS codes]. To convert old log data from FIPS to ISO the command usercountry:convert-region-codes can be used. The command will convert all codes stored before the date of switching to a GeoIP2 provider.

Changes in API
Until a GeoIP2 provider is selected nothing changes and still old FIPS codes and region names will be returned.
After switching to one all old codes included in archives will be converted [FIPS => ISO] if possible and region names will be used accordingly.

Switching back from GeoIP2 to GeoIP is only possible by disabling GeoIP2 plugin. This is on purpose as it may cause problems if FIPS and ISO codes are mixed too much in the database.

replaces #12553
fixes #4487

diosmosis

Left some comments & suggestions, haven't tested locally yet.

plugins/GeoIp2/Commands/ConvertRegionCodesToIso.php

plugins/GeoIp2/GeoIP2AutoUpdater.php

diosmosis · 2018-04-06T02:37:32Z

plugins/GeoIp2/GeoIP2AutoUpdater.php

+            if (file_exists($outputPath)) {
+                unlink($outputPath);
+            }
+            unlink($path);


For debugging purposes, would we want to keep the files around so users can see what was downloaded? Maybe just the archive, not the extracted files.

Haven't spent much thoughts on that. It's currently the same as it is done in the already existing GeoIPAutoUpdater.

Any thoughts @mattab? If it is useful, I think it can be added in another PR, though, so no need to worry about it now.

Let's keep the existing GeoipAutoUpdater behavior for now 👍

plugins/GeoIp2/LocationProvider/GeoIp2.php

plugins/UserCountry/API.php

plugins/UserCountry/LocationProvider.php

tests/PHPUnit/Fixtures/ManyVisitsWithGeoIP.php

diosmosis · 2018-04-06T20:30:20Z

Noticed there's a bug in pear/archive_tar which piwik/decompress uses which causes a fatal error when downloading databases on PHP 7: https://pear.php.net/bugs/bug.php?id=21218

I think matomo/decompress uses the fixed version.

EDIT: one way to fix this is to run composer require pear/archive_tar locally.

diosmosis · 2018-04-06T21:44:55Z

@mattab tested locally all features seem to work, except unarchiving on PHP 7. Leaving up to you whether my remaining comments should be addressed.

diosmosis · 2018-04-08T20:36:22Z

@sgiehl Looked again, looks good to me except for php 7 archive tar fatal error (see my comment above for more info).

sgiehl · 2018-04-08T20:40:52Z

I read it. But that seems not a "bug" related to the changes for GeoIP2, right? It should currently occur for GeoIP Legacy as well I guess.
Maybe we could create a issue to fix that later. I'm not able to handle that the next days, if you see an easy solution feel free to fix it ;)

diosmosis · 2018-04-08T21:07:19Z

Easiest fix is to run composer require pear/archive_tar locally, I'll do this later today.

diosmosis · 2018-04-09T01:04:51Z

PHP 7 bug no longer exists after 8fbe252, the offending method is no longer called. Moving back to extractInString fixed this.

fengkaijia · 2018-04-13T10:05:12Z

plugins/UserCountry/API.php

                $dt->filter('GroupBy', array(
                    'label',
-                    function ($label) {
-                        if ($label == '1|ti') {
-                            return '14|cn';


I guess this hard-coded conversion here and start from line 172 is still required for the old format.

True. My original implementation converted all codes so it wasn't needed anymore, but with this implementation it's still needed. Will add it again

sgiehl · 2018-04-23T16:15:06Z

I've now tested the maps with geoip2 changes_

Realtime map should work as before, as it doesn't use any region data.

The other maps will lack some information on region level. (Country and City level will still work as before) All countries where the new iso codes do not match any old fips codes the map will show those visits as "unknown region". (Note: US and Canada are not affected, as they used iso codes before, also some countries where the codes didn't change between fips and iso will still show up correctly).

To have working maps on region level with geoip2 we need to create completely new maps that also show correct regions (based in ISO) and then force all location providers to use ISO region codes in order to show region data on the map

plugins/GeoIp2/Commands/ConvertRegionCodesToIso.php

diosmosis · 2018-05-02T05:28:20Z

Did another code review, left two comments on the conversion command, will do another local test tomorrow.

…n codes still need to be converted to iso

…e quite fast and this should not be an issue in theory

* require geoip2/geoip2 composer package * Determine region name based on Location Provider * Adds empty GeoIp2 plugin * move location_region column definition to GeoIp2 plugin so it's get changed as soon as the plugin is activated * Adds GeoIP2 location providers * ignore GeoIP2 mmdb files * Adds script to generate GeoIP2 test databases * Adds Command to convert region codes from FIPS to ISO for old log table records * Adds GeoIP2 AutoUpdater * Use GeoIP2 in tests * update test files * code fixes * adds tests * rename old GeoIP providers to Legacy * Let GeoIP autoupdater UI handle GeoIp2 as well * convert region codes to ISO in API after switch to GeoIP2 * do not show GeoIP providers if GeoIP2 plugin is enabled an no GeoIP Legacy provider is still in use * small fixes * review changes * Use correct region translations * Show correct message if no database can be found * if log tables have been converted, use archive date to check if region codes still need to be converted to iso * fix tests * Improves extracting GeoIP2 databases * Adjust GeoLocation diagnostics * readds old taiwan fixes * Assume all third party location providers as 'recommended' * Download database over HTTPS * remove outdated comment (see matomo-org#12411) * Remove indication that Geoip2 may be slow, since we found it should be quite fast and this should not be an issue in theory * skip detection if IP is empty & do not try convert IP to IPv4 * remove downloaded file if an error occurs while extracting * command should be runnable multiple times * use ISO codes for suggested region codes * reload after wizard success * Drop table if exists. * Fix two translation keys. * add special region handling for UK * update system test * update UI files * submodule update * update test files

sgiehl added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Apr 5, 2018

sgiehl added this to the 3.4.1 milestone Apr 5, 2018

sgiehl mentioned this pull request Apr 5, 2018

[WIP] GeoIP 2 integration #12553

Closed

19 tasks

sgiehl force-pushed the geoip2plugin branch from c2dd2ba to 5737184 Compare April 5, 2018 22:24

diosmosis reviewed Apr 6, 2018

View reviewed changes

sgiehl force-pushed the geoip2plugin branch 3 times, most recently from 20577a6 to 26df27c Compare April 6, 2018 09:22

sgiehl force-pushed the geoip2plugin branch from fe6f870 to bef1d7f Compare April 6, 2018 21:33

sgiehl force-pushed the geoip2plugin branch 2 times, most recently from cff3c4d to 8fbe252 Compare April 7, 2018 19:18

fdellwing mentioned this pull request Apr 11, 2018

Add IP2Location to recommended geolocation providers and fix a possib… #12711

Closed

fengkaijia reviewed Apr 13, 2018

View reviewed changes

sgiehl force-pushed the geoip2plugin branch 4 times, most recently from 69e64d0 to 4e3a77b Compare April 23, 2018 13:41

mattab added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Apr 23, 2018

sgiehl force-pushed the geoip2plugin branch 2 times, most recently from c8a5cc9 to 142316c Compare April 30, 2018 10:39

diosmosis reviewed May 2, 2018

View reviewed changes

plugins/GeoIp2/Commands/ConvertRegionCodesToIso.php Show resolved Hide resolved

mattab mentioned this pull request May 2, 2018

Activate DB-Ip support by default for all users #12811

Closed

Stefan Giehl and others added 22 commits May 3, 2018 23:15

review changes

d398911

Use correct region translations

84efc51

Show correct message if no database can be found

0d705d1

if log tables have been converted, use archive date to check if regio…

52a32d8

…n codes still need to be converted to iso

fix tests

90f2d8d

Improves extracting GeoIP2 databases

3cd3996

Adjust GeoLocation diagnostics

5b4ce5e

readds old taiwan fixes

1558b1a

Assume all third party location providers as 'recommended'

d0dca1e

Download database over HTTPS

c11b06d

remove outdated comment (see #12411)

0dc5343

Remove indication that Geoip2 may be slow, since we found it should b…

2009a67

…e quite fast and this should not be an issue in theory

skip detection if IP is empty & do not try convert IP to IPv4

348d19a

remove downloaded file if an error occurs while extracting

c73e0c8

command should be runnable multiple times

0ca6304

use ISO codes for suggested region codes

6a4aa0d

reload after wizard success

b6bf901

Drop table if exists.

275d79b

Fix two translation keys.

171647e

add special region handling for UK

fc8974f

update system test

3deb67d

update UI files

d1769d0

sgiehl added a commit to matomo-org/plugin-CustomDimensions that referenced this pull request May 3, 2018

updates tests for matomo-org/matomo#12699

52251a9

submodule update

2f95d41

sgiehl force-pushed the geoip2plugin branch from e76b737 to 2f95d41 Compare May 3, 2018 21:17

update test files

ad9bb99

mattab merged commit 18ad4f7 into 3.x-dev May 4, 2018

mattab deleted the geoip2plugin branch May 4, 2018 05:15

sgiehl mentioned this pull request Aug 5, 2018

Updates omnifixture #13249

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GeoIP2 implementation as a plugin #12699

GeoIP2 implementation as a plugin #12699

sgiehl commented Apr 5, 2018 •

edited

diosmosis left a comment

diosmosis Apr 6, 2018

sgiehl Apr 6, 2018

diosmosis Apr 6, 2018

mattab Apr 7, 2018

diosmosis commented Apr 6, 2018 •

edited

diosmosis commented Apr 6, 2018

diosmosis commented Apr 8, 2018

sgiehl commented Apr 8, 2018

diosmosis commented Apr 8, 2018 •

edited

diosmosis commented Apr 9, 2018

fengkaijia Apr 13, 2018

sgiehl Apr 21, 2018

sgiehl commented Apr 23, 2018

diosmosis commented May 2, 2018

GeoIP2 implementation as a plugin #12699

GeoIP2 implementation as a plugin #12699

Conversation

sgiehl commented Apr 5, 2018 • edited

diosmosis left a comment

Choose a reason for hiding this comment

diosmosis Apr 6, 2018

Choose a reason for hiding this comment

sgiehl Apr 6, 2018

Choose a reason for hiding this comment

diosmosis Apr 6, 2018

Choose a reason for hiding this comment

mattab Apr 7, 2018

Choose a reason for hiding this comment

diosmosis commented Apr 6, 2018 • edited

diosmosis commented Apr 6, 2018

diosmosis commented Apr 8, 2018

sgiehl commented Apr 8, 2018

diosmosis commented Apr 8, 2018 • edited

diosmosis commented Apr 9, 2018

fengkaijia Apr 13, 2018

Choose a reason for hiding this comment

sgiehl Apr 21, 2018

Choose a reason for hiding this comment

sgiehl commented Apr 23, 2018

diosmosis commented May 2, 2018

sgiehl commented Apr 5, 2018 •

edited

diosmosis commented Apr 6, 2018 •

edited

diosmosis commented Apr 8, 2018 •

edited