Merge lp://staging/~richardw/jarmon/independent-ds-transformers-1188220 into lp://staging/jarmon

Proposed by Richard Wall
Status: Needs review
Proposed branch: lp://staging/~richardw/jarmon/independent-ds-transformers-1188220
Merge into: lp://staging/jarmon
Diff against target: 178 lines (+26/-25)
2 files modified
docs/examples/jarmon_example_recipes.js (+2/-2)
jarmon/jarmon.js (+24/-23)
To merge this branch: bzr merge lp://staging/~richardw/jarmon/independent-ds-transformers-1188220
Reviewer Review Type Date Requested Status
Petr P (community) functional testing Approve
Richard Wall functional testing Needs Resubmitting
Review via email: mp+169468@code.staging.launchpad.net

Description of the change

I hacked things around a bit.

RrdDsProxy now accepts transformer and passes as an argument to RrdQuery/RrdQueryRemote.getData

I'm not really happy with this but it does have the desired effect (at least for me with Fedora/Chrome/collectd5)

When I eventually get round to releasing another Jarmon, I might update / simplify all this RRD download and parsing code.

Anyway, please try out jarmon.js from this branch and let me know if it fixes things for you.

To post a comment you must log in.
Revision history for this message
Petr P (trubus) wrote :

I just tested it on our setup - it has the desired effect, I'm almost completely satisfied, but other thing turned up.

When the resulting graph is mostly negative (let's imagine Y-axis has max = 100 and min = -1000000, in other words, there is a lot more TX than RX for example), it sets the prefix incorrectly (according to the max value only) and it generates lot of steps.

I propose using the greater value of axis.min and axis.max in "this.options.yaxis.ticks" instead of axis.max only. Other possibility is to use difference (axis.max - axis.min), but that seems a bit more tricky to me.

Other than that it works fine, thanks a lot!

Peter

Revision history for this message
Petr P (trubus) wrote :

Just to be a bit more precise - I don't propose using greater value, but the value that is farther away from zero :)

Revision history for this message
Petr P (trubus) wrote :

It seems a quick fix, could you add this small change? Then I'll approve :)

652c652
< if( Math.pow(1000, si+1)*0.9 > axis.max ) {
---
> if( Math.pow(1000, si+1)*0.9 > axis.max && -(Math.pow(1000, si+1))*0.9 < axis.min) {

78. By Richard Wall

add extra condition suggested by trubus to take into account negative y axis

79. By Richard Wall

choose best si based on total length of y axis - negative to positive. Also add a few more step sizes to graphs without any ticks.

Revision history for this message
Richard Wall (richardw) wrote :

I applied your diff and it does give properly spaced negative ticks, but I couldn't clearly understand why.
I came up with a slightly different condition which seems to have the same effect and which I think I understand.

See what you think and let me know if it works for you.

I also added a couple of extra step sizes because (on my system ) I was graphs with only two tick labels.

Thanks again for testing.

-RichardW.

review: Needs Resubmitting (functional testing)
Revision history for this message
Petr P (trubus) wrote :

Works as expected, great job!

review: Approve (functional testing)

Unmerged revisions

79. By Richard Wall

choose best si based on total length of y axis - negative to positive. Also add a few more step sizes to graphs without any ticks.

78. By Richard Wall

add extra condition suggested by trubus to take into account negative y axis

77. By Richard Wall

hack things around to allow independent ds transformers, even when they are in the same RRD file

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: