742824 - send raw data from talos to staging graph server

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

in order to fully develop and test the new graph server, we need to send data to it.  This will be raw data instead of summarized data.

This data will go from production talos machines to the staging server in addition to the regular posting to the production graph server.

upon error, we will silently fail.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: send raw values from talos to graph server (0.9)

this is the patch I was using.  I would like to get some general feedback on this before a massive cleanup.  I think it needs some polish and cleanup, but otherwise it should be pretty good.

Comment 2

[Jeff Hammel]

Comment on attachment 612638 [details] [diff] [review]
send raw values from talos to graph server (0.9)

diff -r cb3dd73f0524 talos/mobile_profile/prefs.js
--- a/talos/mobile_profile/prefs.js	Fri Mar 09 17:47:26 2012 -0500
+++ b/talos/mobile_profile/prefs.js	Mon Mar 12 13:46:18 2012 -0400
@@ -13,4 +13,4 @@
 
 // for fennec, it's convenient to always enable the error console in case
 // something goes wrong
-user_pref("devtools.errorconsole.enabled", true);
+//user_pref("devtools.errorconsole.enabled", true);

Why is this part of this patch?

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

I believe I mentioned something about a small amount of cleanup, that is an accident.

Comment 4

[Jeff Hammel]

Comment on attachment 612638 [details] [diff] [review]
send raw values from talos to graph server (0.9)

The post_multipart stuff should be refactored before we land.  post_multipart is a pretty badly written function, but i would rather fix it than fork it.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: send raw results to staging graph server (1.0)

ok, this is refactored and tested locally.  On try server now.

Comment 6

[Jonathan Eads ( :jeads )]

I've included two json examples below:

{"test_machine": {"name": "talos-r3-fed-009", "os": "linux", "osversion": "Ubuntu 11.10", "platform": "x86_64"}, "test_build": {"name": "Firefox", "version": "14.0a1", "revision": "7727a7150837", "branch":  "Mozilla-Inbound-Non-PGO", "id": "20120406110343"}, "testrun": {"date": "1334004618", "suite": "Talos ts", "options": {"rss": false, "tpchrome": true, "tpmozafterpaint": false, "tpcycles": "10", "tppagecycles": 1, "tprender": false, "tpdelay": "", "responsiveness": false, "shutdown": true}}, "results": {"pagename": [759], "pagename": [606]}, "results_aux": {"shutdown": [822, 717]}}

{"test_machine": {"name": "talos-r3-fed64-026", "os": "linux", "osversion": "fedora 12", "platform": "x86_64"}, "test_build": {"name": "Firefox", "version": "14.0a1", "revision": "e91ea8e4cf15", "branch":  "Try", "id": "20120409191850"}, "testrun": {"date": "1334031563", "suite": "Talos ts_places_generated_max", "options": {"rss": false, "tpchrome": true, "tpmozafterpaint": false, "tpcycles": "10", "tppagecycles": 1, "tprender": false, "tpdelay": "", "responsiveness": false, "shutdown": true}}, "results": {"pagename": [2889], "pagename": [571], "pagename": [558], "pagename": [603], "pagename": [586], "pagename": [550], "pagename": [566], "pagename": [595], "pagename": [623], "pagename": [601], "pagename": [607], "pagename": [657], "pagename": [550], "pagename": [602], "pagename": [570], "pagename": [575], "pagename": [583], "pagename": [599], "pagename": [546], "pagename": [576]}, "results_aux": {"shutdown": [546, 439, 442, 432, 455, 449, 409, 452, 445, 461, 455, 456, 471, 451, 447, 480, 446, 454, 452, 459]}}


In the results attribute there are multiple occurrences of the the string "pagename" instead of the actual webpage name found in the test.  There are multiple cases where this seems to be happening in the json structures submitted.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

both of these are the 'ts' case and I have corrected the hacked 'pagename' and the results to be an array.

I have another push on try server, and we will see how it goes.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: send raw results to staging graph server (1.1)

updated patch to ignore failures from posting to staging as well as not printing anything related to the return code or status to avoid the dreaded regex('error').  

Also, this patch fixes the ts stuff mentioned earlier in the comments.

Comment 9

[Jeff Hammel]

+      if fields:
+          # Summarized results to the official graph server
+          content_type, body = encode_multipart_formdata(fields, files)
+      else:
+          # We are posting raw results to the staging graph server
+          content_type = "application/x-www-form-urlencoded"
+          body = "data=%s" % urllib.quote(files)
+

So I think we *don't* want to do this.  You're passing in what is actually a field (data=your stuff) to files.  The only reason we currently pass a field at all is actually a bug:

https://bugzilla.mozilla.org/show_bug.cgi?id=716057

We send -- literally -- the encoded "key=value" for no reason which, ABICT, is not even paid attention to by the graphserver code.

run_tests.py:          links += process_Request(post_file.post_multipart(results_server, results_path, [("key", "value")], [("filename", "data_string", data_string)]))

Have you tried just using fields=[('data', raw_results)]? (You may have to use ('data', urllib.urlquote(raw_results)) though we should make post_multipart do this for you if it doesn't work OOTB). Assuming the rest of the code is correct, this should work, i think (depending on the graphserver code that's reading it; in any case it should be a correctly encoded multi-part form I think)

I think the function signature should change to something like:

def post_multipart(host, path, fields=(), files=()):

to be most flexible

* (I have no idea why what i call path is currently called selector here. I'm fine keeping the name but I have never heard it called that before, almost always path or path_info)

I haven't done bug 716057 yet since there wasn't really a reason, but I can put up a fix there if that is helpful.  Should be easy.

I'll review the rest of the patch subsequently.

Comment 10

[Jeff Hammel]

+    browser_dump, counter_dump, print_format, test_config = results[testname]

This appears many different places in the code :(  Not necessary for this patch, but we should file a follow-up bug to use an object here instead some crazy tuple that needs to be changed in many places (and then fix it, of course)

Comment 11

[Jeff Hammel]

+    def raw_values(self):
+        retVal = {}
+        for result in self.results:
+            retVal[result['page']] = result['runs']
+        return retVal

I'd probably write

return dict([(result['page'], result['runs']) for result in self.results])

+    print post_file.post_multipart("http://10.8.73.29", "/views/api/load_test", None, raw_results)
+    print "done posting raw results to staging server"

So this is a hack and its probably fine for now (aside from the issues from comment 9) as we transition from current graphserver to new graphserver but we should clearly and loudly mark it as such and file a bug for how it should actually work.  AIUI, for instance, production runs will start failing if this graphserver isn't up (!).  That can be fixed with a try-except but ultimately specifying a graphserver we want to send raw values to should live in configuration, not hard-coded here.

There's a lot of hand-serialization of JSON here.  We should file a bug so we can get talos on JSON.

So, summary:
* fix the post_multipart function to be a generic HTTP multi-part post and pass in ('data', raw_results) to its fields
* The rest is fine but we need follow-up bugs on having a results object instead of a long tuple
* and being able to specify server(s) to send raw results to
* and to get talos on simplejson/json in production so that we can sensibly serialize this code

Comment 12

[Jeff Hammel]

Comment on attachment 613676 [details] [diff] [review]
send raw results to staging graph server (1.1)

See https://bugzilla.mozilla.org/show_bug.cgi?id=742824#c11

Fix post_multipart is the main thing and a bunch of follow-up bugs

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: send raw results to staging graph server (1.2)

carryover r+ as this addresses the post_multipart issues.  Once try server is back online I will run a pass through there and land this.

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

http://hg.mozilla.org/build/talos/rev/e3ec1e3176eb