Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor: flexible Y-Min and Y-Max settings #6051

Merged
merged 5 commits into from Sep 15, 2016
Merged

Refactor: flexible Y-Min and Y-Max settings #6051

merged 5 commits into from Sep 15, 2016

Conversation

alexanderzobnin
Copy link
Contributor

Refactor of #5720

thoj and others added 5 commits August 12, 2016 09:37
@thoj
Feature: Flexible Y-Min and Y-Max settings.
839b320 
Y-Min and Y-Max is now string.
New usage for both Y-Min and Y-Max (Where X is a real number):
  >X Y-Max/Y-Max is auto if data is above X else X
  <X Y-Max/Y-Min is auto if data is below X else X
  =X Y-Max/Y-Min is scaled to current value +/- X
  ~X Y-Max/Y-Min is caled to average value +/- X

Example: Y-Min: <100 Y-Max: >200
If all points are within 100 and 200: Y-Min is 100 and Y-Max is 200
If some points are above 200: Y-Min is 100 and Y-Max is auto
If some points are below 100: Y-Min is auto and Y-Max is 200
if some points are below 100 and above 200: Y-Min and Y-Max is auto

Tests for new settings added
@thoj
Use parseFloat instead of implicit conversion
56710c3
@alexanderzobnin
Merge branch 'autoscaling-min-span' of https://github.com/thoj/grafana …
8db812b 
…into thoj-autoscaling-min-span
@alexanderzobnin
feat(flexible_y-min/max): refactor.
f7b95f9
@alexanderzobnin
feat(flexible_y-min/max): added more tests.
c30d14c
@torkelo torkelo merged commit 4fa22e2 into grafana:master Sep 15, 2016
@torkelo torkelo mentioned this pull request Sep 15, 2016
Closed
@torkelo torkelo added this to the 4.0.0-Beta1 milestone Sep 15, 2016
This was referenced Sep 15, 2016
Closed
Closed
Closed
@cfra
Copy link
Contributor

cfra commented Sep 16, 2016

I just tried this out and am not able to use it very well.
a) plain negative numbers for limits don't seem to work anymore
b) I am not able to set ymax in a way so that it defaults to e.g. 150 but will scale up if data becomes too
large
c) I cannot define ymax for multiple graphs from a template

It might also be that I am doing it wrong, I just skimmed over the code because I did not find any documentation.

@torkelo
Copy link
Member

torkelo commented Sep 17, 2016

@alexanderzobnin can you check those this ^ (a, b, c)

@alexanderzobnin
Copy link
Contributor Author

@torkelo ok, will check this.

@alexanderzobnin
Copy link
Contributor Author

@cfra can you explain b) and c) ?

@alexanderzobnin alexanderzobnin mentioned this pull request Sep 17, 2016
Merged
3 tasks
@cfra
Copy link
Contributor

cfra commented Sep 22, 2016 •
edited

b) c) relate to the same usecase:
I have a dashboard with a large set of graphs showing the distribution of wifi clients over ap locations, each showing a different timerange. I would like to have all graphs default to the Y-Range 0-1000 so that total client count is easily comparable. However I don't want a graph to be clipped if I encounter more than 1000 clients, but I would like it to be automatically scaled to a sufficient Y-Range, e.g. 0-1500. (This is speaking about a single graph now, not all graphs in the dashboard)

b) I tried achiving this with different modifiers. Toying around with it now, the behavior seems somewhat errativ. I leave Y-Min at 0 and play with Y-Max. The test graph peaks at 126. If I don't enter anything for Y-Max, it ends up at 150. For other inputs it seems somewhat erratic as displayed in this table.

Input for Y-Max Resulting Y-Max
<10 10
<100 150
<200 150
>10 150
>100 100
>150 150
>200 200

Maybe my expectations about the semantics are incorrect and what I want is not possible. The behavior of >x seems to be almost what I need, but I do not comprehend why >10 yields 150 and >100 yields 100. For <, I also do not comprehend why <10 and <100 yield different results.

c) I would like to adjust all graphs at once to another scale e.g. to go from 0..1000 to 0..1500. For this, it would be very useful if I could define Y-Max as >$ymax with ymax being a dashboard variable instead of having to adjust it separately for all graphs.

@alexanderzobnin
Copy link
Contributor Author

@cfra ok, I will check this.

@alexanderzobnin
Copy link
Contributor Author

@cfra can you attach screenshots with incorrect behaviour?

@cfra
Copy link
Contributor

cfra commented Sep 22, 2016

@alexanderzobnin Here you go, let me know if you need anything else: http://nowhere.ws/dump/cropped/

@alexanderzobnin
Copy link
Contributor Author

Do you use stack mode in this graph? I think, problem's root is here.

@cfra
Copy link
Contributor

cfra commented Sep 22, 2016

Yes, the graph uses stacked mode. I uploaded the json of the graph here, if that helps. https://np.tl/paste/nMgDevPm#vBOJgWSOar1-Tz8EMl0tAY/ZVH7de+BA8DdnlENIaga

@alexanderzobnin
Copy link
Contributor Author

@cfra thanks. I need to add more tests for stack mode.

@alexanderzobnin
Copy link
Contributor Author

@torkelo seems, we need to move Y-min and Y-max calculating to jquery.flot.js. Because if stack mode enabled, stacked series values calculates there. So we need to place autoscaleSpanOverride() after this loop: https://github.com/grafana/grafana/blob/master/public/vendor/flot/jquery.flot.js#L1247

@torkelo
Copy link
Member

torkelo commented Sep 26, 2016

@alexanderzobnin I think I have a solution for that

@vaizki
Copy link

vaizki commented Feb 3, 2017

I've tried this with the docker image grafana/grafana:master which reports itself as v4.2.0-pre1 (commit: 457d6c8) and the y-axis disappears completely if I enter any of these special expressions (such as >100). Has this been reverted or replaced but I just can't spot it in GitHub or is it just broken?

@bcwilsondotcom
Copy link

I see the same behavior as vaizki.

@tzach tzach mentioned this pull request Apr 13, 2017
Closed
@hahnjo hahnjo mentioned this pull request Apr 30, 2018
Open
@Birne94 Birne94 mentioned this pull request Aug 29, 2018
Closed
@robfredtwitch
Copy link

For anyone else who ended up here like I did, it says in the closed issue above that this PR was reverted #13076 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.0.0-beta1
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@alexanderzobnin @cfra @torkelo @vaizki @bcwilsondotcom @robfredtwitch @thoj