-
Notifications
You must be signed in to change notification settings - Fork 295
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.
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
add support for heatmaps #1547
add support for heatmaps #1547
Conversation
Builds on and replaces #1511. |
Adds a heatmap line style that can be used to render a set of lines on an axis as a heatmap instead of a line. There can only be a single heatmap on a particular axis. The `:heatmap` operator can be used to select this style. The cells for the heatmap are determined based on the step size for the time axis and the tick grid for the value axis. For normal lines each line that overlaps a given cell will contribute one to the count. For percentile approximations that would used with the `:percentiles` operator, they can now be used with `:percentiles-heatmap` to map the distribution to a heatmap. In this case the count for a cell will be determined by the distribution for the percentile approximation. Additional axis specific parameters have been added for `scale`, `u`, `l`, `palette`, and `label` with a prefix of `heatmap_`. The scale will control how counts are mapped to colors. Since a heatmap can combine many lines, the default legend test is just `Heatmap`. The `heatmap_label` parameter on the URL can be used to customize the label.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So much cleaner than my code, thanks! And thanks for catching the dark background issues.
I still want to do something with a ptile scale and I have a couple of questions around the legend but we can start with this.
private val counts: Array[Array[Double]] = computeCounts() | ||
|
||
/** | ||
* Min and max count for the heatmap. The min will always be 0. The max will be the largest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should min
always be zero? I could see a rare edge case where there is only one percentile or even a solid "wall" of data where every bucket has a value > 0. It might be nice to have the scale show the actual min in that case so users would know there was something in every bucket. See this example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a possible improvement for the future, but risks some ambiguity. In general for the lower/upper bounds on the heatmap I was a bit torn. One option that would be fairly consistent with the way it works on an axis would be to simply ignore cells that exceed the bounds. However, since it is visible that could be confused with no data for that interval which isn't really an issue with the normal graph bounds. So currently it uses a clamping behavior and essentially maps values that exceed a bound to that bound.
|
||
/** Set of ticks for the color scale used in legends. */ | ||
val colorTicks: ArraySeq[ValueTick] = { | ||
val numTicks = palette.colorArray.size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO bug we can fix later, there are a couple of things with palettes.
- If we have more colors in a custom palette than there are bucket ticks, we get an AIOOBE here.
- If there are, say 13 colors in the palette, then we just get the min and max but no values in between for the color legend. I have some code that can wrap the legend to show all the tick values if we want it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is just a copy paste bug in the json encoding, it should be using the label for the color tick there. Pushed a fix.
var cellMin = minValue | ||
var i = 0 | ||
while (i < yTicks.length) { | ||
val cellMax = yTicks(i).v |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still like to add a percentiles scale that creates cells with a consistent height and ticks that align with the bucket boundaries. That's what most percentile heatmap implementations do so users are expecting that. The current scale with a weighting makes it a bit confusing, e.g. the single percentile example has a smaller count for the top row and that makes it seem like more percentile buckets were returned than expected.
Users may also want a bit more fidelity around the lower latency bands. The linear scale aggregates a lot of data at the bottom while the buckets at the top are tall. Log scale doesn't work well either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can think about it for the future, but it is quite messy. The spacing is not uniform and was chosen to be mathematically convenient to compute the bucket and give a reasonable approximation, but that doesn't translate to being easy for a user to reason about. None of the ticks would be on convenient boundaries and the non uniform spacing means you couldn't really omit values to have a user reason about it so the axis gets crowded with major ticks and labels. I think we can see how usage goes and go from there.
val py2 = if (i == heatmap.yTicks.length) y1 else yscale(heatmap.yTicks(i).v) | ||
// Use white base to allow gradients based on alpha to show up clearly if the | ||
// overall background color is black | ||
g.setColor(Color.WHITE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah nice catch.
Adds a heatmap line style that can be used to render a set of lines on an axis as a heatmap instead of a line. There can only be a single heatmap on a particular axis. The `:heatmap` operator can be used to select this style. The cells for the heatmap are determined based on the step size for the time axis and the tick grid for the value axis. For normal lines each line that overlaps a given cell will contribute one to the count. For percentile approximations that would used with the `:percentiles` operator, they can now be used with `:percentiles-heatmap` to map the distribution to a heatmap. In this case the count for a cell will be determined by the distribution for the percentile approximation. Additional axis specific parameters have been added for `scale`, `u`, `l`, `palette`, and `label` with a prefix of `heatmap_`. The scale will control how counts are mapped to colors. Since a heatmap can combine many lines, the default legend test is just `Heatmap`. The `heatmap_label` parameter on the URL can be used to customize the label.
Adds a heatmap line style that can be used to
render a set of lines on an axis as a heatmap
instead of a line. There can only be a single
heatmap on a particular axis. The
:heatmap
operator can be used to select this style.
The cells for the heatmap are determined based
on the step size for the time axis and the tick
grid for the value axis. For normal lines each
line that overlaps a given cell will contribute
one to the count.
For percentile approximations that would used
with the
:percentiles
operator, they can nowbe used with
:percentiles-heatmap
to map thedistribution to a heatmap. In this case the
count for a cell will be determined by the
distribution for the percentile approximation.
Additional axis specific parameters have been
added for
scale
,u
,l
,palette
, andlabel
with a prefix ofheatmap_
. The scalewill control how counts are mapped to colors.
Since a heatmap can combine many lines, the
default legend test is just
Heatmap
. Theheatmap_label
parameter on the URL can beused to customize the label.