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

Explore: Loading query field takes too long and allows typing in it before it has loaded #52213

Open
polibb opened this issue Jul 14, 2022 · 14 comments
Labels
area/explore area/frontend prio/medium Important over the long term, but may not be staffed and/or may need multiple releases to complete. type/bug

Comments

@polibb
Copy link
Contributor

polibb commented Jul 14, 2022

What happened:

On the Explore page, the field for a query loads after a long while and deletes all that has been written in it before it has loaded fully. It takes around 10 seconds sometimes.

On the video: Reload the page. Change the datasource. Type in some random "query" just for the example. See the field load fully and delete your progress in the process.

Screen.Recording.2022-07-13.at.12.19.32.mov

What you expected to happen:

The field for the query either loads immediately, or it preserves the query I have written already once it the field has fully loaded.

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

Open the Explore page and go to Code tab for the query operation.
Change the datasource and start typing a query for it. The field for a query loads after a while and deletes all that has been written in it before it has loaded fully.

Anything else we need to know?:

Discussed that it is an issue with @dimitarvdimitrov and is loading so slowly on his system.

Environment:

  • Grafana version: 9.0.3
  • OS Grafana is installed on: macOS 12.4, arm64
  • User OS & Browser: Chrome 103.0.5060.114
@polibb polibb changed the title Explore: Loading query field for Log browser takes too long and allows typing in it before it has loaded Explore: Loading query field takes too long and allows typing in it before it has loaded Jul 14, 2022
@Elfo404
Copy link
Member

Elfo404 commented Jul 20, 2022

does the same thing happen also in dashboards? i suspect this specific to the plugin itself, not Explore.
cc. @grafana/observability-logs-and-traces

@ivanahuckova ivanahuckova added type/bug prio/medium Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Jul 20, 2022
@gabor gabor self-assigned this Jul 20, 2022
@gabor
Copy link
Contributor

gabor commented Jul 21, 2022

what seems to be happening, is that when switching the datasource, we try to apply the query from the "previous" datasource (like, switching from prometheus to loki), and somehow we do it after label-browser has loaded, so if you enter something while the label-browser is loading, things get overwritten when it loads.

will look more into this.

@dimitarvdimitrov
Copy link

does the same thing happen also in dashboards? i suspect this specific to the plugin itself, not Explore. cc. @grafana/observability-logs-and-traces

only happens in explore, not in dashbaords.

when switching the datasource

I can confirm this only happens when switching the datasource. If I open a new tab with the loki datasource and start typing a query before the labels have loaded, my query does not get deleted.

@gabor
Copy link
Contributor

gabor commented Jul 25, 2022

i did some more investigation, and the bug seems to be in the generic grafana-explore-mode-code here: https://github.com/grafana/grafana/blob/main/public/app/features/explore/state/query.ts#L262-L295

what happens is, when switching between datasources, this part of the code is responsible for trying to represent the "query in previous datasource" somehow in this datasource. it calls various async methods on the datasources, and at the end sets a new query-expression based on what the datasources return. the problem happens when these events happen:

  1. we switch to a different datasource
  2. importQueries starts, and calls some async methods
  3. while those async methods are running, the user enter some query in the query-field
  4. now [2] finishes, and overwrites the user's output with the result of [2]

to trigger this, the easiest i have found is to add some "sleep for 10seconds" code around https://github.com/grafana/grafana/blob/main/public/app/features/explore/state/query.ts#L290, something like this should do it:

    await new Promise((resolve) => {
      setTimeout(resolve, 5000);
    });

@grafana/observability-experience-squad could you take a look please?

@gabor gabor removed their assignment Jul 25, 2022
@Elfo404
Copy link
Member

Elfo404 commented Jul 25, 2022

thanks for taking a look @gabor, my mistake. we'll look into this better.

@gelicia gelicia added the needs investigation for unconfirmed bugs. use type/bug for confirmed bugs, even if they "need" more investigating label Jul 26, 2022
@ifrost ifrost self-assigned this Sep 16, 2022
@ifrost
Copy link
Contributor

ifrost commented Sep 16, 2022

I wasn't able to reproduce it in the latest version. I think that with recent changes to mixed data sources in Explore the behavior changed and the editor is loaded only after queries are imported. In the case of Loki, importing queries may be slow if the request for getting labels is slow (Loki requires labels for importing to skip labels that do not exist). On the other hand, labels are cached so it's going to happen only once.

A side effect of recent changes is that Explore shows the previous/stale editor until importing is complete (see video below). Maybe we should show the user that the query builder is loading? One more option is to skip importing if it takes too long, but maybe better would be better to add some logic to Loki plugin to skip label verification if it takes too long and simply use all provided labels.

Screen.Recording.2022-09-16.at.14.32.55.mov

@dimitarvdimitrov
Copy link

this is still happening for me on v9.2.0-33906pre (469f915). Just tried it on an internal grafana instance. Can share this in a DM.

Is it possible to just abandon the labels lookup if the user has typed something? Or if the initial prometheus query is empty?

@ifrost
Copy link
Contributor

ifrost commented Sep 19, 2022

Is it possible to just abandon the labels lookup if the user has typed something? Or if the initial prometheus query is empty?

Yes, this could possibly be done in Loki plugin (cc @grafana/observability-logs)

@gabor
Copy link
Contributor

gabor commented Sep 19, 2022

@dimitarvdimitrov sorry, could you explain in more detail, what is still happening for you?

thanks!

@dimitarvdimitrov
Copy link

I was replying to @ifrost's comment:

I wasn't able to reproduce it in the latest version.

Whatever I typed into the query field after changing the datasource is still getting erased after the query is imported.

@gabor
Copy link
Contributor

gabor commented Sep 19, 2022

@dimitarvdimitrov thanks for the info.. could you doublecheck something please?

you see, both behaviours end the same way, except a minor detail:
(let's say you have two datasources, named "X" and "Y").

A (the original report):

  • you open datasource X
  • you switch to datasource Y
  • at this point, the user-interface already switched to datasource-Y (this is the difference)
  • you enter something in the query field
  • after some time it is erased

B (what @ifrost described):

  • you open datasource X
  • you switch to datasource Y
  • at this point, the user-interface still displays datasource X (this is the difference)
  • you enter something in the query field
  • after some time, the user-interface switches to display datasource Y, and the query field is erased

when you are switching between datasources of the same type, like Loki datasources, case A and case B looks very similar. could you please check, if you are seeing case A or case B?

thanks!

@dimitarvdimitrov
Copy link

Now I see what you were asking.

I'm not sure how to tell if the UI changed the datasource. The dropdown in the top left changes immediately. Is it the text in parenthesis next to the query ID?
Screenshot 2022-09-19 at 17 28 48

If yes, then I am observing the original report. I start with an empty query, change the datasource from the dropdown, the text in parenthesis changes, I write something in the query field, after some time my query is erased.

I also reproduced by starting with a non-empty query. In this case after some time, the UI doesn't erase my new query, it reverts it to the importer version of my starting query.

Should I file a new report for the issue ifrost showed? In my mind it's the same user experience, that's why I didn't realize there was a difference.

@ifrost
Copy link
Contributor

ifrost commented Sep 20, 2022

We had a chat with about it with @gabor. We don't want to skip label validation in Loki to workaround this problem, but rather find a better flow for query imports. I've created an issue for it here #55462, once implemented it should fix the problem.

@ifrost ifrost removed the needs investigation for unconfirmed bugs. use type/bug for confirmed bugs, even if they "need" more investigating label Sep 20, 2022
@ifrost ifrost removed their assignment Sep 20, 2022
@ifrost ifrost self-assigned this Jan 20, 2023
@ifrost
Copy link
Contributor

ifrost commented Jul 7, 2023

Should be fixed by #62747

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/explore area/frontend prio/medium Important over the long term, but may not be staffed and/or may need multiple releases to complete. type/bug
Projects
None yet
Development

No branches or pull requests

8 participants