UI: Unify behavior for time ranges when min / max time is not available / applicable #4908
Conversation
matej-g
force-pushed
the
ui-store-min-time-fix
branch
from
f68e411
to
7bd36f2
Compare
|
IMO this is a step in the right direction but we also get a lot of questions about what the infinity symbols mean so I'd like to fix it in one go :/ perhaps it would be easy to add a tooltip for those icons saying something like "This node either has no information or is unable to synchronize its state"? We could add this if both min/max times are invalid. Maybe we could add this not even on the icons but on the address itself? Something akin to https://stackoverflow.com/questions/7117073/add-a-tooltip-to-a-div. Could we add this information in the |
I like this idea a lot, less confusing than just having icons. I think we could distinguish all cases, i.e. if no min and max time, we can provide information that the time range is unknown; if only one of the ranges is available, we can just say that the store has no max or min time limit. I'm not much of a front-end developer 😅, but I'll try to come back with a solution. |
|
@GiedriusS I made few more changes, as suggested:
|
matej-g
force-pushed
the
ui-store-min-time-fix
branch
from
358b84b
to
5c23d48
Compare
Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
- Change icon to 'minus' icon - Add tooltip with information on time range - Specify the min / max time is in UTC Signed-off-by: Matej Gera <matejgera@gmail.com>
matej-g
force-pushed
the
ui-store-min-time-fix
branch
from
5c23d48
to
14fc653
Compare
| 'Last Successful Health Check', | ||
| 'Last Message', | ||
| ]; | ||
|
|
||
| export const MAX_TIME = 9223372036854775807; | ||
| export const storeTimeRangeMsg = (validMin: boolean, validMax: boolean): string => { | ||
| if (!validMin && !validMax) { |
There was a problem hiding this comment.
I think it is important to distinguish between the cases:
- Time range is
MIN_TIME; MAX_TIMEe.g. when Prometheus still hasn't uploaded any blocks. In this case, Query will always send all queries to that StoreAPI; - Time range is
MAX_TIME; MIN_TIMEe.g. when Thanos Store has no blocks. In this case, Query will not send any queries to it.
As is right now, both stores would show up with minuses in the Store page and with the same tooltip. I suggest at the minimum to change this function to distinguish between these two states so that it would be clear what's happening. Perhaps something like:
"This store's time range data is not available but we will send all queries to it"? Or maybe we could make the minuses different colors depending on the state? For example, the minuses could be green if we will still send all queries to it. Or perhaps the node's address could have a different color depending on the state e.g. green = we will send all queries to it, yellow = we will send certain queries to it depending on the labels & min/max time, red = no queries will be sent to it?
WDYT @matej-g ?
There was a problem hiding this comment.
Hm, I'm wondering whether min / max time ranges and the information whether we are querying the store are two separate things. For the min / max, I would expect simply to have the information stated in the dashboard if it is available - if not, regardless of the reason (i.e. no block uploaded or no block in store), we just signal that this information cannot be showed because it is unknown.
But I think you're onto something with color coding the nodes (perhaps we could even add another column, besides Status and add details on querying?), although it feels like a separate feature.
There was a problem hiding this comment.
ping @GiedriusS, how do you suggest to proceed? Do you agree with the suggestion and if so, should we do this in the current PR?
There was a problem hiding this comment.
Mhm, I guess you are correct. Whether or not we are sending queries to a node is different information altogether and we could work on this in future pull requests. 👍
|
Could you please merge main into your branch? |
|
Done, thanks for the review! I also opened a separate issue for that color-coding idea. |
Signed-off-by: Matej Gera matejgera@gmail.com
Changes
Components which expose store API behave somewhat differently when it comes to returning store time range information, where for example:
math.MinInt64or empty response, i.e.00000-01-01T00:00:00ZThis means that on the actual UI, we can get various results, ranging from
Invalid dateto0000-01-01 00:00:00- which all signalize the same thing: the information is not available or not known.My suggestion here is to:
1-(math.MaxInt64 - 1)If people find the icon confusing, possible alternative would be to use a different icon or some default, minimum time, like 0 UNIX timestamp.
Verification
UI tests have been adjusted. I also did few manual checks with the interactive tests.