Skip to content

Commit

Permalink
chart: fix multi-Y limit inconsistency (#1687)
Browse files Browse the repository at this point in the history
When explicitly setting the axis it was inclusive of the
limit value meaning you could actually use one more than
the max setting. Settings like axis per line were honoring
the actual limit. Fixed them to be consistent and bumped
the configured limit by 1 for backwards compatibility with
any uses of the additional explicit axis.
  • Loading branch information
brharrington authored Aug 21, 2024
1 parent f753099 commit 1872ec2
Show file tree
Hide file tree
Showing 31 changed files with 29 additions and 3 deletions.
2 changes: 1 addition & 1 deletion atlas-chart/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ atlas {
max-height = 1000
max-zoom = 2.0
max-lines-in-legend = 50
max-yaxes = 4
max-yaxes = 5
}
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ case class Grapher(settings: DefaultSettings) {
.getOrElse(Features.STABLE)

import com.netflix.atlas.chart.GraphConstants.*
val axes = (0 to MaxYAxis).map(i => i -> newAxis(params, i)).toMap
val axes = (0 until MaxYAxis).map(i => i -> newAxis(params, i)).toMap

val vision = params.get("vision").map(v => VisionType.valueOf(v))

Expand Down Expand Up @@ -311,7 +311,7 @@ case class Grapher(settings: DefaultSettings) {

val plots = plotExprs.toList.sortWith(_._1 < _._1).map {
case (yaxis, exprs) =>
val axisCfg = config.flags.axes(yaxis)
val axisCfg = config.flags.getAxis(yaxis)
val dfltStyle = if (axisCfg.stack) LineStyle.STACK else LineStyle.LINE

val statFormatter = axisCfg.tickLabelMode match {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,11 @@ case class ImageFlags(
case None => settings.primaryPalette(theme)
}
}

def getAxis(index: Int): Axis = {
axes.get(index) match {
case Some(axis) => axis
case None => throw new IllegalArgumentException(s"invalid axis: $index")
}
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,25 @@ class GrapherSuite extends FunSuite {
"&ylabel.0=Axis%200&ylabel.1=Axis%201"
}

imageTest("multi-Y, 5 axes") {
"/api/v1/graph?e=2012-01-01T00:00&q=(,0,1,2,3,4,),(,:dup,:axis,),:each"
}

test("multi-Y, 6 axes") {
val uri = "/api/v1/graph?e=2012-01-01T00:00&q=(,0,1,2,3,4,6,),(,:dup,:axis,),:each"
intercept[IllegalArgumentException] {
grapher.evalAndRender(Uri(uri), db)
}
}

imageTest("multi-Y, 5 axis per line") {
"/api/v1/graph?e=2012-01-01T00:00&q=0,1,2,3,4&axis_per_line=1"
}

imageTest("multi-Y, 6 axis per line") {
"/api/v1/graph?e=2012-01-01T00:00&q=0,1,2,3,4,6&axis_per_line=1"
}

imageTest("significant time boundaries and tz=US/Pacific") {
"/api/v1/graph?q=name,sps,:eq,:sum&s=e-2d&e=2015-06-17T13:13&no_legend=1&tz=US/Pacific"
}
Expand Down
Binary file modified atlas-webapi/src/test/resources/GraphApiSuite/42bb9a56.png
Binary file modified atlas-webapi/src/test/resources/GraphApiSuite/7a906f27.png

0 comments on commit 1872ec2

Please sign in to comment.