[Piglit] [PATCH] framework/log: add an HTTP logger
Thomas Wood
thomas.wood at intel.com
Tue Sep 22 07:17:53 PDT 2015
On 21 September 2015 at 19:13, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> On Mon, Sep 21, 2015 at 05:06:17PM +0100, Thomas Wood wrote:
>> Add an HTTP logger so that it is possible to monitor the progress and status of
>> piglit remotely through a webserver. The webserver runs on port 8080 and
>> responds to requests for /summary and /results, which provide the current
>> summary and results in JSON format.
>> ---
>>
>> Sometimes it is useful to be able to monitor piglit remotely, especially during
>> long test runs such as when running the full igt test profile. This patch adds
>> an HTTP server as part of the logging infrastructure to fulfil this objective.
>>
>> There are a few issues to be resolved, such as making the HTTPLogServer thread
>> safe, so that it uses the appropriate locking when reading state data. The
>> server also runs indefinitely, so it may be useful to add a special method to
>> stop it. Passing the full test result back to the log system could also be
>> split out into a separate patch.
>>
>>
>> framework/log.py | 80 ++++++++++++++++++++++++++++++++++++++++++++---
>> framework/programs/run.py | 2 +-
>> framework/test/base.py | 2 +-
>> 3 files changed, 77 insertions(+), 7 deletions(-)
>>
>> diff --git a/framework/log.py b/framework/log.py
>> index 759974a..31d2b39 100644
>> --- a/framework/log.py
>> +++ b/framework/log.py
>> @@ -32,6 +32,12 @@ import abc
>> import itertools
>> import threading
>> import collections
>> +from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler
>> +try:
>> + import simplejson as json
>> +except ImportError:
>> + import json
>> +
>>
>> __all__ = ['LogManager']
>>
>> @@ -66,7 +72,7 @@ class BaseLog(object):
>> """
>>
>> @abc.abstractmethod
>> - def log(self, status):
>> + def log(self, result):
>> """ Print the result of the test
>>
>> This method is run after the test completes, and is used to log the
>> @@ -132,9 +138,9 @@ class QuietLog(BaseLog):
>> self._print_summary()
>> self._state['running'].remove(self.__counter)
>>
>> - def log(self, status):
>> + def log(self, result):
>> with self._LOCK:
>> - self._log(status)
>> + self._log(result['result'])
>>
>> def summary(self):
>> with self._LOCK:
>> @@ -244,13 +250,70 @@ class DummyLog(BaseLog):
>> def start(self, name):
>> pass
>>
>> - def log(self, status):
>> + def log(self, result):
>> pass
>>
>> def summary(self):
>> pass
>>
>>
>> +class HTTPLogServer(threading.Thread):
>> + class RequestHandler(BaseHTTPRequestHandler):
>> + INDENT = 4
>> +
>> + def do_GET(self):
>> + if self.path == "/summary":
>> + self.send_response(200)
>> + self.end_headers()
>> + status = {
>> + "complete": self.server.state["complete"],
>> + "running" : self.server.state["running"],
>> + "total" : self.server.state["total"],
>> + }
>
> The formatting on this is wrong:
> status = {
> "complete": self.server.state["complete"],
> "running" : self.server.state["running"],
> "total" : self.server.state["total"],
> }
>
>
>> + self.wfile.write(json.dumps(status, indent=self.INDENT))
>> + elif self.path == "/results":
>> + self.send_response(200)
>> + self.end_headers()
>> + results = self.server.state["results"]
>> + self.wfile.write(json.dumps(results, indent=self.INDENT))
>> + else:
>> + self.send_response(404)
>> + self.end_headers()
>> +
>> + def __init__(self, state):
>> + super(HTTPLogServer, self).__init__()
>> + self._httpd = HTTPServer(("", 8080), HTTPLogServer.RequestHandler)
>
> It might be worthwhile to make this value configurable in piglit.conf,
> in case someone wants to run on a port other than 8080, it is a pretty
> heavily used port
No problem, I've prepared a patch to do this.
>
>> + self._httpd.state = state
>> +
>> + def run(self):
>> + self._httpd.serve_forever()
>> +
>> +
>> +class HTTPLog(BaseLog):
>> + """ A Logger that serves status information over http """
>> +
>> + def __init__(self, state):
>> + super(HTTPLog, self).__init__(state)
>> + self._name = None
>> +
>> + def start(self, name):
>> + with self._LOCK:
>> + self._name = name
>> + self._state['running'].append(self._name)
>> +
>> + def log(self, result):
>> + with self._LOCK:
>> + self._state['running'].remove(self._name)
>> + self._state['complete'] += 1
>> + status = result['result']
>> + assert status in self.SUMMARY_KEYS
>> + self._state['summary'][status] += 1
>> + self._state['results'][self._name] = result
>> +
>> + def summary(self):
>> + pass
>> +
>> +
>> class LogManager(object):
>> """ Creates new log objects
>>
>> @@ -273,6 +336,7 @@ class LogManager(object):
>> 'quiet': QuietLog,
>> 'verbose': VerboseLog,
>> 'dummy': DummyLog,
>> + 'http': HTTPLog,
>> }
>>
>> def __init__(self, logger, total):
>> @@ -284,7 +348,13 @@ class LogManager(object):
>> 'lastlength': 0,
>> 'complete': 0,
>> 'running': [],
>> - }
>> + 'results': {},
>
> I'm not sure how I feel about this; it looks like we're storing a
> non-insignificant second copy of the data in TestrunResult, which seems
> bad. On the other hand I'm not sure off the top of my head how to pass
> the TestrunResult to the LogManager without ugly hacks.
I had the same thought, but on closer inspection it looks like the
result backends don't actually store the whole set of results in
memory. I'm not sure how important sharing the full results over http
is, although it might be useful to monitor which tests have already
executed.
>
>> + }
>
> Please dedent this one level like it was before.
>
>> +
>> + # start the http server for http logger
>> + if logger == 'http':
>> + self.log_server = HTTPLogServer(self._state)
>> + self.log_server.start()
>>
>> def get(self):
>> """ Return a new log instance """
>> diff --git a/framework/programs/run.py b/framework/programs/run.py
>> index bf8cc10..da99ac6 100644
>> --- a/framework/programs/run.py
>> +++ b/framework/programs/run.py
>> @@ -158,7 +158,7 @@ def _run_parser(input_):
>> log_parser.add_argument("-l", "--log-level",
>> dest="log_level",
>> action="store",
>> - choices=['quiet', 'verbose', 'dummy'],
>> + choices=['quiet', 'verbose', 'dummy', 'http'],
>> default='quiet',
>> help="Set the logger verbosity level")
>> parser.add_argument("--test-list",
>> diff --git a/framework/test/base.py b/framework/test/base.py
>> index 798e5a6..86086a8 100644
>> --- a/framework/test/base.py
>> +++ b/framework/test/base.py
>> @@ -191,7 +191,7 @@ class Test(object):
>> self.result['traceback'] = "".join(
>> traceback.format_tb(exception[2]))
>>
>> - log.log(self.result['result'])
>> + log.log(self.result)
>> else:
>> log.log('dry-run')
>>
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
More information about the Piglit
mailing list