Timeseries to table transformation: Update Output Changes

[codeincarnate]

What is this feature?

When addressing issues with the Time Series to table transformation we accidentally broke backwards compatibility. This PR attempts to address that while still fixing the underlying issue. Namely that multiple series being returned from a single query were not being handled correctly.

This can be seen with the React Table gdev-dashboard which previously would only show one row, even though it should be showing 3 as each query has 3 series returned.

This also returns the functionality of breaking out labels into individual fields.

Why do we need this feature?

To preserve backwards compatability of this transform while still working for cases with multiple series.

Please check that:

• It works as expected from a user's perspective.
• If this is a pre-GA feature, it is behind a feature toggle.
• The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

[domasx2]

Tested, app o11y back to normal 👍

Namely that multiple series being returned from a single query were not being handled correctly.
This can be seen with the React Table gdev-dashboard which previously would only show one row, even though it should be showing 3 as each query has 3 series returned.

It was definitely handling multiple series from a single query. I think I get it now, the case that it wasn't handling is multiple series with the same exact label values. Did not know that can happen with "real" data sources

Not related to app o11y, but looks weird:

It seems it's picking up all time fields of all data frames, ending up with at least as many identical options as there are series returned. I'm guess the intention here is to handle the case when there are multiple time fields in a single data frame, so should probably only contain distinctly named fields

[torkelo]

Would be good to restore the unit tests changed in #76801

and add additional tests for the new scenarios/cases added in that PR

I do agree that the transform could be improved for queries that return multiple series with same labels (or no labels) but different display names (For value field), the old behavior there was too limited to only look at labels and not also take into account display name for the value field (or frame name).

[codeincarnate]

Thanks for the feedback @domasx2 and @torkelo 🙂. I'm going to be getting this implemented asap.

[codeincarnate]

I put the original tests back in with very slight updates to some of the tests, in particular the order of frames is coming out differently than originally. I don't believe this has an impact beyond the indexes for referencing the results.

@domasx2 I did have a question about this test. I'm a bit surprised this is expecting two frames. Seems like it should be one frame since incoming frames are mapped onto fields instead. Feel like I'm missing something here?

[domasx2]

I'm a bit surprised this is expecting two frames. Seems like it should be one frame since incoming frames are mapped onto fields instead. Feel like I'm missing something here?

It maps each refId (query) onto a separate data frame. Same behavior as with prometheus "table" queries. Different queries may have separate, even non overlapping, sets of labels. It may not make sense to smush them into a single table. But "join" or "merge" transforms can be used to further merge into a single data frame based on some join criteria if wanted

[codeincarnate]

Thanks for the feedback @domasx2. Working through that now

[codeincarnate]

Worked out the tests and I believe that this should be good to go now

Oops: realized I forgot about the time field selection. Looking at that right now

[codeincarnate]

Added code to restrict time field selection as well. It assumes that all time fields within frames with the same refId are the same. This may not always be true, but seems like it should be in the majority of circumstances. Would definitely appreciate another look @domasx2 and @torkelo 🙏

[bfmatei]

Hey @codeincarnate !

I don't have the context for reviewing the code but I would like to mention that I did test out your branch w/ the app observability plugin and it seems that the fix is working fine for our cases.

[domasx2]

please merge ASAP, it's currently broken for customers

[mdvictor]

LGTM

[oscarkilhed]

Tested this out, and it seem to work as expected.

[mdvictor]

@torkelo would you like to add anything to this or are we good to merge?