507408 - AUS should pass the ?force=1 flag through to bouncer

Status: RESOLVED FIXED

(AUS Graveyard :: Systems, defect)

Description

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

When a user manually checks for updates by clicking "Help | Check for updates...", the request to AUS has a flag in it, "force=1" to indicate it was a manual check.  Metrics needs this flag to also be included in the request to bouncer for the download so that we can track the difference between automatic update downloads and manually initiated downloads.

Comment 1

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

I mentioned to Morgamic that I would really like to get this patch into AUS before an automatic notification of Major Update for 3.5.2 is released.  Otherwise, we'll have no ability to properly categories those upgrades.

Comment 2

[Michael Morgan [:morgamic]]

How do you want the bouncer url to work?  Just add &force=1?

Comment 3

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

Yep, that will be perfect.

Comment 4

[Michael Morgan [:morgamic]]

Ok, will get to it early next week.

Comment 5

[Michael Morgan [:morgamic]]

Attachment: v1, passthru for force=1 and updated acceptance tests

Comment 6

[[e-mail] (:bhearsum)]

Comment on attachment 393248 [details] [diff] [review]
v1, passthru for force=1 and updated acceptance tests

This looks fine...but this is the first time I've ever seen this code, so take that FWIW!

Comment 7

[Michael Morgan [:morgamic]]

Need to add logic to support both ?force=1 and &force=1 cases.

Comment 8

[Michael Morgan [:morgamic]]

Attachment: follow-up patch, adds check for ? to make sure we don't build invalid URLs

Comment 9

[Michael Morgan [:morgamic]]

This was checked in on trunk.  Tests pass and URLs are valid.

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #8)
> Created an attachment (id=439368) [details]
> follow-up patch, adds check for ? to make sure we don't build invalid URLs

There was an additional change to Verify.txt that landed with this:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/webtools/aus/tests&command=DIFF_FRAMESET&file=Verify.txt&rev1=1.13&rev2=1.14&root=/cvsroot

And it fails for me, presumably because 
 tests/data/3/Synthetic/1.0/platform/1000000002/locale/bouncerchannel
only has a CVS dir. Perhaps a forgotten 'cvs add' ?

Comment 11

[Michael Morgan [:morgamic]]

txt files were missing -- added those to trunk.

Comment 12

[Nick Thomas [:nthomas] (UTC+12)]

A bouncer url is always going to have a ?product=foo on it so I'm wondering what lead to comment #7 and 
 http://mxr.mozilla.org/mozilla/source/webtools/aus/xml/inc/xml.class.php#114

I ask because on the beta channel we don't use bouncer and point directly at ftp.m.o. Eg: https://aus2.mozilla.org/update/3/Firefox/3.6.3/20100401064631/Darwin_Universal-gcc3/en-US/beta/Darwin%2010.3.0/default/default/update.xml?force=1
points to http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/3.6.4-candidates/build4/update/mac/en-US/firefox-3.6.4.complete.mar?force=1
That still works if I use curl, but I'd like to confirm that Apache will reliably drop the option without us needing a special config there.

Comment 13

[Michael Morgan [:morgamic]]

We could alternatively drop it for anything that isn't a bouncer URL.

Comment 14

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: Only append '&force=1' to bouncer urls

Like this perhaps. Passes the tests.

Comment 15

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

Regarding comment #12,  If we don't use bouncer for beta downloads, then that means we can't see / don't track downloads for beta channel at all.  That surprises me.  Has that always been the case?

Comment 16

[Michael Morgan [:morgamic]]

Comment on attachment 447953 [details] [diff] [review]
Only append '&force=1' to bouncer urls

This works.  Thanks Nick.

Comment 17

[Michael Morgan [:morgamic]]

Daniel - as far as I know yes.  RC's I think get distributed over bouncer, though?

Comment 18

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 447953 [details] [diff] [review]
Only append '&force=1' to bouncer urls

Landed with a slight tweak to the description of the bouncerchannel test:

Checking in tests/Verify.txt;
/cvsroot/mozilla/webtools/aus/tests/Verify.txt,v  <--  Verify.txt
new revision: 1.16; previous revision: 1.15
done
Checking in xml/inc/xml.class.php;
/cvsroot/mozilla/webtools/aus/xml/inc/xml.class.php,v  <--  xml.class.php
new revision: 1.6; previous revision: 1.5
done

Comment 19

[Nick Thomas [:nthomas] (UTC+12)]

'beta' is so overloaded, 'scuse the verbosity ...

(In reply to comment #15)
> Regarding comment #12,  If we don't use bouncer for beta downloads, then that
> means we can't see / don't track downloads for beta channel at all.  That
> surprises me.  Has that always been the case?

In the period before a new release we've pointed people on the beta update channel to ftp.m.o because we
 * don't want to have to wait for the mirrors to sync
 * don't need the mirror capacity anyway
 * felt uncomfortable about distributing pre-release builds more widely

As the count of users on beta is now around 7 figures (across all versions) we're starting to think about how to we can use the mirrors while still remaining nimble. And the network woes of the last week meant updates to 3.6.4 build6 were served via bouncer so we didn't stress the San Jose datacenter further. This is still ad-hoc, but may get adopted into the release process.

(In reply to comment #17)
> Daniel - as far as I know yes.  RC's I think get distributed over bouncer,
> though?

In general, any updates or installers that are going to cause significant traffic will be set up in bouncer. Things like 3.6RC1 definitely fall into that category. In practice that means just about everything except these beta channel updates prior to general release.

3.6.4 has been unusual, as it's the first time we've backported a major new feature to a stable branch, and the installers have been offered as '3.6.4 betas' on mozilla.com at
  http://www.mozilla.com/en-US/firefox/all-beta.html
That's via bouncer so we are counting them, the current product is Firefox-3.6.4build6.

Comment 20

[Nick Thomas [:nthomas] (UTC+12)]

Daniel, do you need to make any changes to the metrics infra before we deploy this change ?

Comment 21

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

Yes.  We should file a Metrics bug to add an attribute to the downloads data mart to track which requests have this parameter set.

Comment 22

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

That does not need to be done before this change is deployed though.

Comment 23

[Nick Thomas [:nthomas] (UTC+12)]

Tagged and deployed in bug 569160.