1164260 - Consider changing OF to show number oranges/push

Status: RESOLVED FIXED

(Tree Management Graveyard :: OrangeFactor, defect)

Description

[Jonathan Griffin (:jgriffin)]

The way OF currently works, the concept of "testrun" is very broken, and thus the OF numbers are somewhat arbitrary.

We also know we need to move the OF logparser out of PHX1 if we're going to continue using it (bug 1163758) and the only thing the logparser currently gives us is a (not very consistent) testrun count.

One option to address both of these issues is to change OF to report the number of oranges/push; in other words, to make "testrun" = "push".

Doing this would make the OF numbers more precise, and eliminate the need for logparsing altogether.

I've done this on a local instance, and it works.  One side-effect is that the OF jumps up every weekend (from ~8 to ~20) because there is little to no coalescing and so the number of oranges that are masked by SETA during the week becomes obvious.

I don't think this is a bad thing, but it will need to be communicated well.

Any objections to going this route?

Comment 1

[Jonathan Griffin (:jgriffin)]

One other place we use details from logparsing is to display the number of runs of a certain suite made on a certain machine, for the "Oranges by machine" table.  I'm not sure how often this is used, it's probably not a big deal to omit it.

Comment 2

[Ed Morley [:emorley]]

(In reply to Jonathan Griffin (:jgriffin) from comment #1)
> One other place we use details from logparsing is to display the number of
> runs of a certain suite made on a certain machine, for the "Oranges by
> machine" table.  I'm not sure how often this is used, it's probably not a
> big deal to omit it.

The tracking of failures by machine is already pretty broken - eg the 'count' field on the page is often zero, as are the totals under "suite total runs". Even if they were working, it's also pretty hard to find something actionable in that view - since it really needs percentages and a few other ways of slicing the data.

We also have ouija (http://54.215.155.53/slave_failures.html) which is actually more useful than OF's feature at the moment. For Treeherder, there's bug 1071152 on file.

As such, I think we can just ditch the "Oranges by machine" table.

Comment 3

[Jonathan Griffin (:jgriffin)]

Attachment: Stop using information parsed from logs,

This was fun, I got to exercise my DEL key a lot!

The impacts to OF are just as described in comments 0 and 1; most of the code I removed was actually dead.  We now use json-pushes to get the number of pushes/day.  We could potentially add caching for this so we hit json-pushes less often, but I think this incarnation of OF is likely to go away by Q3 so it didn't seem worthwhile.

Comment 4

[Mark Côté [:mcote]]

Comment on attachment 8605344 [details] [diff] [review]
Stop using information parsed from logs,

Review of attachment 8605344 [details] [diff] [review]:
-----------------------------------------------------------------

Just a couple nits, one probably for a follow-up.  I can't believe we had so much dead server code!

...Well maybe I can.

::: html/scripts/woo.utils.js
@@ +570,2 @@
>        }
> +      tableData.push([machine, count, bugs[i], orangeCount]);

Are we not dropping this table as edmorley suggested?  Or are you doing this later?

::: server/handlers.py
@@ +206,5 @@
> +            tree_dict = json.loads(urllib2.urlopen(tree_url, timeout=30).read())
> +            for pushid in tree_dict:
> +                pushdate = time.localtime(tree_dict[pushid]['date'])
> +                pushdate = time.strftime("%Y-%m-%d", pushdate)
> +                pushlog_dict[pushdate].extend([pushid])

Er why are you not using .append(pushid)?

@@ +349,4 @@
>                  oranges[record['date']]['testruns'] = testrun_count
>  
>          data['oranges'] = oranges
> +        #print oranges['2015-05-02']

Ditch this please.

Comment 5

[Jonathan Griffin (:jgriffin)]

Yeah, we had experimented with a lot of features in OF which were later turned off.

I'll remove the entire "Oranges by machine" table; I had just removed the last column, which was the only one that required data from the logs.  But, since it's not too useful, let's kill it!

Comment 6

[Jonathan Griffin (:jgriffin)]

Attachment: Stop using information parsed from logs,

Fix nits; remove more code to nuke the 'Oranges by machine' table

Comment 7

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8606332 [details] [diff] [review]
Stop using information parsed from logs,

Carry r+ forward

Comment 8

[Ed Morley [:emorley]]

I'm happy to do this as a followup if you'd prefer, but the UI refers to {"testrun", "test-run", "test run"} at several points for titles/descriptions/graph labels. These should now be {"pushes", "push"}. eg:
https://hg.mozilla.org/automation/orangefactor/file/7bb599d15aa4/html/scripts/woo.bugs.js#l274
https://hg.mozilla.org/automation/orangefactor/file/7bb599d15aa4/html/scripts/woo.main.js#l188

Comment 9

[Ed Morley [:emorley]]

Oh and the orangefactor weekly email text:
https://hg.mozilla.org/automation/orangefactor/file/7bb599d15aa4/woo_mailer.py#l20

Comment 10

[Jonathan Griffin (:jgriffin)]

Thanks, I'll fix those too before I deploy.

Comment 11

[Jonathan Griffin (:jgriffin)]

Attachment: Stop using information parsed from logs,

Replaced occurrences of 'test run' and 'testrun' with 'push' in the UI

Comment 12

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8606520 [details] [diff] [review]
Stop using information parsed from logs,

Review of attachment 8606520 [details] [diff] [review]:
-----------------------------------------------------------------

Carry r+ forward

Comment 13

[Jonathan Griffin (:jgriffin)]

http://hg.mozilla.org/automation/orangefactor/rev/bf5a3ca217d3

Comment 14

[Jonathan Griffin (:jgriffin)]

Deployed and posted a note on dev.platform about the change: https://groups.google.com/forum/#!topic/mozilla.dev.platform/dhi8HqiU31Q