Add the support of group by for metric to Arithmetic #63
Conversation
…g the 'output name' field as a prefix when group by is used in arithmetic metrics
README.md
Outdated
| @@ -22,8 +22,7 @@ Create a new datasource with a name and select `type` as `MetaQueries` | |||
| ## Examples | |||
| #### Arithmetic | |||
| Lets you perform arithmetic operations on one or more existing queries. | |||
|  | |||
|  | |||
|  | |||
There was a problem hiding this comment.
not sure why would we remove existing examples ?
There was a problem hiding this comment.
I didn't even see this was changed from the previous merge request. I reverted these changes (I may add the last screenshot later, but with the light theme for consistency)
| @@ -82,7 +82,20 @@ | |||
| <label class="gf-form-label" bs-tooltip="ctrl.target.errors.expression" style="color: rgb(229, 189, 28)" ng-show="ctrl.target.errors.expression"> | |||
| <i class="fa fa-warning"></i> | |||
| </label> | |||
|
|
|||
| <label class="gf-form-label query-keyword width-4"> | |||
| Order | |||
| </select> | ||
| </div> | ||
| <label class="gf-form-label query-keyword width-4"> | ||
| Size |
| @@ -119,8 +119,15 @@ function (angular, _, dateMath, moment) { | |||
|
|
|||
| } | |||
| else if (target.queryType === 'Arithmetic') { | |||
| var expression = target.expression || ''; | |||
| var queryLetters = Object.keys(targetsByRefId).filter(x => expression.indexOf(x + '[') !== -1); | |||
There was a problem hiding this comment.
why are we filtering out arithmetic queries ?
There was a problem hiding this comment.
I don't really know why this was changed on the last PR, but apparently this code is the reason why it is not possible to make nested arithmetic grouped metrics.
As soon as I have some time I will take a look at this.
There was a problem hiding this comment.
@Gauravshah if the expression did not use some metric, then the metric is not considered here.
There was a problem hiding this comment.
is there any advantage to skip ? people can use A.var1 as well. And I am not sure what else would they use to access
| try { | ||
| result = expressionFunction.apply(this, resultTotal.value); | ||
| } catch(err) { | ||
| console.log(err); |
There was a problem hiding this comment.
can we add more logging details ?
| this.target.orderSize = 5; | ||
| } | ||
|
|
||
| this.orderSizes = [ |
There was a problem hiding this comment.
why are we limiting the results by orderSize ? can you help me understand the use case for it ?
There was a problem hiding this comment.
I use this so some insignificant results don't clutter a graph.
For example, in the graph on this PR description I want to see the hit rate on some redis servers. But the only servers that are interesting to me are the ones with low hit rate (low hit rate is an issue for me).
The orderSize helps me unclutter the graph and visualize faster the servers where I need to take action.
Follow up of PR #42