Fix rescaling y-axis edge cases #1437
Conversation
| const linearMetrics = [ | ||
| 'histogram-linear', | ||
| 'quantity', | ||
| 'counter', | ||
| 'labeled_counter', | ||
| ]; |
There was a problem hiding this comment.
These metrics have linear graph type, but initially I only had histogram-linear and quantity on this list, so other linear graphs such as counter and labeled_counter did not scale (see counter and labeled_counter)
After adding them to the list, we can rescale y-axis for counter and labeled_counter now.
Working example of a counter metric:
| yScaleType === 'scalePoint' | ||
| ) { | ||
| // do not change yDomain when all percentiles are selected | ||
| // (we want the graph to match the violin plot initially) | ||
| if ($store.visiblePercentiles.length === 5) return yDomain; |
There was a problem hiding this comment.
We were not able to rescale the y axis because we used to have the same set of data for yDomain regardless of toggled percentiles. In my last change, I calculated the yDomain range to only include the highest and smallest values available of the shown percentile lines, so that this array will dynamically change if a percentile is toggled. However this means that the range is more tightly scoped than before, and you can see the difference in initial behavior here: dev vs stage.
@rafrombrc wanted the initial range of the graph to match the violin plot graph, so I added this condition here to preserve the yDomain range if all percentile lines are visible (i.e no percentiles toggled yet)
Below you can see that the yDomain range matches the violin plot initially.
| if (data[0].transformedPercentiles) { | ||
| visiblePercentiles.forEach((p) => { | ||
| yData = yData.concat([ | ||
| ...data.map((arr) => arr.transformedPercentiles[p]), | ||
| ]); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Currently on dev this metric doesn't have any data. After changing data source to $store.transformedPercentiles (which I believe is the main source of data for Glean metrics, we use $store.percentiles for desktop probes), it does now.
This PR fixes 3 things that we noticed after testing dev:
Use transformed percentile data for Android metrics. For example, the graph for this metric doesn't display any data because we were using data from the regular percentile source (like we did for telemetry probes), instead of sourcing from
transformedPercentiles, which is for Android metrics.Add more metrics to linear graph group, such as counter and labeled_counter. This will let us rescale y-axis for these metrics too, since currently they do not on dev (see examples for counter and labeled_counter)
@rafrombrc pointed out that we should keep the y domain range to match the violin plot at first, and only rescale it when a percentile is toggled. I added a condition to fix this.