API should have explicit GeoIP header

RESOLVED WONTFIX

Status

P3
normal
RESOLVED WONTFIX
5 years ago
5 years ago

People

(Reporter: basta, Assigned: mat)

Tracking

Avenir
2014-01-14
x86_64
Windows 7
Points:
---
Dependency tree / graph

Details

(Reporter)

Description

5 years ago
Right now we pull the geoip data from `API-Filter`, which isn't ideal because it doesn't give us any data if a region isn't returned.

Instead, there should be some header like `API-Geo` which contains a value like the following:

- error: GeoIP was attempted but the lookup failed.
- worldwide: No region could be detected based on the location.
- us: The user was detected to be in the US

The absence of the API-Geo header should indicate that no GeoIP lookup was performed and the client should take no action.
(Reporter)

Updated

5 years ago
Depends on: 948560

Updated

5 years ago
Priority: -- → P3
(Assignee)

Updated

5 years ago
Assignee: nobody → mpillard
(Assignee)

Comment 1

5 years ago
The region middleware also tries to guess the region from the Accept-Language header if geoip lookup returned 'worldwide'. In that case, should we return whatever region we ended up after geoip, or just the 'worldwide' we got from geoip ?
Flags: needinfo?(mattbasta)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 2

5 years ago
Thar be dragons. Either remove that code or make it transparent. I wasn't even aware that we were still doing that in the API.
Flags: needinfo?(mattbasta)
Neither was I. Over the break, can we statsd log the frequency of incidents where we assign a region via SIM, IP, and language? It'd be interesting to see how frequently each of those happen.
(Reporter)

Comment 4

5 years ago
Over the break? Krupa won't be testing.
Commit the logging today, cherry-pick it, investigate results on 1/3.
(Assignee)

Comment 6

5 years ago
We already log this information, have been for a while.
statsd.incr('z.regions.middleware.source.url')
statsd.incr('z.regions.middleware.source.geoip')
statsd.incr('z.regions.middleware.source.accept-lang')
(Assignee)

Updated

5 years ago
Target Milestone: --- → 2014-01-14
(Assignee)

Comment 8

5 years ago
While not strictly a duplicate, bug 964524 (and the consumer pages side bug 966335) make this obsolete.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.