643897 - label axes and show appropriate unit name

Status: RESOLVED INCOMPLETE

(Webtools Graveyard :: Graph Server, defect, P3)

Description

[Robert Helmer [:rhelmer]]

Units are assumed to be seconds, and are multiplied by 1000 for flot which wants ms

x-axis should only be "time" for time-based units, same for the seconds->ms conversion

Comment 1

[Robert Helmer [:rhelmer]]

I think we'd need to add this to the JSON API, perhaps the default could be seconds unless overridden (it can be bytes in the case of memory tests, for example).

Comment 2

[Justin Lebar (not reading bugmail)]

So this (or bug 659304) really tripped me up a few days ago (bug 681755 comment 47).

TBPL displays memory tests' results as MB (1MB = 2^20 bytes), but graphserver displays bytes.  I compared the TBPL results to graphserver's results by looking at the top three digits of the graphserver results.  But of course that meant that I thought the TBPL results beat graphserver's by 2-3%.

If we can't get units from the JSON feed, can we just hack graphserver to understand which tests report bytes, and display sensible units (megabytes) instead of bytes?  I presume this is what TBPL is doing.

Comment 3

[Robert Helmer [:rhelmer]]

(In reply to Justin Lebar [:jlebar] from comment #2)
> So this (or bug 659304) really tripped me up a few days ago (bug 681755
> comment 47).
> 
> TBPL displays memory tests' results as MB (1MB = 2^20 bytes), but
> graphserver displays bytes.  I compared the TBPL results to graphserver's
> results by looking at the top three digits of the graphserver results.  But
> of course that meant that I thought the TBPL results beat graphserver's by
> 2-3%.
> 
> If we can't get units from the JSON feed, can we just hack graphserver to
> understand which tests report bytes, and display sensible units (megabytes)
> instead of bytes?  I presume this is what TBPL is doing.

Yes I think that'd be a fine temporary solution, although long-term I'd like to see the tests submitting this info along with the results, so the UI simply expose it.

Comment 4

[Scott Johnson (:jwir3)]

Attachment: Screenshot of the unlabeled chart

We should also label the units (or give some indication of what the results mean) when the popup chart is display (I'm attaching a screenshot).

Comment 5

[Justin Lebar (not reading bugmail)]

It's not at all clear from this bug, but we're talking about the new graphserver (at http://graphs-new.mozilla.org) here.  I don't think much (any?) energy is going into the old graphserver at this point.

Comment 6

[Robert Helmer [:rhelmer]]

(In reply to Justin Lebar [:jlebar] from comment #5)
> It's not at all clear from this bug, but we're talking about the new
> graphserver (at http://graphs-new.mozilla.org) here.  I don't think much
> (any?) energy is going into the old graphserver at this point.

Not really, I'll fix any critical bugs in the old one for now but not adding features like this to it.

I am hoping to make graphs-new the default this quarter FWIW, since there's a lot of confusion over this :)

Comment 7

[Mark Côté [:mcote]]

Btw I've done a lot of work on an axis-labels plugin for flot for the orangefactor website:

https://github.com/markrcote/flot-axislabels

Comment 8

[Robert Helmer [:rhelmer]]

rniwa just landed a frontend patch for this:
https://github.com/mozilla/graphs/commit/ba7b2b21f5b001f8513aca8e613f6e0920d8fff3

We just need talos/graphserver to supply the units in the JSON and it'll work for us too.

Comment 9

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #8)
> rniwa just landed a frontend patch for this:
> https://github.com/mozilla/graphs/commit/
> ba7b2b21f5b001f8513aca8e613f6e0920d8fff3
> 
> We just need talos/graphserver to supply the units in the JSON and it'll
> work for us too.

Is it easy/possible to get talos to do this in the short term ^?

Comment 10

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

talos could do that without too much trouble.  The devil is in the details, but it doesn't sound too outrageous.

Comment 11

[Robert Helmer [:rhelmer]]

graphs.m.o is deprecated in favor of datazilla at this point; not actively working on this

Comment 12

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.