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

Graphite: Fix autocomplete when tags are not available #31680

Merged
merged 14 commits into from Mar 16, 2021

Conversation

ifrost
Copy link
Contributor

@ifrost ifrost commented Mar 4, 2021

What this PR does / why we need it:

This is to fix #17429. Long story short the query builder's autocomplete input makes 2 requests to graphite-web: one to get a list of metrics and one to get a list of tags and merges both lists together:

dropdown

The problem is when one of the requests fails, nothing is returned in the suggests list.

The request to get the list of tags may fail in two scenarios:

  1. Graphite is upgraded to 1.1 (supporting tags) but tags are disabled in the config and the database is not been migrated. No tables for tags are created causing the endpoint to get the list of tags to return HTTP 500.

To mitigate this problem the errors for tags request are caught to return an empty list and the error is displayed in the console.

  1. The version of the datasource in the configuration is set to 1.1 (supporting tags) while the actual version of Graphite is lower (without tags support). In this case autocomplete makes a request to non-existing tags endpoint causing the break.

This may however happen unintentionally in some edge cases. When creating a new Graphite data source the version input is initially populated:

graphite-version-dropdown

But this value is not being saved if it's not changed explicitly. In Explore section the query component falls back to 1.1 when the version is not defined, but this fall back was changed in 7.4.0 (from 0.9 to 1.1) meaning that all data sources created before 7.4.0 (intentionally created for 0.9 with the default value in the input) are now treated as 1.1 and make an attempt to get tags, hence breaking autocomplete.

To avoid similar problems in future the initial version is saved when a new data source is created.

We could also automatically detect the version of Graphite by calling /version endpoint (https://github.com/graphite-project/graphite-web/blob/master/webapp/graphite/version/views.py)

Which issue(s) this PR fixes:

Fixes #17429.

@ifrost ifrost requested a review from aocenas March 4, 2021 13:46
@ifrost ifrost changed the title Graphite: [draft] fix autocomplete when tags are not available Graphite: fix autocomplete when tags are not available Mar 4, 2021
@ifrost ifrost requested a review from a team March 5, 2021 08:50
@@ -13,6 +14,7 @@ const graphiteVersions = [
{ label: '1.0.x', value: '1.0' },
{ label: '1.1.x', value: '1.1' },
];
const DEFAULT_GRAPHITE_VERSION = graphiteVersions[2];
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: probably makes sense to have last(graphiteVersions) so that when new version is added it's automatically the latest version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've extracted the list to graphite_versions.ts to use the default (now last from the list) in datasource.ts and ConfigEditor.tsx

public/app/plugins/datasource/graphite/datasource.ts Outdated Show resolved Hide resolved
In some configurations graphite-web fail to return the list of tags. It shouldn't however prevent displaying list of metrics (which is concatenated with tags).
The version of Graphite is preselected in the dropdown but was not saved in jsonData initially.
@ifrost ifrost force-pushed the ifrost/17429_Graphite_query_auto-complete_breaks branch from b578683 to 355f844 Compare March 8, 2021 10:11
@ifrost ifrost marked this pull request as ready for review March 8, 2021 15:35
@ifrost ifrost requested review from a team and aocenas March 8, 2021 15:35
@ifrost ifrost self-assigned this Mar 9, 2021
@ifrost
Copy link
Contributor Author

ifrost commented Mar 10, 2021

It should also resolve #24287

Copy link
Member

@aocenas aocenas left a comment

Choose a reason for hiding this comment

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

One small additional comment but otherwise this look nice, good work.

public/app/plugins/datasource/graphite/query_ctrl.ts Outdated Show resolved Hide resolved
@ifrost ifrost merged commit 6495a73 into master Mar 16, 2021
@ifrost ifrost deleted the ifrost/17429_Graphite_query_auto-complete_breaks branch March 16, 2021 09:59
ryantxu pushed a commit to dejapong/grafana that referenced this pull request Mar 30, 2021
* Return empty list of tags when tags are not available

In some configurations graphite-web fail to return the list of tags. It shouldn't however prevent displaying list of metrics (which is concatenated with tags).

* Populate jsonData with default version of Graphite

The version of Graphite is preselected in the dropdown but was not saved in jsonData initially.

* Fix a typo

* Show a popup with an error message

* Always use the latest Graphite value as the default one when creating a datasource

* Move autocomplete error handling to GraphiteQueryCtrl

* Test error handing in Graphite autocomplete

* Test default Graphite version fallback

* Rename graphite_versions.ts to versions.ts

* Remove redundant import

* Code formatting, minor renaming

* Remove redundant error info
@ifrost ifrost added this to the 8.0.0 milestone Mar 31, 2021
@dprokop dprokop changed the title Graphite: fix autocomplete when tags are not available Graphite: Fix autocomplete when tags are not available May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Graphite query auto-complete breaks when using Graphite without tags
3 participants