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

Query/UI: state is shared between panels #2578

Closed
GiedriusS opened this issue May 8, 2020 · 5 comments · Fixed by #2707
Closed

Query/UI: state is shared between panels #2578

GiedriusS opened this issue May 8, 2020 · 5 comments · Fixed by #2707

Comments

@GiedriusS
Copy link
Member

GiedriusS commented May 8, 2020

Deduplication / partial responses set a variable that is shared between two or more panels even though the button elements are separate and nearby each of the panels.

Thanos, Prometheus and Golang version used:

thanos, version 0.12.2 (branch: HEAD, revision: c21b548)
build user: gstatkevicius@gstatkevicius-desktop
build date: 20200508-15:45:24
go version: go1.14.2

Object Storage Provider:

N/A

What happened:

Thanos Query sends HTTP requests with incorrect parameters.

What you expected to happen:

Thanos Query executes HTTP requests with parameters that are set near to the query.

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

Add two or more panels. Select differing options on each panel. Click "Execute". Watch that parameters are set according to the last modification, not what they actually are near to the panel.

image

In this case, I have disabled "Partial Response" on the second panel but it got applied to first one as well as you can see in the screenshot.

@GiedriusS GiedriusS changed the title Query/UI: state is not shared properly between panels Query/UI: state is shared between panels May 8, 2020
@ranjithkumar007
Copy link
Contributor

ranjithkumar007 commented May 9, 2020

When we click on execute, params for this query are loaded here, which goes on to check the localStorage value of enable-partial-response item here. But this item enable-partial-response in localStorage is same across all panels. Similar issue with dedup.
A quick fix could be instead of using enable-partial-response as item, we can use enable-partial-response + self.id as a key in localStorage. I tested it locally with this change and it worked,
I don't have any experience with web-dev. I am not sure if this is an elegant way of fixing the issue.

@GiedriusS
Copy link
Member Author

When we click on execute, params for this query are loaded here, which goes on to check the localStorage value of enable-partial-response item here. But this item enable-partial-response in localStorage is same across all panels. Similar issue with dedup.
A quick fix could be instead of using enable-partial-response as item, we can use enable-partial-response + self.id as a key in localStorage. I tested it locally with this change and it worked,
I don't have any experience with web-dev. I am not sure if this is an elegant way of fixing the issue.

That's probably a good to solve it. 👍 Also, perhaps it would be worth checking if upstream Prometheus has the same issue and if the new UI has it too? I haven't checked myself. Then, we could fix this there and everyone would benefit, not just Thanos.

@ranjithkumar007
Copy link
Contributor

Thanks for the suggestion. I will look if Prometheus has the same issue.

@ranjithkumar007
Copy link
Contributor

I checked Prometheus with regular and new react UI. I didn't find any shared state issue with either of them. As highlighted in the images below, each param of different graphs is assigned to different variables(eg: g0.stacked, g1.stacked).

normal_ui

react_ui

@ranjithkumar007
Copy link
Contributor

ranjithkumar007 commented May 9, 2020

We had issue in this file. So, I looked at relevant prometheus source code here.

But this item enable-partial-response in localStorage is same across all panels.

In contrast to the above, prometheus index.js used localStorage for only enable-query-history and history. So, I think there should not be any issues with Prometheus UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants