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: Add unpack query builder hint #65608

Merged
merged 6 commits into from
Mar 30, 2023
Merged

Loki: Add unpack query builder hint #65608

merged 6 commits into from
Mar 30, 2023

Conversation

svennergr
Copy link
Contributor

@svennergr svennergr commented Mar 30, 2023

What is this feature?

This PR adds hints if users have log lines that are in a packed format. Those lines come from the pack stage at ingestion: https://grafana.com/docs/loki/latest/clients/promtail/stages/pack/

Special notes for your reviewer:

  1. Start Grafana and Loki (make devenv sources=loki)
  2. Write a new LogQL query using the query builder.
  3. Select place as label key and moon as value.
  4. See unpack parser being suggested.

image

@matyax
Copy link
Contributor

matyax commented Mar 30, 2023

We should also suggest it in Monaco, correct? Like we do with detected parsers.

Copy link
Contributor

@matyax matyax left a comment

Choose a reason for hiding this comment

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

Looks good!

public/app/plugins/datasource/loki/lineParser.test.ts Outdated Show resolved Hide resolved
const lineField = frame.fields.find((field) => field.type === FieldType.string);
if (lineField == null) {
return { hasJSON: false, hasLogfmt: false };
return { hasJSON: false, hasLogfmt: false, hasPack: false };
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration .github/workflows/pr-codeql-analysis-javascript.yml:analyze. As part of the setup process, we have scanned this repository and found 27 existing alerts. Please check the repository Security tab to see all alerts.

@svennergr
Copy link
Contributor Author

We should also suggest it in Monaco, correct? Like we do with detected parsers.

Added in 1bb559a. Let me know if I miss anything!

@svennergr svennergr requested a review from matyax March 30, 2023 12:47
@matyax
Copy link
Contributor

matyax commented Mar 30, 2023

Verified!

imagen

Maybe we should show both json and unpack as detected?

@svennergr
Copy link
Contributor Author

Maybe we should show both json and unpack as detected?

This is coming from a customer call we had earlier and we (@ivanahuckova) quickly aligned on not showing JSON parser if we think log lines are packed. Might be a bit misleading, because all packed lines are JSON.

Copy link
Contributor

@matyax matyax left a comment

Choose a reason for hiding this comment

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

👌

@github-actions
Copy link
Contributor

Backend code coverage report for PR #65608
No changes

@github-actions
Copy link
Contributor

Frontend code coverage report for PR #65608

Plugin Main PR Difference
loki 83.74% 83.74% 0%

@svennergr svennergr merged commit d790cb1 into main Mar 30, 2023
@svennergr svennergr deleted the svennergr/add-unpack-hint branch March 30, 2023 13:18
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.

None yet

3 participants