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

Loki: Kick start your query now applies templates to the current query #87658

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

matyax
Copy link
Contributor

@matyax matyax commented May 10, 2024

This PR changes kick start your query to support keeping some of the pipe operations in the original query when the user chooses to replace the current query with a pattern.

To achieve that:

  • We added a list of operations to keep.
  • Then filter the operations in the original query using this list.
  • Then filter operations in the pattern that are already present in the original query*
  • Finally we merge the operations in the original query with those in the selected pattern.
  • This extra change was to prevent producing queries with, for example, empty line filters (when there was previously another line filter), duplicated parsers, etc.

Work in combination with logs or metrics patterns.

Which issue(s) does this PR fix?:

Fixes #76207

Special notes for your reviewer:

Demo:

DEmo.mov

Copy link
Member

@ivanahuckova ivanahuckova left a comment

Choose a reason for hiding this comment

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

Yay for this! Left an idea bellow.

@@ -23,6 +23,27 @@ type Props = {
onAddQuery?: (query: LokiQuery) => void;
};

const keepOperations: string[] = [
Copy link
Member

Choose a reason for hiding this comment

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

Could we reuse some of the logic that already exists, because whenever we add a new operation, this would be an another place that we would need to update.

I was thinking about using category from QueryBuilderOperationDefinition which defines where are shown operations:

image

WDYT?

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 think this is a very good idea! I'll see if we can use that.

@matyax matyax force-pushed the matyax/kick-start-your-metrics branch from b3633d9 to 2470a8e Compare May 16, 2024 10:09
@matyax matyax force-pushed the matyax/kick-start-your-metrics branch from 3f1f790 to dc99417 Compare May 16, 2024 15:56
@matyax matyax requested a review from ivanahuckova May 16, 2024 15:57
@matyax matyax changed the title Kick start your query: keep pipe operations in the original query Loki: Kick start your query now applies templates using the current query May 16, 2024
@matyax matyax changed the title Loki: Kick start your query now applies templates using the current query Loki: Kick start your query now applies templates to the current query May 16, 2024
@matyax matyax added add to changelog no-backport Skip backport of PR labels May 16, 2024
@matyax matyax requested a review from a team May 16, 2024 16:00
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.

Loki: Metric kick start your query should be applied on top of your log query
2 participants