Closed Bug 763550 Opened 11 years ago Closed 11 years ago

depend on datazilla client

Categories

(Testing :: Talos, defect)

Product:
Testing
Component:
Talos
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: k0scist, Unassigned)

References

Details

Attachments

(2 files, 2 obsolete files)

example output
879 bytes, text/plain
Details
proposed implementation
23.87 KB, patch
jmaher
: review+
Details | Diff | Splinter Review 
Talos should use https://github.com/mozilla/datazilla_client to post
data to datazilla
The client require json/simplejson so we will have to resolve bug 744405 prior to this (or concurrently)
Depends on: 744405
Blocks: 721097
Depends on: 774480
Pythons that we will have to support in mozharness production:
fedora 32, 64: 2.6.2
leopard 2.5.1 (EOL fx 17)
snow 2.6.1
lion 2.7.1

If we only need this for non-leopard in mozharness, we're good targeting 2.6+.
If we need this on leopard, we have to target 2.5.1+.
If we need this for buildbot talos (non-mozharness), we'll need 2.4+.
So this may actually be impossible until we deprecate talos on python 2.4 or modify datazilla_client.

https://github.com/mozilla/datazilla_client/blob/master/dzclient/__init__.py

uses relative imports, which I believe appear in python 2.5.
The modification there is pretty trivial (removal of exactly one period), so I don't think it should block this. Obviously whenever we can drop 2.4 support we'd want to put the period back, since implicit relative imports are evil ;-)
(In reply to Jeff Hammel [:jhammel] from comment #3)
> So this may actually be impossible until we deprecate talos on python 2.4 or
> modify datazilla_client.
> 
> https://github.com/mozilla/datazilla_client/blob/master/dzclient/__init__.py
> 
> uses relative imports, which I believe appear in python 2.5.

Actually, scratch that.  Since we only care about https://github.com/mozilla/datazilla_client/blob/master/dzclient/client.py we don't need this file.  That'll teach me to try to think late in the day.  I haven't surveyed client.py for 2.5+isms, but the __init__ issues can at least be ignored
So one thing we'll have to figure out in production is how we're going to deal with the client key for oauth2.  :jmaher, :jeads, any ideas?  (And please feel free to mark the bug security if there's anything non-public worth saying)
(In reply to Jeff Hammel [:jhammel] from comment #6)
> So one thing we'll have to figure out in production is how we're going to
> deal with the client key for oauth2.  :jmaher, :jeads, any ideas?  (And
> please feel free to mark the bug security if there's anything non-public
> worth saying)

I vaguely recall :ctalbert saying at one point a while back that this could be handled via talos.json.
(In reply to Carl Meyer (:carljm) from comment #7)
> (In reply to Jeff Hammel [:jhammel] from comment #6)
> > So one thing we'll have to figure out in production is how we're going to
> > deal with the client key for oauth2.  :jmaher, :jeads, any ideas?  (And
> > please feel free to mark the bug security if there's anything non-public
> > worth saying)
> 
> I vaguely recall :ctalbert saying at one point a while back that this could
> be handled via talos.json.

talos.json is public http://hg.mozilla.org/mozilla-central/file/eea94a9b40a1/testing/talos/talos.json so I'm not sure if that works; :ctalbert?
I didn't realize talos.json was public. There are tons of areas where the releng automation uses various secret keys. We don't need to re-invent any architecture here. Just download something off one of the releng non-public servers, use it and clean it up. Ask Armenzg or bhearsum for a proper location.
(In reply to Clint Talbert ( :ctalbert ) from comment #9)
> I didn't realize talos.json was public. There are tons of areas where the
> releng automation uses various secret keys. We don't need to re-invent any
> architecture here. Just download something off one of the releng non-public
> servers, use it and clean it up. Ask Armenzg or bhearsum for a proper
> location.

I ticketed what to do with credentials at bug 776612. I marked it confidential in case any security issues came up. I'm certainly not asking any architecture reinvention...I mostly just need to know how to make talos actually read the credentials, which, since releng will keep and deploy the credentials for production, is really their call
so datazilla_client uses a minimal constructor like:

req = DatazillaRequest(
    host="datazilla.mozilla.org",
    project="project",
    oauth_key="oauth-key",
    oauth_secret="oauth-secret",
    )

We need the request method even to get the JSON string of results.  I'm not precisely sure what a good architecture is for this for the time
So this will probably require a few API changes for talos.  Currently we use a datazilla url like:

http://10.8.73.29/views/api/load_test

(See http://hg.mozilla.org/build/talos/file/c583afd8c89a/talos/PerfConfigurator.py#l288)

However, the datazilla_client takes a host and project.  See comment 11
Attached file example output — 
example output for the v8 benchmark
Attached patch WIP (obsolete) — — Splinter Review 
This is sadly bitrotted, but has pretty much everything except posting.  I'll unbitrot and put something for posting, but bug 776612 prevents an end-to-end chain right now
Attached patch WIP 2 (obsolete) — — Splinter Review 
This is about as far as I can get without bug 776612 being resolved.  How much is left to do kinda depends on how we pass in the data.  It may be necessary to restructure how we pass things into results -> output as we currenly only pass the urls (and have a few things in results).

Also, https://bugzilla.mozilla.org/show_bug.cgi?id=731159#c23 . Probably not actually related but worth bearing in mind
Attachment #649420 - Attachment is obsolete: true
Attachment #649469 - Flags: feedback?(jmaher)
Oh, forgot to mention this depends on the trunk of datazilla_client, not on pypi yet.  Once everything is good to go, pypi will be updated
Comment on attachment 649469 [details] [diff] [review]
WIP 2

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

::: setup.py
@@ +10,5 @@
>  
>  version = "0.0"
>  
> +dependencies = ['PyYAML',
> +                'mozdevice >= 0.2',

can we make mozdevice 0.3 here?  That will save another bug.

::: talos/PerfConfigurator.py
@@ +285,5 @@
>  
>          # default raw_results_url
>          # TODO: deprecate and set via invoker (buildbot, etc)
>          if not self.config.get('datazilla_urls'):
> +            self.config['datazilla_urls'] = ["http://10.8.73.29/views"]

should we switch to the live one now?

::: talos/output.py
@@ +431,5 @@
>      def post(self, results, server, path, scheme):
>  
> +        project = path.strip('/')
> +        req = DatazillaRequest.create(scheme, server, project, None, None, results)
> +        req.submit()

are there any try/excepts we need here?

::: talos/results.py
@@ +408,5 @@
>                      counter_results[counter_name].append(value)
>  
>      def shutdown(self, counter_results):
>          """record shutdown time in counter_results dictionary"""
> +        counter_results.setdefault('shutdown', []).append(int(self.endTime - self.startTime))

int or float?
Attachment #649469 - Flags: feedback?(jmaher) → feedback+
(In reply to Joel Maher (:jmaher) from comment #17)
> Comment on attachment 649469 [details] [diff] [review]
> WIP 2
> 
> Review of attachment 649469 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: setup.py
> @@ +10,5 @@
> >  
> >  version = "0.0"
> >  
> > +dependencies = ['PyYAML',
> > +                'mozdevice >= 0.2',
> 
> can we make mozdevice 0.3 here?  That will save another bug.

Sure

> ::: talos/PerfConfigurator.py
> @@ +285,5 @@
> >  
> >          # default raw_results_url
> >          # TODO: deprecate and set via invoker (buildbot, etc)
> >          if not self.config.get('datazilla_urls'):
> > +            self.config['datazilla_urls'] = ["http://10.8.73.29/views"]
> 
> should we switch to the live one now?

I don't see why not.

> ::: talos/output.py
> @@ +431,5 @@
> >      def post(self, results, server, path, scheme):
> >  
> > +        project = path.strip('/')
> > +        req = DatazillaRequest.create(scheme, server, project, None, None, results)
> > +        req.submit()
> 
> are there any try/excepts we need here?

That's a good question.  The short answer is:  I don't know.  DatazillaClient should have some sort of error handling here (though it probably mostly doesn't), and Talos should have other pieces.  I don't really know what those are though I'll try to figure these pieces out.  We should probably enumerate the possibilities

> ::: talos/results.py
> @@ +408,5 @@
> >                      counter_results[counter_name].append(value)
> >  
> >      def shutdown(self, counter_results):
> >          """record shutdown time in counter_results dictionary"""
> > +        counter_results.setdefault('shutdown', []).append(int(self.endTime - self.startTime))
> 
> int or float?

Currently, we get a (python) long int out for both self.endTime and self.startTime and subtracting them also gives a long int (even though the value is fairly small).  I cast to an int to prevent the long int nonsense, though it can just as easily be a float
as specced out in bug 776612, we will use --authfile to specify the credentials in the form of http://hg.mozilla.org/build/buildbot-configs/file/default/mozilla/passwords.py.template for keys oauthSecret, oauthKey
The actual posting part is untested.  To use this patch you must be using the trunk of datazilla_client, https://github.com/mozilla/datazilla_client/, which is not yet on pypi.
Attachment #649469 - Attachment is obsolete: true
Attachment #651508 - Flags: review?(jmaher)
Correction, datazilla_client has been released to pypi now :)
Comment on attachment 651508 [details] [diff] [review]
proposed implementation

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

Overall this looks really good.

::: talos/output.py
@@ +4,5 @@
>  
>  """output formats for Talos"""
>  
>  import filter
> +import imp

imp? python v2.4?
Attachment #651508 - Flags: review?(jmaher) → review+
Yep, imp works on python 2.4 too
pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=491bcb172689  I wouldn't be at all surprised if there were errors.
(In reply to Jeff Hammel [:jhammel] from comment #24)
> pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=491bcb172689  I
> wouldn't be at all surprised if there were errors.

So far, the failures are all https://github.com/mozilla/datazilla_client/issues/24
Try run for 491bcb172689 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=491bcb172689
Results (out of 69 total builds):
    exception: 5
    success: 23
    warnings: 1
    failure: 40
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla.com-491bcb172689
So this doesn't deal with error handling at all, part of which should probably happen in datazilla_client and part of which will need to happen in talos.  I don't really know of a good way to bootstrap this, outside of taking :jeads' time to help map out what all can/will go wrong. 

At least we should make some noise when we're sending to a URL in talos.  I'll make this change and hope it doesn't break all the things I'm currently testing on try:

https://tbpl.mozilla.org/?tree=Try&rev=c427d78c08a4

So far, so green.
And the changesets do make it in to datazilla.  Going to land this.
pushed: http://hg.mozilla.org/build/talos/rev/e3ab367914e6
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.