Skip to content
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

Series order should be guaranteed by the SDK #366

Closed
srclosson opened this issue Jun 3, 2021 · 7 comments · Fixed by #425
Closed

Series order should be guaranteed by the SDK #366

srclosson opened this issue Jun 3, 2021 · 7 comments · Fixed by #425
Assignees
Labels
bug Something isn't working

Comments

@srclosson
Copy link
Member

What happened: This bug is probably debatable... So IMHO

Many plugins don't order the series, and many customer endpoints do not return series in the same order either. When data arrives at the panel, it's confusing when series change order because the "green" trend is always changing, or if you are viewing data in the table panel, the series, or the order of the fields in the series will change.

What you expected to happen:

The SDK should guarantee some sort of ordering in the series it's being given. In the case of timeseries data, if panels are always looking for the "time" field to be the first series or "column", then the SDK should ensure this ordering exists.

From a users perspective, my "green" trend should always stay green.

Alternatively, if multiple queries are used, then the order of those queries in the editor should be retained.
(Ie: if you have a refID of "A", "B", and "C", The "A" query results should be ordered first.

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

I used the splunk plugin, and this query. Any panel will show the issue:

The three queries were:

| mstats avg(_value) prestats=true WHERE metric_name="cpu.idle.value" AND index="grafana_metrics_index" span=10s
| timechart avg(_value) AS CPUIdle span=60s
| fields - _span
| mstats avg(_value) prestats=true WHERE metric_name="cpu.system.value" AND index="grafana_metrics_index" span=10s
| timechart avg(_value) AS CPUSystem span=60s
| fields - _span
| mstats avg(_value) prestats=true WHERE metric_name="cpu.user.value" AND index="grafana_metrics_index" span=10s
| timechart avg(_value) AS CPUUser span=60s
| fields - _span

Environment:

@srclosson srclosson added the bug Something isn't working label Jun 3, 2021
@kminehart
Copy link
Contributor

kminehart commented Jun 3, 2021

Wouldn't this be sort of a problem with how you might be processing results before putting them into a data frame?

If I get a response like this from some serivce,

b := `{
    "results_A": [...],
    "results_B": [...]
}`

And if you unmarshal that as a map,

m := map[string]interface{}

json.Unmarshal(m, b)

and then loop over those results:

for k, v := range m {
    // use the map key `k` as the name of the field
    data.NewFrame("frame", data.NewField(k, v...))
}

you're going to end up with unordered results because maps are unordered in Go, and looping over them can yield a different result each time.

@elliotdobson
Copy link

We just recently updated from Grafana v7.3.7 to v8.0.1 and noticed that this has started happening on our panels that are using CloudWatch as their datasource.

Alternatively, if multiple queries are used, then the order of those queries in the editor should be retained.
(Ie: if you have a refID of "A", "B", and "C", The "A" query results should be ordered first.

I'm fairly confident that this is how it was in v7.3.7. Now in v8.0.1 it's just random.

We didn't test any version in between the two that I've mentioned so not sure what version it started happening in exactly.

@JesperCph
Copy link

@kxwsd

First noticed this issued from version 8.0.0. Also using the CloudWatch datasource. For the moment it renders most of my dashboards worthless.

I’m considering rolling back to version 7.3.8.

@kminehart
Copy link
Contributor

@kxesd @JesperCph This might be an isssue specifically in the CloudWatch datasource since it's shipped with Grafana. I would open an issue in github.com/grafana/grafana

@leehuk
Copy link

leehuk commented Jul 13, 2021

@kminehart This was opened against the cloudwatch datasource and closed as an SDK issue: grafana/grafana#35944

@hammond756
Copy link

My issue on the Grafana repo was closed because it is marked as a duplicate of this issue. Will this be be picked up as part of the sdk?

@sunker
Copy link
Contributor

sunker commented Nov 18, 2021

We now have three issues related to the CloudWatch plugin that I think are symptoms of this problem.
grafana/grafana#40011 (thanks @hammond756)
grafana/grafana#41773
grafana/grafana#41303

It's not the order of the timeseries in one given query that is non-deterministic - it's the order of the returned queries.

My understanding is that the SDK should guarantee that order of refIds in backend.QueryDataResponse should be the same as the order of refIds in backend.QueryDataRequest. This does not seem to be the case. I can reproduce this locally using the CloudWatch plugin, but also using Prometheus and CloudMonitoring. It seems like the order is incorrect every tenth time or so.

See this example using CloudWatch.
timeseries-order-meta-inspector

cc @grafana/plugins-platform-backend

@marefr marefr added this to Backlog in Plugins Platform (DEPRECATED) via automation Nov 18, 2021
@marefr marefr self-assigned this Nov 18, 2021
@marefr marefr moved this from Backlog to In review in Plugins Platform (DEPRECATED) Nov 18, 2021
Plugins Platform (DEPRECATED) automation moved this from In review to Done Nov 18, 2021
marefr added a commit that referenced this issue Nov 18, 2021
This will make sure that the order of query responses (refIDs) are always 
stable (increasing order, e.g. A, B, C even if query requests has the order C, A, B.

Fixes #366
sunker pushed a commit that referenced this issue Jan 26, 2022
This will make sure that the order of query responses (refIDs) are always 
stable (increasing order, e.g. A, B, C even if query requests has the order C, A, B.

Fixes #366
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

8 participants