1146863 - Improve the partial suggestion using actual usage number

Status: RESOLVED FIXED

(Release Engineering :: Applications: Shipit, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

For now, the algorithm to suggest partial is stupid, it suggests the last versions published (2 for releases, 3 for beta).

The PR proposes to improve the results by using actual ADI.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Implement most of the feature

I have a few remaining problems:
* we need to have a vertica access to retrieve the data
I can push the json files somewhere but I don't think we want that to be public.
Anyway, having it on the same machine as ship-it run a few time for day should be the easiest solution.

* in kickoff/views/submit.py, I have hardcoded the URL to http://localhost/, not sure how we want to set this up? (running it locally would tackle this issue)

* Beta ADI are missing. I reported bug 1145178. For now, still proposing the 2 last shipped builds.

Despite these issues, guys, I would like to have your feedback on this change.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Attachment: 0001-generate-json-files-from-the-vertica-db.patch

Requested by Ben.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Attachment: 0002-Automatically-fill-the-partial-generation-field-from.patch

Comment 4

[[e-mail] (:bhearsum)]

For background, we had an e-mail conversation about this. Tl;dr is that Vertica doesn't have a JSON api, so we're going to dump its data with a Python script, make it publicly available, and Ship It's frontend will source it.

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 8582372 [details] [review]
Implement most of the feature

Feedback given in PR.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Release management has now a VM and we can publish the results here:
http://relman-vm1.dmz.scl3.mozilla.com/partial.json
(vpn access needed)
I updated the PR.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8582372 [details] [review]
Implement most of the feature

Ben, I think this is ready now!

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 8582372 [details] [review]
Implement most of the feature

I think this looks good now....I merged it.

Can you see how things are looking on https://ship-it-dev.allizom.org/ before we push to prod? Hopefully we can do that push before 38.0b4 next week.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

I wrote this PR to update the URL to match the new URL
https://github.com/mozilla/ship-it/pull/17

Comment 12

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

We had some issues with this today with 40.0.1. The suggestion was
  40.0build5, 39.0build6, 38.0.5b3build1, 35.0.1build1

* 40.0 and 39.0 are correct
* 39.0.3 should be added, it has the highest ADI right now
* 38.0.5b3build1 is bogus, was probably mean to be 38.0.5
* next would be 38.0.1, 37.0.2, and then 35.0.1

I'm getting ADI from https://dataviz.mozilla.org/views/DesktopADI/VersionChannelADI; August 10 data. Perhaps the Vertica datasource is unmaintained ?   https://ship-it.mozilla.org/cron/partial.json says it's recently updated but the numbers look very off to me.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

(In reply to Nick Thomas [:nthomas] from comment #12)
> * 39.0.3 should be added, it has the highest ADI right now
OK, probably because the vertica db is not updated frequently enough.
Saptarshi, Sheeri would it be possible to update the data daily? Thanks


> * 38.0.5b3build1 is bogus, was probably mean to be 38.0.5
This is fixed for a while but ship-it prod instance is not up to date:
https://github.com/mozilla/ship-it/commit/6f809e28e1b77bd30f219cad5cdc25f8a26fa428

> I'm getting ADI from
> https://dataviz.mozilla.org/views/DesktopADI/VersionChannelADI; August 10
> data. 
To bad here is no API to access these data...

Perhaps the Vertica datasource is unmaintained ?  
> https://ship-it.mozilla.org/cron/partial.json says it's recently updated 
The update is the generation of the json file, not the latest refresh of vertica data.

Comment 14

["Saptarshi Guha[:joy]"]

The data base fhr_rollups_daily_base (despite the name ..) cannot be updated daily. It depends on the 'de-orphaning' ETL which is once a week. Hence this table can only be updated once a week

Comment 15

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

Rob, how does Socorro get ADI data ? Perhaps we can arrange access so we can use it here too

Comment 16

[Robert Helmer [:rhelmer]]

(In reply to Nick Thomas [:nthomas] from comment #15)
> Rob, how does Socorro get ADI data ? Perhaps we can arrange access so we can
> use it here too

Socorro queries a Hive DB in SCL - there's a little VM in SCL just for accessing this, it runs a cron job which pushes the data to an RDS instances in AWS. Maybe this could push somewhere for you as well.

Alternatively - as this is already in the Socorro RDS instance, you could potentially pull it from crash-stats. I don't see any API endpoint for this when logged into https://crash-stats.mozilla.com/api/ right now though.

:peterbe/:adrian, is there a plan to have a crash-stats API endpoint for ADI? 

:lonnen, any problem with such an API (provided it required Persona auth or token and special privs)?

Comment 17

[[DEACTIVATED] Adrian Gaudebert]

As far as I know, we plan on exposing ADIs as part of our public API. I do not know if we have explicit permission to do that, though. The ADI middleware service exists but is currently only accessible internally, if we have agreement from Lonnen we can easily expose that in the public API.

Comment 18

[Peter Bengtsson [:peterbe]]

(In reply to Robert Helmer [:rhelmer] from comment #16)
> (In reply to Nick Thomas [:nthomas] from comment #15)
> > Rob, how does Socorro get ADI data ? Perhaps we can arrange access so we can
> > use it here too
> 
> Socorro queries a Hive DB in SCL - there's a little VM in SCL just for
> accessing this, it runs a cron job which pushes the data to an RDS instances
> in AWS. Maybe this could push somewhere for you as well.
> 
> Alternatively - as this is already in the Socorro RDS instance, you could
> potentially pull it from crash-stats. I don't see any API endpoint for this
> when logged into https://crash-stats.mozilla.com/api/ right now though.
> 
> :peterbe/:adrian, is there a plan to have a crash-stats API endpoint for
> ADI? 
> 
Any day now :)
It's on my todo-list to add such an endpoint.

Because it's quite new I haven't decided perfect yet how the constraints should be controlled or changed. For example, doing a GET on ADI for the last 2 months might cause such a large slow query that it'd significantly slow down our database. So that might be restrained to smaller date ranges. 
Also, at the time of writing there is no way to filter by operating system. You have to filter by product name and version. 

Wanna set up a blocker on https://bugzilla.mozilla.org/show_bug.cgi?id=1194329 ?

Comment 19

[Lonnen :lonnen]

I'm fine with an api. Now that more than 1 project relies on it, I'd even be ok breaking that logic out and putting it into it's own service that both projects can consume, rather than using crash-stats as a passthrough.

Comment 20

[John Jensen]

Hi there,

> we plan on exposing ADIs as part of our public API. 

Am I interpreting this correctly? We are planning on creating a public API where anyone can obtain information about our active profiles, per day, per build/channel/etc?

Comment 21

[Lonnen :lonnen]

I spoke with John on Friday and I believe we're ok to move forward here. We're talking about using already public numbers from the crash reporter for a feature on an auth-protected internal tool.

The raw data is kind of messy and the data push is fairly brittle. With other and more reliable pipelines nearly done I'm not sure it makes sense to break out the common functionality from Socorro into a new service, though depending on Socorro for this info seems like a weird dependency. That might warrant a little extra thought.

Comment 22

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

Sylvestre, what's the status here ?

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Yes, I contacted the telemetry team to have better data, they have something for this.

Comment 24

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

OK. I'd really like to fix this up as we generated these partials for 45.0.1 - 45.0 is correct, but 42.0, 41.0.2, 39.0 aren't so great. We really ought to have 44.0.2 in there, and 43.0.4 would have been useful too. Could we add a manual step for RelMan to check the partials when setting up a new release ?

Comment 25

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

From 1194329 we can query like this:

https://crash-stats.mozilla.com/api/ADI/?end_date=2016-03-17&start_date=2016-03-17&platforms=Windows&product=Firefox&versions=44.0&versions=44.0.1&versions=44.0.2&versions=45.0
https://crash-stats.mozilla.com/api/ADI/?end_date=2016-03-17&platforms=Windows&product=Firefox&start_date=2016-03-17&versions=45.0b9&versions=45.0&versions=46.0b1

except that the latter doesn't give only beta versions for the RC (versions=45.0).

Comment 26

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

I'm going to have a stab at using the Socorro data.

Comment 27

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

Attachment: PR for switching to crash-stats ADI data

Comment 28

[Rail Aliiev [:rail]]

Comment on attachment 8765336 [details] [review]
PR for switching to crash-stats ADI data

Commented in PR

Comment 29

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

Ready to land the PR, but first bug 1282735 for the server-side changes to support it.

Comment 30

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

I realized I'm not actually blocked by server-side changes as requests should be added to vendor/, and a symlink handles the change of script in the cron job. I've deployed with that added to the PR. Bug 1282735 becomes tidy up work. Still need to verify https://ship-it.mozilla.org/cron/partial.json is updated by the new script.

Comment 31

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

The cron works, eg {"release": [{"version": "45.0.1", "ADI": 1288229}, {"version": "47.0.1", "ADI": 1251634}, {"version": "46.0", "ADI": 416029}, {"version": "45.0.2", "ADI": 1205393}, {"version": "47.0", "ADI": 85961983}, {"version": "46.0.1", "ADI": 6095365}, {"version": "45.0", "ADI": 394895}], "lastUpdate": "30/06/2016 03:01 using 28/06/2016 ADI data", "beta": [{"version": "48.0b2", "ADI": 203671}, {"version": "48.0b3", "ADI": 1405879}, {"version": "48.0b1", "ADI": 295490}, {"version": "48.0b4", "ADI": 2514}], "esr": [{"version": "45.0.2esr", "ADI": 32879}, {"version": "45.0.1esr", "ADI": 56243}, {"version": "38.7.1esr", "ADI": 95134}, {"version": "38.8.0esr", "ADI": 210381}, {"version": "45.1.0esr", "ADI": 87733}, {"version": "45.1.1esr", "ADI": 128450}, {"version": "45.2.0esr", "ADI": 1367821}]}

There are a couple of bugs which mean we still get the wrong partials
* the JS code expects the results to be in descending order of ADI, but it's random coming out of crash-stats
* traversing a X.0 version in the ADI list will match X.0.y in getVersionWithBuildNumber(), eg get 46.0.1buildN instead of 46.0buildN

Comment 32

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

Attachment: PR to fix up use of blocklist ADI

Explanations on the PR.

Comment 33

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

Deployed PR 74, waiting for the cron to run to refresh the ADI data.

Comment 34

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

https://github.com/mozilla/ship-it/pull/75 to fix up one more case where we're matching X.0.Y for X.0 when we shouldn't. Deployed and getting sensible results now - eg for a 48.0 build1 the suggestion is 
  47.0.1build1,47.0build3,46.0.1build1,45.0.1build1
on ADI of
  {"version": "47.0",  "ADI": 76838356}, 
  {"version": "47.0.1", "ADI": 9165114},
  {"version": "46.0.1", "ADI": 5314865},
  {"version": "45.0.1", "ADI": 1225742},
  {"version": "45.0.2", "ADI": 1123018},
  {"version": "46.0",    "ADI": 387763},
  {"version": "45.0",    "ADI": 378686}

Possibly a regression in the number of partials suggested for ESR though, getting 4 instead of 1.

Comment 35

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

I'm not planning on pushing on a telemetry datasource, as the blocklist one seems adequate for now. Will be working on other improvements, eg bug 1292419 for the # of partials, bug 1229682 for using ADI for beta, bug 1177235 & bug 1223196 for RC builds on beta.

RESOLVED GOODENOUGH ?

Comment 36

[Rail Aliiev [:rail]]

(In reply to Nick Thomas [:nthomas] from comment #35) 
> RESOLVED GOODENOUGH ?

YES!