Change everything.me endpoint for 2.0 and above

VERIFIED FIXED in 2.2 S7 (6mar)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: pdol, Assigned: kgrandon)

Tracking

({verifyme})

unspecified
2.2 S7 (6mar)
x86
macOS
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.1S+, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.1S verified, b2g-v2.2 fixed, b2g-master fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(8 attachments)

Reporter

Description

4 years ago
Currently, 2.0 and above devices point directly to an Everything.me endpoint.  As we transition their services over to Mozilla, changing that endpoint for 2.0 and above to a Mozilla controlled one would mean we can avoid relying on a traffic redirection solution from Everything.me in the future.

The endpoint should be updated from api.everything.me to appsearch.services.mozilla.com.

Until Mozilla hosts the new service, the server will actually redirect to api.everything.me.  

NOTE: Before this is landed, it needs to be tested to make sure the endpoint redirection is working like expected.
Assignee

Comment 1

4 years ago
I'll submit a pull request for each branch (2.0, 2.1, 2.2, and 3.0), and we can test each one if desired before landing.
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Whiteboard: [systemsfe]
Reporter

Comment 6

4 years ago
I'm marking this as blocking because of the risk to the product of not making the change.  It is understood that it won't get to all users, but some is better than none.
Bhavana and I had an offline conversation about blocking.
blocking-b2g: --- → 2.0+
Assignee

Comment 7

4 years ago
I've attached a few pull requests here which update this endpoint. The endpoint seems to work in the browser, but I'm getting some weird responses when interacting with the API on a device. I'm seeing the following response: {"errorCode":-9,"processingTime":0.71,"errorString":"Security Authentication denied"} 

Peter - My assumption is that this is coming back from the snapshot server. I think I probably need to reach out to some of the E.me guys. Are they working with us on bugzilla or should I talk to them offline?
Flags: needinfo?(pdolanjski)
Assignee

Comment 8

4 years ago
I tried looking for this error code within the dump of the snapshot server I have from Jan 6th, but could not find the string. I'm assuming that I either don't have the most up-to-date snapshot codebase, or that e.me is running something different.
Reporter

Comment 9

4 years ago
(In reply to Kevin Grandon :kgrandon from comment #7)
> Peter - My assumption is that this is coming back from the snapshot server.
> I think I probably need to reach out to some of the E.me guys. Are they
> working with us on bugzilla or should I talk to them offline?

+NI on Ran, we can definitely communicate through the bug
Flags: needinfo?(pdolanjski) → needinfo?(ran)
It looks like it's being redirected to api.everything.me (live server) instead of fxos-stg.everything.me (snapshot server).
Flags: needinfo?(ran)
Assignee

Comment 11

4 years ago
Peter - it seems like the API redirection is incorrect. Who can investigate this for us?
Flags: needinfo?(pdolanjski)
Reporter

Comment 12

4 years ago
Sorry for the confusion guys.  This SHOULD be redirecting to the live server for now since once verified we'll ask partners to take these patches.  By the time they apply them, I expect that api.everything.me will actually be redirecting to the snapshot server.  If they change it before that, we want it going against the live server.  Does that make sense?
Flags: needinfo?(ran)
Flags: needinfo?(pdolanjski)
Flags: needinfo?(kgrandon)
Assignee

Comment 13

4 years ago
Peter - this works, but it also means that we can't land the patches due to the authentication failures. We need to wait until the redirection is done to land it. I think this is generally ok, but it means that if some devices ship before we uplift this, then they might not get the patch. I'm not sure what kind of release schedule we're looking at coming up though.

If this can wait to land until the redirection is done, then we should be fine here.
Flags: needinfo?(kgrandon) → needinfo?(pdolanjski)
Reporter

Comment 14

4 years ago
Why are there authentication errors on the live server but not the snapshot?
Ran, I don't know how you'll be redirecting traffic once the snapshot is live.  Will we hit this same issue at that point?
Flags: needinfo?(pdolanjski) → needinfo?(kgrandon)
The live server (for 2.x) has only one auth requirement - apiKey parameter. The snapshot server has none.

I tested your patch and found that your redirection omits the original query string parameters from the http request, leading to missing apiKey param, and therefore an auth error.
Flags: needinfo?(ran)
Assignee

Comment 16

4 years ago
Seems like we need to update our redirect to include the apiKey, or wait until the transition is done.
Flags: needinfo?(kgrandon)
(All querystring params are omitted by the redirection and all are crucial)
Reporter

Comment 18

4 years ago
(In reply to Kevin Grandon :kgrandon from comment #16)
> Seems like we need to update our redirect to include the apiKey, or wait
> until the transition is done.

If we can do this before the transition that would be ideal since there are devices that are launching regularly, so the fewer that need to go through the redirection back to Mozilla (after the transition) the better.
Assignee

Comment 19

4 years ago
(In reply to Peter Dolanjski [:pdol] from comment #18)
> If we can do this before the transition that would be ideal since there are
> devices that are launching regularly, so the fewer that need to go through
> the redirection back to Mozilla (after the transition) the better.

We certainly can, but I guess whoever wrote the redirect needs to do that. I'm not sure who that is - peter can you follow-up with them?
Reporter

Comment 20

4 years ago
Travis, can your team look into why the redirect is omitting the original query string parameters from the http request?
Flags: needinfo?(tblow)
Assignee

Updated

4 years ago
See Also: → 1134250
Assignee: kgrandon → jlaz
Flags: needinfo?(tblow)
Assignee

Comment 21

4 years ago
We may want to create a new bug for the querystring issue (or just throw this bug back at me once it's addressed). Thanks!
The redirect should now preserve the original http request's query string parameters.  Kevin, please feel free to test this and let me know if you have any issues
Assignee: jlaz → kgrandon
Assignee

Comment 23

4 years ago
I seem to be seeing some kind of connection reset error when trying to access the redirect. It's also not working in a browser. The URL I'm trying to reach:

https://appsearch.services.mozilla.com/partners/1.0/Apps/search/?limit=24&categoryId=207&iconFormat=20&apiKey=79011a035b40ef3d7baeabc8f85b862f&deviceId=fxos-app://collection.gaiamobile.org/60dfd603-5658-46ce-bb15-1b8a7e3370e3&ctx={"lc":"en-US","tz":"-5","v":"3.0.0.0-prerelease","dn":"flame","ct":"wifi","sr":"480x853.5"}& 

Justin - any ideas?
Flags: needinfo?(jlaz)
Assignee

Comment 24

4 years ago
Seems the redirect is back up now. Unfortunately we are still seeing the security error when redirecting to the live server :(

{"errorCode":-9,"processingTime":0.8,"errorString":"Security Authentication denied"}
Flags: needinfo?(jlaz)
I assume it's because the params are actually sent via POST not GET in the client :/
Assignee

Comment 26

4 years ago
Thanks for the input Ran, that sounds very likely. It sounds like we need to wait to get a staging server set up somewhere that we can use if we can't wait to land this until the cutover date.
Greetings Kevin,

Can we try to test again against appsearch.services.mozilla.com?  We've configured a nginx proxy_pass backend to handle the redirect while hopefully preserving the POST data.  Thanks!
Assignee

Comment 28

4 years ago
Woohoo! This is working nicely after the changes, thank you!

Peter - do you think we're good to land this? Do we need any QA signoff?
Flags: needinfo?(pdolanjski)
Assignee

Comment 29

4 years ago
Comment on attachment 8559373 [details] [review]
Github pull request to 3.0

Dale - could you give this a review when you have a minute? Thanks!
Attachment #8559373 - Flags: review?(dale)
Reporter

Comment 30

4 years ago
(In reply to Kevin Grandon :kgrandon from comment #28)
> Woohoo! This is working nicely after the changes, thank you!
> 
> Peter - do you think we're good to land this? Do we need any QA signoff?

Please don't land it until QA verifies it.
Naoki, can you verify this next week?
Flags: needinfo?(pdolanjski) → needinfo?(nhirata.bugzilla)
If e.me does the switch over to the stage server; wouldn't it be a redirect of a redirect?  I haven't tested that scenario, nor can I until next week.

I'll have to add this portion as part of my testing on monday as e.me makes the switch over.  I assume this is the reason why you suggested that I test next week instead of today.  Is that correct?
Flags: needinfo?(pdolanjski)
Kevin, I did a search on everything.me on the repo:
https://github.com/mozilla-b2g/gaia/search?utf8=✓&q=everything.me&type=Code
https://github.com/mozilla-b2g/gaia/search?p=3&q=everythingme&type=Code&utf8=✓

I saw that
apps/homescreen/everything.me/config/config.js  has a few refs to the server, specifically the images.
Is that ok?
Flags: needinfo?(kgrandon)
Assignee

Comment 33

4 years ago
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #32)
> I saw that
> apps/homescreen/everything.me/config/config.js  has a few refs to the
> server, specifically the images.
> Is that ok?

In v2.0 and above we do not use the legacy home screen, so it should not be needed. This code would probably get updated in bug 1134250, which could land on master if we wanted.
Flags: needinfo?(kgrandon)
Assignee

Comment 34

4 years ago
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #31)
> If e.me does the switch over to the stage server; wouldn't it be a redirect
> of a redirect?  I haven't tested that scenario, nor can I until next week.
> 
> I'll have to add this portion as part of my testing on monday as e.me makes
> the switch over.  I assume this is the reason why you suggested that I test
> next week instead of today.  Is that correct?

Yes, but generally a double redirect would be fine. If redirects A -> C and B -> C are working, it's pretty unlikely that A -> B -> C would not work.
Comment on attachment 8559373 [details] [review]
Github pull request to 3.0

Yup looks good cheers
Attachment #8559373 - Flags: review?(dale) → review+
Reporter

Comment 36

4 years ago
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #31)
> If e.me does the switch over to the stage server; wouldn't it be a redirect
> of a redirect?  I haven't tested that scenario, nor can I until next week.
> 
> I'll have to add this portion as part of my testing on monday as e.me makes
> the switch over.  I assume this is the reason why you suggested that I test
> next week instead of today.  Is that correct?

Exactly.  Now that the snapshot server is live, this test should give you the same results as without the patch.
Flags: needinfo?(pdolanjski)
Assignee

Comment 37

4 years ago
Comment on attachment 8559373 [details] [review]
Github pull request to 3.0

Naoki - flagging you for qa-approval here. Please set the flag whenever you think this is ready. Thanks!
Attachment #8559373 - Flags: qa-approval?(nhirata.bugzilla)
Assignee

Comment 38

4 years ago
Comment on attachment 8559374 [details] [review]
Github pull request to 2.2

These are just uplifts so carrying the review.
Attachment #8559374 - Flags: review+
Assignee

Updated

4 years ago
Attachment #8559375 - Flags: review+
Assignee

Updated

4 years ago
Attachment #8559376 - Flags: review+
Assignee

Comment 39

4 years ago
Ok, going to go ahead and get this into master as we've changed the implementation in master to remove web results from the search app and collections appear to be working. We can wait for the qa verification if needed before the uplift.

In master: https://github.com/mozilla-b2g/gaia/commit/7894b929f1b0394f3c997f72a6482bc7813e758d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Assignee

Comment 40

4 years ago
Comment on attachment 8559376 [details] [review]
Github pull request to 2.0

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Everything.me search migration.
[User impact] if declined: Maybe the thing will stop working?
[Testing completed]: Manual testing (and existing tests passing)
[Risk to taking this patch] (and alternatives if risky): Low risk, just changing the endpoint.
[String changes made]: None.
Attachment #8559376 - Flags: approval-gaia-v2.0?(bbajaj)
The only thing I see is a slight performance degredation, which I'm not sure if it's the rendering of the new style or the redirects.

The other thing I noticed is that the search is based on language of the device versus the geolocation, which I guess is ok and doesn't have to do with this patch.

The other thing I also noticed is that Yahoo is not showing the Japanese version of the website.  Duckduckgo, google, and Bing show the Japanese version.  This also doesn't have to do with this patch.
Flags: needinfo?(nhirata.bugzilla)
I spoke too soon; I found that all the icons are generic for the games collection.
* Testing w/ SIM data connection only, the search is slow.  It takes at least 7 seconds to show results.

* Collection only showed WW English rather than US icons.
Comment on attachment 8559373 [details] [review]
Github pull request to 3.0

I approve of the change for 3.0.  There's some performance issues with the redirect/redirect that I would like Peter to take a look at before ok'ing any other version.  If he's ok w/ it then we can proceed.

The initial Geolocation finding will also cause a slowness in the intial look at collections, showing blank icons at first.
Attachment #8559373 - Flags: qa-approval?(nhirata.bugzilla) → qa-approval+
Please see comment 44 and image in comment 42.
Is the performance issue without a SIM card acceptable?

I think the redirect/redirect causes some performance degradation.
Flags: needinfo?(pdolanjski)
Flags: needinfo?(nhirata.bugzilla)
Keywords: verifyme
We need to verify this once it lands on 2.0/2.1, hence the Ni on Naoki. Kevin does the same pull request for 2.0 apply to 2.1 as well ?
Attachment #8559376 - Flags: approval-gaia-v2.0?(bbajaj) → approval-gaia-v2.0+
Flags: needinfo?(kgrandon)
Assignee

Comment 47

4 years ago
(In reply to bhavana bajaj [:bajaj] from comment #46)
> Kevin does the same pull request for 2.0 apply to 2.1 as well ?

Yes, I'll try to uplift.

v2.0: https://github.com/mozilla-b2g/gaia/commit/8eaa7ab64bc2c6fa882d6e16766f27ed89c86f34
v2.1: https://github.com/mozilla-b2g/gaia/commit/89db88a76ae8363cd7f8e7a07d35ae24058aec36

Looks like there is a conflict in 2.2 which I'll resolve.
Flags: needinfo?(kgrandon)
Assignee

Comment 48

4 years ago
Rebased the v2.2 patch and will land once try results come in.
I looked at the code change and there's low risk in terms of any regressions.  My main concern stems from the double redirect.

Having said that, I'll take the time to verify today.
Target Milestone: --- → 2.2 S7 (6mar)
For some reason the collections won't launch when I built my own gaia build v2,2 on the mac.  Not sure why. investigating.
logcat shows:
d 'SIGNAL_POLL'
D/wpa_supplicant(  978): nl80211: survey data missing!
V/WLAN_PSA(  210): NL MSG, len[048], NL type[0x11] WNI type[0x5050] len[028]
E/HWComposer(  206): Non-uniform vsync interval: 7265406980
E/HWComposer(  206): Non-uniform vsync interval: 6556927
I/GeckoDump(  206): [system] [HomescreenWindow][Homescreen][homescreen][78262.050]  Handling mozbrowserasyncscroll event...
I/GeckoDump(  206): [system] [HierarchyManager][78262.092] mozChromeEvent
I/GeckoDump(  206): [system] [HomescreenWindow][Homescreen][homescreen][78262.147]  Handling mozbrowsermetachange event...
I/GeckoDump(  206): [system] [HomescreenWindow][Homescreen][homescreen][78262.149] publishing internal event: themecolorchange
I/GeckoDump(  206): [system] [HomescreenWindow][Homescreen][homescreen][78262.151] publishing external event: themecolorchange
I/GeckoDump(  206): [system] [HomescreenWindow][Homescreen][homescreen][78262.168]  Handling mozbrowsermetachange event...
I/GeckoDump(  206): [system] [HomescreenWindow][Homescreen][homescreen][78262.170] publishing internal event: themecolorchange
I/GeckoDump(  206): [system] [HomescreenWindow][Homescreen][homescreen][78262.170] publishing external event: themecolorchange
I/GeckoDump(  206): [system] [HomescreenWindow][Homescreen][homescreen][78262.178] publishing internal event: titlestatechanged
I/GeckoDump(  206): [system] [HomescreenWindow][Homescreen][homescreen][78262.179] publishing external event: titlestatechanged

Nothing changes on the device though.
Reverting the patch caused the collection to open on my device.  I spoke with kevin and he says it works fine on his device.  Not sure the difference.  We both had flashed a v2.2 build and then made gaia on top.  

Still investigating v2.1 and v2.0 as I don't have those branches on my machine yet.
So I checked all three versions from the builds that we have on pvt.  The latest builds should have the gaia changes and they open up the collections just fine.  I'm not sure what the difference is.  I guess it has something to do with my repo.  I'm going to troubleshoot that on the side.

I did notice performance issues when clicking on links where it will appear all white screened for several seconds ( up to 10 seconds ) before switching to the website on a wifi connection on all three versions.
I just realized using the Japan VPN, I'm getting WW again for the redirect.  The geolocation is done via IP.  Is the IP getting passed through?  103.17.199.77

The ll in the logcat is showing Tokyo for the URL passed:
E/GeckoConsole( 3295): Content JS LOG at app://collection.gaiamobile.org/gaia_build_defer_create.js:295 in log: evme API request: https://appsearch.services.mozilla.com/partners/1.0/Search/bgimage/?width=127&height=127&categoryId=349&_checksum=09177edd1dd0bcd126cea02a181bbf9a&apiKey=79011a035b40ef3d7baeabc8f85b862f&deviceId=fxos-app://collection.gaiamobile.org/bf681f71-ba4d-437c-ad8e-ca5985a8187f&ctx={"lc":"en-US","tz":"-5","v":"2.1.0.0-prerelease","dn":"flame","ct":"wifi","sr":"480x853.5","ll":"35.685,139.7514"}&
To note, currently : 
appsearch.services.mozilla.com	canonical name = appsearch.prod.mozaws.net.
Name:	appsearch.prod.mozaws.net
Address: 54.213.1.224
Name:	appsearch.prod.mozaws.net
Address: 54.213.18.52
Posted file 2.0_logcat.txt
WWE for 2.0 for a different reason, can't get geolocation address.  See logcat
Posted file 2.2_logcat.txt
2.2 also has broken geolocation.
Assignee

Comment 61

4 years ago
I thought we had disabled geolocation from 2.2 already.
Doesn't the server side still need the ip to get geolocation for the requestor?  That's what I'm referring to in regards to geolocation.  Otherwise they get the default WWE content instead of the country content.
I didn't mean the client side geolocation.
Greetings Naoki,

We've redeployed the nginx reverse proxy to us-east, as we discovered that everything.me's endpoint is actually hosted on AWS's us-east region.  Instead of the traffic having to travel from the user, to the current proxy (us-west), to its destination (us-east), then back to our proxy, the latency should go down.  

I haven't performed the DNS cutover yet so we can collect more performance data, but let me know when you are able to test via IRC
I've switched the appsearch endpoint to our US-East production stack as there is now a much more improved response time.  Naoki, please feel free to retest the endpoint to see if there are still performance issues
Flags: needinfo?(nhirata)
Hi Vincent, Hi Steven,

This is the bug which replacing e.Me and already land on 2.0M Woodduck.
Could you please help to land this on 2.1S Dolphine? Thanks!
Flags: needinfo?(vliu)

Updated

4 years ago
blocking-b2g: 2.0+ → 2.1S?

Updated

4 years ago
Blocks: Woodduck
Reporter

Updated

4 years ago
Blocks: 1142087
blocking-b2g: 2.1S? → 2.1S+

Updated

4 years ago
Flags: needinfo?(vliu)
Status: RESOLVED → VERIFIED
Reporter

Updated

4 years ago
Flags: needinfo?(pdolanjski)
Reporter

Updated

4 years ago
Group: mozilla-employee-confidential

Comment 66

4 years ago
Hi Steve, as we discussed today, could you kindly verify or plan for landing this change in 2.1S?
Flags: needinfo?(styang)
(In reply to jshen from comment #66)
> Hi Steve, as we discussed today, could you kindly verify or plan for landing
> this change in 2.1S?

the solution has been landed in 2.1S. ni? mike for the verification on 2.1S. thanks
Flags: needinfo?(styang) → needinfo?(mlien)
verified and fixed with v2.1S
Build ID               20150419161204
Gaia Revision          1f2df8c408f9cc23863ad0991459e1e47a877e57
Gaia Date              2015-04-14 21:52:01
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/6e48d0ec7755
Gecko Version          34.0
Device Name            scx15_sp7715ea
Firmware(Release)      4.4.2

Below is the part of log:
E/GeckoConsole( 1253): Content JS LOG at app://collection.gaiamobile.org/gaia_build_defer_view.js:414 in log: evme API request: https://appsearch.services.mozilla.com/partners/1.0/Apps/search/?limit=24&categoryId=207&iconFormat=20&apiKey=79011a035b40ef3d7baeabc8f85b862f&deviceId=fxos-app://collection.gaiamobile.org/cddffa30-a8bd-4f57-a5aa-76f5778168fb&ctx={"lc":"en-US","tz":"8","v":"2.1.0.0-prerelease","dn":"SP7715A","ct":"wifi","sr":"480x853.5"}&
Flags: needinfo?(mlien)
Flags: needinfo?(nhirata) → needinfo?(nhirata.bugzilla)
Flags: needinfo?(nhirata.bugzilla)
You need to log in before you can comment on or make changes to this bug.