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

Service selection: Replacing datasource resource calls with query runners #651

Merged
merged 27 commits into from
Aug 2, 2024

Conversation

gtk-grafana
Copy link
Contributor

@gtk-grafana gtk-grafana commented Jul 31, 2024

Replacing direct datasource resource calls with runtime datasource to provide sceneQueryProvider.

This will prevent duplicate calls from being made until the user changes a variable or time range that the query depends on.

This PR refactors the service selection scene to call the volume API using a scene query provider. On routing back to the service route, the volume API will not be re-called unless 10% of the time range has elapsed since the time range was activated (for relative queries).

@gtk-grafana gtk-grafana changed the title WIP: Replacing datasource resource calls with query runners Service selection: Replacing datasource resource calls with query runners Aug 1, 2024
@gtk-grafana gtk-grafana marked this pull request as ready for review August 1, 2024 19:38
@gtk-grafana gtk-grafana requested a review from a team as a code owner August 1, 2024 19:38
@gtk-grafana gtk-grafana self-assigned this Aug 1, 2024
@svennergr
Copy link
Contributor

@gtk-grafana I don't think you need the VAR_SERVICE_EXPR_HACK. What I did in ServiceSelectionScene to make this work:

  1. Add:
  protected _variableDependency = new VariableDependencyConfig(this, {
    variableNames: [VAR_DATASOURCE],
    onReferencedVariableValueChanged: this.onReferencedVariableValueChanged.bind(this),
  });
  1. onReferencedVariableValueChanged
  private onReferencedVariableValueChanged(variable: SceneVariable) {
    if (variable.state.name === VAR_DATASOURCE) {
      if (this.state.$data instanceof SceneQueryRunner) {
        this.state.$data.runQueries();
      }
      return;
    }
  }

@gtk-grafana gtk-grafana requested review from svennergr and a team August 2, 2024 12:09
@gtk-grafana
Copy link
Contributor Author

Thanks Sven, should be updated now, and I also added an e2e test to ensure that changing the datasource re-triggers the volume query

Copy link
Contributor

@svennergr svennergr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Love the tests! Left a few nits, but great stuff.

@gtk-grafana
Copy link
Contributor Author

Thanks for the feedback, everything should be updated now!

Quick bug fix: I noticed that when running queries against loki-ops I was still getting the cancelled queries I worked so hard to fix, found out it was the loading state was gating the render of the body, which was cleaning up the existing query runners. Dunno why it wasn't happening with the gdev-loki datasource, but I just changed the render method to show the body when we have services, so we don't clean up all those query runners for panels for a specific service that will probably be in the next request as well.
If the service isn't in the new volume request, it will trigger a query that gets cancelled when the volume comes back, but I think we're stuck with that unless we can set a dependency on a query runner that says don't update your query on time range change until the volume call completes.

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

Successfully merging this pull request may close these issues.

3 participants