Resource monitor API for dumping data as JSON

RESOLVED FIXED in mozilla32

Status

Testing
Mozbase
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

Trunk
mozilla32
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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).
(Assignee)

Updated

5 years ago
Blocks: 893388
(Assignee)

Comment 1

5 years ago
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+
(Assignee)

Comment 3

4 years ago
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/mozilla-central/rev/f50bbf4738d0
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.