Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Conditionally make requests to Google services based on client country detected by IP geolocation #548

Merged
merged 6 commits into from
Jun 5, 2019

Conversation

jwalgran
Copy link
Contributor

@jwalgran jwalgran commented May 29, 2019

Overview

Modifies the code that makes requests to Google services to pass the detected country code as an argument when appropriate and to switch to China-based hosts when required.

Connects #537
Connects #539

Demo

2019-05-29 16 10 25

US

IP Geolocation

Screen Shot 2019-06-03 at 1 45 15 PM

Translation

Screen Shot 2019-05-29 at 3 49 26 PM

Static Map

Screen Shot 2019-05-29 at 3 49 40 PM

Dynamic Map

Screen Shot 2019-05-29 at 3 50 16 PM

CN

IP Geolocation

Screen Shot 2019-06-03 at 1 51 37 PM

Translation

Screen Shot 2019-05-29 at 3 51 14 PM

Static Map

Screen Shot 2019-05-29 at 3 51 41 PM

Dynamic Map

Screen Shot 2019-05-29 at 3 53 17 PM

Testing Instructions

The screencast in the Demo section shows the app working both on a US network and connected to a China-hosted VPN endpoint.

The IP location screenshots were taken while this branch was deployed to staging, showing the ipgeolocation.io service working correctly.

diff --git a/src/app/src/actions/clientInfo.js b/src/app/src/actions/clientInfo.js
index 10082a6..059a763 100644
--- a/src/app/src/actions/clientInfo.js
+++ b/src/app/src/actions/clientInfo.js
@@ -1,9 +1,9 @@
 import { createAction } from 'redux-act';
 
-import axios from 'axios';
+// import axios from 'axios';
 
 import {
-    makeGetClientInfoURL,
+    // makeGetClientInfoURL,
     logErrorAndDispatchFailure,
 } from '../util/util';
 
@@ -11,19 +11,20 @@ export const startFetchClientInfo = createAction('START_FETCH_CLIENT_INFO');
 export const failFetchClientInfo = createAction('FAIL_FETCH_CLIENT_INFO');
 export const completeFetchClientInfo = createAction('COMPLETE_FETCH_CLIENT_INFO');
 
-const request = axios.create({
-    // Allow for a slow connection but ensure that Google components attempt to
-    // load with default client details if the client info API request hangs
-    // indefinitely.
-    timeout: 10000,
-});
+// const request = axios.create({
+//     // Allow for a slow connection but ensure that Google components attempt to
+//     // load with default client details if the client info API request hangs
+//     // indefinitely.
+//     timeout: 10000,
+// });
 
 export function fetchClientInfo() {
     return (dispatch) => {
         dispatch(startFetchClientInfo());
 
-        return request
-            .get(makeGetClientInfoURL())
+        // return request
+        //     .get(makeGetClientInfoURL())
+        return Promise.resolve({ data: { country_code2: 'CN' } })
             .then(({ data }) => dispatch(completeFetchClientInfo(data)))
             .catch(err => dispatch(logErrorAndDispatchFailure(
                 err,
  • Refresh the facility detail page and verify that that all resources load correctly and that requests to Google resources are made to .cn domains.
  • Optionally test that IP geolocation works in development by looking up the ipgeolocation.io API key in shared credential storage, adding
REACT_APP_IPGEOLOCATION_API_KEY=abcd-KEY-HERE

to .env.backend and rerunning ./scripts/server

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@rajadain
Copy link
Contributor

Hmm. Even after making the recommended changes, I'm still seeing the main Google APIs being used:

image

@jwalgran
Copy link
Contributor Author

That's because of the timeout that appears in the console. If it takes longer that 10s to do the IP geolocation, it just assumes a default location.

@jwalgran
Copy link
Contributor Author

I am seeing gateway timeouts from the free service.

Screen Shot 2019-05-30 at 10 58 22 AM

We may need to go with a paid service.

@rajadain
Copy link
Contributor

rajadain commented Jun 3, 2019

Taking another look at this.

@rajadain
Copy link
Contributor

rajadain commented Jun 3, 2019

Hmm actually, can't review this because of merge conflicts. Will wait until they are resolved before checking this out.

@jwalgran
Copy link
Contributor Author

jwalgran commented Jun 3, 2019

I fixed the merge conflict but you will need an API key to merge this properly. I'll update the testing instruction and let everyone know when this is ready.

@jwalgran
Copy link
Contributor Author

jwalgran commented Jun 3, 2019

@open-apparel-registry/operations I originally began the implementation of detecting the brower's country using the free http://userinfo.io/ service. There was some significant downtime of that service last Thursday that prompted me to find a paid alternative.

I evaluated https://ipinfo.io/, https://ipdata.co/, and https://ipgeolocation.io

This PR has now has a successful integration of the https://ipgeolocation.io service.

I chose https://ipgeolocation.io for the following reasons:

  • Low cost: Their $15 monthly plan covers 150,000 requests. 2 orders of magnitude more than our current page view count. Additional blocks of 50,000 requests are $5.
  • A dashboard with impressive uptime statistics.
  • Support for using request origin rather than an API key, allowing us to make requests from the client without worrying about exposing a key or taking on the burden of writing our own server-side proxy. We are allowed one origin on the lowest subscription tier, but that includes subdomains so it will cover both staging and production. This PR has support for conditionally using an API key in development, but we don't really need IP geolocation so it will be optional in development.

I tested this PR with a free personal account but would like to sign up for a paid account under a company address and credit card to complete the implementation.

Next Steps:

  1. Are there any objections to going ahead with using https://ipgeolocation.io?
  2. If not I would need some help setting up the paid account. Once credentials for the account are available I can set up the request origin and finish testing the implementation on staging.

@rbreslow
Copy link
Contributor

rbreslow commented Jun 3, 2019

Are there any objections to going ahead with using https://ipgeolocation.io?

I think this looks good. The support for using request origin sounds elegant. 👍

It's worth surfacing that MaxMind seems to be an informal standard for GeoIP. For example, BIND defaults to MaxMind: https://kb.isc.org/docs/aa-01149. I also believe their GeoIP file format is baked into a couple programming languages, including PHP.

I did a brief search, and it looks like MaxMind also charges $15 per 150,000 requests ($0.0001 * 150,000), although on an credit rather than monthly basis.

See: https://www.maxmind.com/en/geoip2-precision-country-service

If not I would need some help setting up the paid account. Once credentials for the account are available I can set up the request origin and finish testing the implementation on staging.

I can wire this up and ping you on Slack.

@jwalgran jwalgran force-pushed the jcw/conditional-google-cn branch 2 times, most recently from 42f656b to d6d7c7f Compare June 3, 2019 18:51
@jwalgran
Copy link
Contributor Author

jwalgran commented Jun 3, 2019

This is now ready for another look. I updated the description with new demo screenshots and revised instructions.

Thanks, Rocky, for jumping on this and setting up an ipgeolocation.io account.

@rajadain
Copy link
Contributor

rajadain commented Jun 3, 2019

Tested with the mock clientInfo.js stuff, it's working correctly. Will test with the ipgeolocation credentials in the morning, but this is looking good!

Copy link
Contributor

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 tested, works nicely. Took a while to get the credentials in Last Pass, but this works well overall. Glad we're able to support these users.

@rajadain rajadain assigned jwalgran and unassigned rajadain Jun 5, 2019
@jwalgran
Copy link
Contributor Author

jwalgran commented Jun 5, 2019

Thanks for taking the time to exercise this.

The free userinfo.io service had some significant downtime during our testing,
prompting us to select a more reliable alternative.

We evaluated ipinfo.io, ipdata.co, and ipgeolocation.io and chose
ipgeolocation.io for the following reasons:

  - Low cost: Their $15 monthly plan covers 150,000 requests. 2 orders of
    magnitude more than our current page view count. Additional blocks of 50,000
    requests are $5.
  - A dashboard with impressive uptime statistics.
  - Support for using request origin rather than an API key, allowing us to make
    requests from the client without worrying about exposing a key or taking on
    the burden of writing our own server-side proxy. We are allowed one origin
    on the lowest subscription tier, but that includes subdomains so it will
    cover both staging and production. This commit includes support for
    conditionally using an API key in development, but we don't really need IP
    geolocation so it will be optional in development.
With this change the application now waits until a request for client
information has succeeded or failed before loading the Google map and uses the
fetched country code to set the region for the the Google map.
Loading Google translate library in China requires that we request the script
from google.cn rather than google.com. To allow this conditional loading we
moved the script tag from `index.html` into a new component that uses a
lifecycle method to append the script tag with the proper `src`.
Conditionally sets the host for the static image request based on whether or not
the client is in China and passes the detected country code to the `region`
parameter.
We do not want non-responsive requests to the client info API to block Google
related features from loading.
@jwalgran jwalgran merged commit d5b351f into develop Jun 5, 2019
@jwalgran jwalgran deleted the jcw/conditional-google-cn branch June 5, 2019 19:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants