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

fix(detected_fields): return parsed labels when parsers are passed #14047

Merged
merged 5 commits into from
Sep 4, 2024

Conversation

svennergr
Copy link
Contributor

What this PR does / why we need it:

The detected_fields API accepts a full "logQL" query to be passed. This includes parsers. If parsers were passed, the SelectLogs statement would parse labels into the streamLabels property. That would then cause the detected_fields logic to run the "detection" logic again and append _extracted to all parsed labels.

This PR changes the behavior: if parsed labels are returned by SelectLogs, the API does not try to be smart and to parse again, but just return structuredmetadata and parsed labels. The parsers returned in that case will be the parsers used in the query.

@@ -1075,6 +1076,8 @@ func (q *SingleTenantQuerier) DetectedFields(ctx context.Context, req *logproto.
if err != nil {
return nil, err
}
// just incject the header to categorize labels
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// just incject the header to categorize labels
// just inject the header to categorize labels

@gtk-grafana gtk-grafana changed the title detected_fields: return parsed labels when parsers are passed fix(detected_fields): return parsed labels when parsers are passed Sep 4, 2024
@gtk-grafana gtk-grafana added the type/bug Somehing is not working as expected label Sep 4, 2024
@CLAassistant
Copy link

CLAassistant commented Sep 4, 2024

CLA assistant check
All committers have signed the CLA.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 4, 2024
Copy link
Collaborator

@trevorwhitney trevorwhitney 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! I would love a benchmark to see how this affects the performance, but given that it's currently broken without this I think we should merge and worry about performance later (ie. make it work before we make it fast).

@trevorwhitney trevorwhitney merged commit aa1ac99 into main Sep 4, 2024
60 checks passed
@trevorwhitney trevorwhitney deleted the svennergr/detect-only-no-parser branch September 4, 2024 18:48
grafanabot pushed a commit that referenced this pull request Sep 4, 2024
…14047)

Co-authored-by: Galen Kistler <[email protected]>
Co-authored-by: Trevor Whitney <[email protected]>
(cherry picked from commit aa1ac99)
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.

4 participants