Elasticsearch: Fix query initialization logic & query transformation from Prometheus/Loki #31322
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ivanahuckova
approved these changes
Feb 22, 2021
There was a problem hiding this comment.
LGTM! I have tested Prom -> Elastic and Loki -> Elastic data source switch and it gave me correct query based on previous data source in both cases. I have also looked at older Elastic dashboards to see if there aren't any unexpected results, but all looked good. 👍 for adding and updating tests.
Good job. :)
| importQueries(queries: DataQuery[], datasourceType: string): ElasticsearchQuery[] { | ||
| if (datasourceType === 'prometheus' || datasourceType === 'loki') { | ||
| return queries.map((query) => { | ||
| let prometheusQuery: PromQuery = query as PromQuery; | ||
| let prometheusQuery = query as PromQuery; | ||
| const expr = getElasticsearchQuery(extractPrometheusLabels(prometheusQuery.expr)); | ||
| return { | ||
| isLogsQuery: true, |
There was a problem hiding this comment.
Do we still need isLogsQuery: true if we are adding type: 'logs'?
There was a problem hiding this comment.
I'm not completely sure about it, i'm keeping it to avoid breaking anything, but for sure I'm gonna revisit it now with the BE refactoring. Thanks for pointing that out!
ryantxu
pushed a commit
that referenced
this pull request
Feb 24, 2021
…from Promethous/Loki (#31322) * Elasticsearch: Fix query initialization logic * Only import prometheus & loki queries as log queries
achatterjee-grafana
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Before the react refactoring, when switching from a prometheus or loki datasource to Elasticsearch in explore, we were generating logs query. While this was still happening, the new query initialization logic was overwriting those changes.
Additionally, everytime we were switching from ANY datasource to elasticsearch we were creating a log query, which is what most users probably want when switching from prometheus/loki, but not from other datasources.
Which issue(s) this PR fixes:
Fixes #31300
Special notes for your reviewer:
I changed a bit the initialization logic, this should be 100% compatible with what we had before, but it needs a bit of testing to be sure.
I'll also create an issue and a follow-up PR to add documentation for the "transform prometheus/loki query to elastic search query functionality"