grafana / explore-traces

Opinionated traces app
Apache License 2.0
9 stars 0 forks source link

Consider all series when finding the max y axis #236

Closed joe-elliott closed 2 days ago

joe-elliott commented 2 days ago

Fixes https://github.com/grafana/explore-traces/issues/213

Will take advice if there's a better way to do this, but this solves this issue:

![Uploading image.png…]()

adrapereira commented 2 days ago

Hmm, it's weird that this fixes it because the end result of the code running is the same between the old and the new version. The only difference is that in the proposed changes we first find the max for the timestamps and then override them with the max for the values.

joe-elliott commented 2 days ago

So I could very much be wrong on what this change does, but to explain my reasoning.

image

It seems that as is the code only considers the values in fields[1] which is the error series. This should check fields[1] through fields[N]? and therefore check the values in the unset series and any others that may be present?

adrapereira commented 2 days ago

You're absolutely right, my bad! Misread slice(1) as taking elements 0 and 1, when it actually takes elements starting at 1. Thanks for the fix ❤️