510982 - Add-on Manager downloads not counted

Status: RESOLVED INVALID

(addons.mozilla.org Graveyard :: Statistics, defect)

Description

[Yongqian]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US) AppleWebKit/530.5 (KHTML, like Gecko) Chrome/2.0.172.39 Safari/530.5
Build Identifier: 

Using the Live HTTP headers addon, I took a look at the requests made while downloading an addon from the Addon Manager:
https://addons.mozilla.org/downloads/file/57433/xoopit_for_gmail-1.0.15809-fx.xpi ->
https://addons.mozilla.org/en-US/downloads/file/57433/xoopit_for_gmail-1.0.15809-fx.xpi ->
https://addons.mozilla.org/en-US/firefox/downloads/file/57433/xoopit_for_gmail-1.0.15809-fx.xpi ->
http://releases.mozilla.org/pub/mozilla.org/addons/8257/xoopit_for_gmail-1.0.15809-fx.xpi


Looking at the requests made when download from AMO:

https://addons.mozilla.org/en-US/firefox/downloads/latest/1865/addon-1865-latest.xpi ->
https://addons.mozilla.org/en-US/firefox/downloads/file/61695/xpi/adblock_plus-1.1.1-fx+sm+tb.xpi ->
http://releases.mozilla.org/pub/mozilla.org/addons/1865/adblock_plus-1.1.1-fx+sm+tb.xpi

As you can see, there are a few differences between the URLs loaded, such as the missing "/xpi/" when downloading from the add-on manager. I'm wondering whether it's possible that downloads though the Add-on manager are not being counted.










Reproducible: Always

Steps to Reproduce:
1.N/A
2.
3.


Expected Results:  
N/A

Comment 1

[Daniel Einspanjer [:dre] [:deinspanjer]]

Attachment: Excerpt of download traffic with IP anonymized

I asked the creator to submit this bug because when I took a look at some data to review his concern, I saw that I might indeed be throwing more requests on the floor than are necessary.

At the moment, the rules for a request to be counted as a download are:
1. Only the first request from a given IP address for a specific URL will be counted within an hour.  If the format of the URL is slightly different, this de-duping will not detect it.

2. Request must have exactly two URL parts before the "/downloads/file" identifier
    Note that this means that I am requiring a product name and a locale.  This check was put into place because the majority of requests that don't have a locale or filename in them are immediately followed by another request that fills in those missing URL components.  Without the check, we'd be double counting those requests. 
3. The next URL part after the constant "/downloads/file/" must be a series of digits representing the file ID.
    This means that the new /latest/ requests are not counted directly as downloads.  This should be okay because /latest/ redirects to the actual file ID so I will pick up the subsequent request.

If you look through the attached log excerpt, you'll see that some requests get a locale but no product name.  In those cases, I am not currently counting the request as a valid download.  For instance, the requests from the first two IP addresses in the attachment would be completely discarded because they never redirected all the way down to a "valid" URL (i.e. they never got a product name).  

Just a little further down in the file though, on line 17, there is a download that does get redirected three times to eventually get a "valid" format including the product name.  (of course, there is also an oddity that line 16 is from the same IP just a few seconds prior and it is already a complete URL format so I cannot explain why there is another request for the file a few moments later with a different URL format).

I need the AMO developers to review this and compare it to the application logic for both the Add-on Manager and the website to help me determine if there needs to be a change made to the log processing.

Comment 2

[Yongqian]

I'm wondering how is the locale to be redirected to determined? What if the locale is empty?

I see a "GET /firefox/downloads/file/36071/ie_tab-1.5.20080823-fx-win.xpi HTTP/1.1" - is that a case of the locale missing?

Comment 3

[Justin Scott [:fligtar]]

I haven't dug very deep into this, but from reading the above comments, everything seems to be correct.

According to the filters Daniel described, the Add-ons Manager download URL in the first comment would be counted. Anything after the file id shouldn't matter, as it is different for every add-on, regardless of whether it has /xpi or not.

And I don't think a download can be served if the redirect didn't complete all the way through adding a locale and application, so we shouldn't count the downloads that don't have a locale.

(In reply to comment #2)
> I'm wondering how is the locale to be redirected to determined? What if the
> locale is empty?
> 

Wil can go into more detail with this but we check a number of things such as the locale the user's browser prefers (in headers) all the way to defaulting to en-US.

Comment 4

[Daniel Einspanjer [:dre] [:deinspanjer]]

The most important thing to verify from my perspective is that it should be considered an invalid request if the series of redirects goes no further than the following:

GET /downloads/file/####/*.xpi
GET /en-US/downloads/file/####/*.xpi

i.e. they don't get a third redirect that puts a product name in there.  The ones with a locale but no product name are the ones I'm seeing in the samplings I took of the log files that I'm concerned about whether they should be counted or not.

Comment 5

[Justin Scott [:fligtar]]

All requests to /downloads should have an application name.

We let some controllers go without it such as https://addons.mozilla.org/en-US/statistics or /developers, but /downloads must always complete the redirect process to an application name.

Comment 6

[Daniel Einspanjer [:dre] [:deinspanjer]]

Perfect.  Then as far as I can tell, I'm counting the downloads as accurately as is currently possible.

Comment 7

[Daniel Einspanjer [:dre] [:deinspanjer]]

Next question.  Why do I see some requests with a response status of 304?  It doesn't seem to make sense with the standard action being a 302 redirect.
My first inclination is to just discard these requests, but I don't want to do that without understanding how they can exist.

Comment 8

[Daniel Einspanjer [:dre] [:deinspanjer]]

Moving my question in comment #7 to new bug 512528.