grafana / grafana

The open and composable observability and data visualization platform. Visualize metrics, logs, and traces from multiple sources like Prometheus, Loki, Elasticsearch, InfluxDB, Postgres and many more.
https://grafana.com
GNU Affero General Public License v3.0
64.7k stars 12.11k forks source link

Missing begin/end on 'step after' graphs #60956

Open sciurius opened 1 year ago

sciurius commented 1 year ago

What happened:

Assume a graph with Graph Style > Line Interpolation 'Step After'.

The graph only shows from the first point in range to the last point in range. Nothing is shown between the start of the range and the first point. Likewise, nothing is shown between the last point and the end of the range.

What you expected to happen:

A 'Step After' style graph is typically used to show the current value of an entity. As long as the value does not change it stays the same. At the start of the time range of the graph there may not be a data point for this entity but nevertheless if there were data points before the time range, it does have a valid value.

Likewise, after the last data point the value should persist until the end of the range, or the current time, whichever comes first.

scrot20230104133802

On the left side, the first data point shown is 08:00,10. The value before 08:00 is not shown.

On the right side, the last data point shown is 12:00,50. The value at 13:00 (=current time) is still 50 but not shown.

How to reproduce it (as minimally and precisely as possible):

I used a Step After graph with this simple CSV:

time,value
2023-01-04 00:00:00,5
2023-01-04 08:00:00,10
2023-01-04 09:00:00,20
2023-01-04 10:00:00,30
2023-01-04 11:00:00,40
2023-01-04 12:00:00,50

Environment:

mellieA commented 1 year ago

@sciurius is this what you expect to see? This is with Grafana 9.3.2 and the time series visualization. I had to update your data to make it UTC time format (it wasn't recognized as a timestamp for me) then adjusted the time range to match the one that you used in your example:

time, value
2023-01-04T00:00:00Z,5
2023-01-04T08:00:00Z,10
2023-01-04T09:00:00Z,20
2023-01-04T10:00:00Z,30
2023-01-04T11:00:00Z,40
2023-01-04T12:00:00Z,50

Screenshot 2023-01-05 at 5 46 03 PM

mellieA commented 1 year ago

I did a little more research and it looks like it was fixed in 9.4 and backported to 9.3 in https://github.com/grafana/grafana/pull/59444

these bits will cause stepped interpolation to extend beyond the data to the edges of the plotting area

Setting to needs more info to wait for confirmation that this is the fix you're looking for

sciurius commented 1 year ago

Great, this fixes the final/end value. Unfortunately the value at the left is still missing. This can not be seen in your example, since you start the time range at 08:00 and there is a data point at 08:00. If you start the time range at 07:00 (as I did) you'll see that the line drops below the graph but does not show the actual value. You can see this even better with these data:

time,value
2023-01-06T00:00:00Z,5
2023-01-06T08:00:00Z,100
2023-01-06T09:00:00Z,120
2023-01-06T10:00:00Z,130
2023-01-06T11:00:00Z,140
2023-01-06T12:00:00Z,150
leeoniya commented 1 year ago

this is expected behavior since your first datapoint (00:00) is outside the view (07:00). the y axis range is always determined from datapoints within the zoomed time range, so your first in-view value connects to the previous off-scale low value.

sciurius commented 1 year ago

This is precisely what this is about. Even though the first datapoint is off-range, the 'step after' entity does have a defined value at the start of the range.

leeoniya commented 1 year ago

this is a tricky one. you can say the same for other interpolation types (smooth, linear). the issue is that the interpolation is done automatically by the painting code (canvas .lineTo()), not in the data. we would have to introduce in-data interpolation just at the edges during y ranging, and this would only affect a small set of use cases where a query returns data outside the zoom range.

the interpolation logic would have to be added into this function. it would have to account for all interpolation types, as well as all scale distributions (linear, log2, log10, symlog).

https://github.com/grafana/grafana/blob/c2ad447f8cccb3f666308fff90c08e975d2c699b/packages/grafana-ui/src/components/uPlot/config/UPlotScaleBuilder.ts#L86

sciurius commented 1 year ago

FYI, https://community.grafana.com/t/showing-a-current-value/76605.

leeoniya commented 1 year ago

When a graph is displayed for the period november 22 0:00 - 24:00 nothing is displayed as there are no data points.

that was actually also addressed by the PR @mellieA linked above.

https://github.com/grafana/grafana/pull/59444#issuecomment-1329941472

i guess rather than doing partial interpolation, we can expand the range backwards when step after or forwards when step before. the main question is whether people would be okay with this only being done with stepped interpolations.

UlrichThiess commented 1 year ago

Did this has changed between 9.2.x and 9.3.x and above? I use "Today" as time range. Hope this will fixed soon.

9.4.0beta-1 grafik

9.2.10 grafik

amarant commented 1 year ago

All the graph with line interpolation parameter set to step after now show an interpolation going forward to the end of the time line, this is a breaking change and it is very misleading and doesn't work accordingly to the connect null value parameter with never or threshold value, I want to know when is the last data point, please if you want to keep this wrong behavior at least add a parameter to choose between extend or not. For now I would have to use step before and transform time in the query but it would now extend before the first data point backward to the from time.

UlrichThiess commented 1 year ago

Still happend in 9.5.0-102803pre - goin back to 9.2.10 (no LoginBug, no broken TimeSeries) Why did you changed this from 9.2.10 to 9.3x? It's a bug, isn't it? Because if there is no data, it should not display any line. Thats simple.

sciurius commented 1 year ago

Because if there is no data, it should not display any line.

True, except for 'step after'. It steps and stays. However, the line should not exceed the end of the active time interval.

UlrichThiess commented 1 year ago

If no data points are recorded, then nothing should be displayed. This applies to all display types and is also symbolized as such in the buttons.

In particular, it is an error if there is an empty data line in the read data for the times. Again: no data, no representation.

If the data is obtained via flux, the data can be filled with |> fill(usePrevious: true ). This method is also better when the data groups are broken, so it continues between the values.

9.2.10 with fill(usePrevious: true) grafik

9.5.0-102803pre without fill(usePrevious: true) grafik

amarant commented 1 year ago

The "step after" is an interpolation, so it should not interpolate more than the "Connect null values" "threshold" duration, or another setting to add to limit the interpolation.

For example I have 2 series with different last value timestamps, how can I show that if the line continue ? It seems the value is constant for the one with missing values.

image image

sciurius commented 1 year ago

Maybe we need a different type of 'step after'? I have several sensors that only report value changes. As long as there is no new value, the current value stays current.

UlrichThiess commented 1 year ago

That's exactly what I mean: Use fill(useprevious:true) and everything is fine. Alternatively, a trigger could help update the data. Or the data source needs to be adjusted.

As said, if there is no data in the table, then there should be no display. A representation of non-existent data is deceptive and inevitably leads to errors in interpretation. Imagine the sensor fails, then a value would still be displayed. That would be a disaster. In the nuclear power plant one would think that the cooling water is still there. With vehicle data, you would think the engine is still running. ...

The representation must always be the same, no matter which form is chosen. What should it look like when I switch from Time Series to Bar Chart?

amarant commented 1 year ago

It is easy to add a "Last Observation Carry Forward" with a time bucket using just the data source, but now it is impossible to show the absence of value, even if I had a "null" value 1h after my last value for each metric Grafana ignore that and continue the line.

UlrichThiess commented 1 year ago

Yes thats the Problem. So if there is no value, i don't want to see a graph. Because it could be, that the sensor is dead. If you want to see the last value as long as the measurement of other sensors is, just add a trigger and fill the missing values.

UlrichThiess commented 1 year ago

@mellieA What do you think about?

UlrichThiess commented 1 year ago

This Bug ist still in Version 9.4.7

mbtx2 commented 1 year ago

Agree that this is a problem, and a change in behavior from 9.2.10 to 9.3.x that broke many of our graphs. To @UlrichThiess and @amarant's point, there is now no way to show the absence of value.

The same thing happens with "step before" with nulls preceeding. That also shouldn't happen.

The fix for this problem: https://github.com/grafana/grafana/pull/59444 caused the issue as previously stated. But it's not a good solution for all of the aforementioned reasons. If the data is null in the next interval before or after the panel range, the line should not extend. If there is some value in the next (or previous) interval, then maybe it should interpolate to that value although I could see why that is a rendering challenge since the time of that next data point may be far in the future. However, if it's null or nonexistent, then it should not extend.

Perhaps there's some way this could be a choice for display, but if that is done, then default behavior should be the old behavior to make sure it's not a breaking change, as it is now.

UlrichThiess commented 1 year ago

Best @torkelo makes a decision here?

torkelo commented 1 year ago

I think this feels like a data transformation question (group by time, keep previous value if no value). I don't think the graph should draw a line to right edge just because you have setup steps interpolation, all the interpolations should behave the same in regards to how they render gaps in the data

UlrichThiess commented 1 year ago

Thank you @torkelo for this clarification. So I may consider this as a bug.

I would be very happy if this would work again in version 9.5.0 like in version 9.2.x.

@sciurius I'm happy to help design your queries the way you want to see them.

sciurius commented 1 year ago

Thanks @Ulrich for the offer. I managed to get the results I want by creating a UNION with a dummy SELECT that adds the last value at the current timestamp. But the queries become ugly. Fortunately the queries do not need much maintenance. Once they work you can forget them ☺.

leeoniya commented 1 year ago

there are two different issues being discussed here regarding stepped interpolation:

  1. y scale auto-range not accounting for datapoints outside the visible x range (the subject of the OP)
  2. interpolation from start or end of data to the edges of the plotting area

The "step after" is an interpolation, so it should not interpolate more than the "Connect null values" "threshold" duration, or another setting to add to limit the interpolation.

this is about point 2. this was changed in https://github.com/grafana/grafana/pull/59444#pullrequestreview-1196693720. the context for that change is https://github.com/grafana/iot-sitewise-datasource/issues/139#issuecomment-1287619249.

looking at it again, i think we can revert the extend: true settings since we now expand the query range to include datapoints outside the view window for iot-sitewise.

https://github.com/grafana/grafana/blob/940768cf76cc5dbf6df3bbbf5655b680300a1cf0/packages/grafana-ui/src/components/uPlot/config/UPlotSeriesBuilder.ts#L253-L254

XANi commented 1 year ago

Just to illustrate: current (9.4.7) way it works causes it to extend indefinitely to the past: image

In this case there is no data point before last day (fresh DB)

If there is some value in the next (or previous) interval, then maybe it should interpolate to that value although I could see why that is a rendering challenge since the time of that next data point may be far in the future. However, if it's null or nonexistent, then it should not extend.

I think that if option to interpolate beyond time range was needed then:

leeoniya commented 1 year ago

the extend-to-edges behavior has now been reverted on main and will land in 9.5.0 (see linked PR). we'll consider adding an option for this in the future, but i suspect there won't be sufficient demand for it to justify adding more panel settings.

github-actions[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had activity in the last year. It will be closed in 30 days if no further activity occurs. Please feel free to leave a comment if you believe the issue is still relevant. Thank you for your contributions!

kaspermarkus commented 2 months ago

the extend-to-edges behavior has now been reverted on main and will land in 9.5.0 (see linked PR). we'll consider adding an option for this in the future, but i suspect there won't be sufficient demand for it to justify adding more panel settings.

The original reason for this issue was actually exactly that, a request for extend-to-edges. I for one need the option (and @alextwomey needs it as well, it seems from his reaction). I dont know how you measure demand, but if there is an open issue for adding that option to panel settings, a link to it would be helpful so one could express the need there