fix: correct _extracted logic in detected fields #14064
Conversation
trevorwhitney
changed the title
pkg/querier/querier.go
Outdated
| } | ||
| } | ||
| func parseEntry(entry push.Entry, lbls *logql_log.LabelsBuilder) (map[string][]string, []string) { | ||
| logFmtParser := logql_log.NewLogfmtParser(false, false) |
There was a problem hiding this comment.
this was part of the problem, we had strict turned on previously
There was a problem hiding this comment.
Does it make sense to only initialise the parser (call logFmtParser := logql_log.NewLogfmtParser(false, false)) when we need it? I.e. move it somewhere here https://github.com/grafana/loki/pull/14064/files#diff-a0b881d1b7b99f439716f189b50eef90ccdaea00845b087c0dbce0e148aa2c0eR1339
There was a problem hiding this comment.
yes, can do... missed that when I change the order to try json first
| @@ -1403,12 +1420,28 @@ func streamsForFieldDetection(i iter.EntryIterator, size uint32) (logqlmodel.Str | |||
| // If lastEntry.Unix < 0 this is the first pass through the loop and we should output the line. | |||
| // Then check to see if the entry is equal to, or past a forward step | |||
| if lastEntry.Unix() < 0 || shouldOutput { | |||
| stream, ok := streams[streamLabels] | |||
| allLbls, err := syntax.ParseLabels(streamLabels) | |||
There was a problem hiding this comment.
the logic added here is to extract the stream labels from the additional labels via parsing and structured metadata. this likely needs a test
need this now that logfmt isn't strict
trevorwhitney
added
type/bug
|
This PR must be merged before a backport PR will be created. |
2 similar comments
|
This PR must be merged before a backport PR will be created. |
|
This PR must be merged before a backport PR will be created. |
trevorwhitney
deleted the
svennergr/detected-fields-only-parsed-check
branch
Co-authored-by: Sven Grossmann <[email protected]> (cherry picked from commit 1b3ba53)
What this PR does / why we need it:
Explore logs passes multiple parsers in the query to
detected_fields, which has resulted in a whole bunch of unknown edge cases. This fixes:_extractedis correctly applied when the incoming stream labels already have extracted fields because of the formatters included in the queryParsedvalues on the entry being parsed if not additional parsed values are foundlogfmtandjsonparsers to already parsed fieldsWhich issue(s) this PR fixes:
Fixes #14063
Special notes for your reviewer:
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mdproduction/helm/loki/Chart.yamland updateproduction/helm/loki/CHANGELOG.mdandproduction/helm/loki/README.md. Example PRdeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR