In bug 883209, we hooked resource monitoring up to the build system and added an HTML viewer. Looking towards the future, we'd like to converge all the users of resource monitoring (build, mozharness, etc) so they can share the same interface for rendering results. This likely means sharing a unified data format. I think it makes sense to add an API to the resource monitor class for dumping the resource data as JSON (or at least as a Python object that can be serialized to JSON).
Created attachment 794296 [details] [diff] [review] 0001-Bug-907331-Dump-resource-usage-data-to-a-serializabl.patch Here's my first stab at it. I didn't add a JSON API, but merely a "convert to dict" which can then be serialized into JSON easily enough. This also facilitates downstream consumers adding additional metadata to the data structure.
Assignee: nobody → gps
Status: NEW → ASSIGNED
Attachment #794296 - Flags: review?(ahalberstadt)
Comment on attachment 794296 [details] [diff] [review] 0001-Bug-907331-Dump-resource-usage-data-to-a-serializabl.patch Review of attachment 794296 [details] [diff] [review]: ----------------------------------------------------------------- Most of the terminology here is over my head, but I don't see anything wrong with code itself. ::: mozsystemmonitor/mozsystemmonitor/resourcemonitor.py @@ +495,5 @@ > + has 2 elements, the float wall time of the event and the string > + event name. > + phases - A list of dicts describing phases. Each phase looks a lot > + like an entry from samples (see below). Some phases may not have > + data recorded against them, so some keys may be None. nit: some values may be None? ::: mozsystemmonitor/mozsystemmonitor/test/test_resource_monitor.py @@ +172,5 @@ > + d = monitor.as_dict() > + > + self.assertEqual(d['version'], 1) > + self.assertEqual(len(d['events']), 2) > + self.assertEqual(len(d['phases']), 2) Wouldn't hurt to add a check for the other keys as well.
Attachment #794296 - Flags: review?(ahalberstadt) → review+
And here we have an r+ that's been lingering for 9 months. What a huge fail on my part. Why doesn't Bugzilla nag about this? If I can land this easily, I'll do it.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f50bbf4738d0 Trivial patch applied cleanly.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.