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.

Sign up for GitHub

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

Jump to bottom

fix: Fix log level detection #12651

Merged
merged 26 commits into from
May 6, 2024
Merged

fix: Fix log level detection #12651

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a9ad857
Fix log level detection
shantanualsi Apr 17, 2024
a1d643d
Small bugfix
shantanualsi Apr 17, 2024
7d70360
Add support for trace and fatal detection
shantanualsi Apr 18, 2024
cbdc4c7
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 18, 2024
8d2ce13
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 19, 2024
00397ee
Support unknown log level and add some tests
shantanualsi Apr 19, 2024
147233a
Add support for LEVEL for discover levels
shantanualsi Apr 19, 2024
5d33553
Fix tests
shantanualsi Apr 19, 2024
bc62249
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 19, 2024
9cd72f9
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 19, 2024
441586b
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 22, 2024
979244b
Add method to parse both logfmt and json to detect level
shantanualsi Apr 22, 2024
207c6b7
Lint
shantanualsi Apr 22, 2024
be71206
Use alternative parsers to discover levels
shantanualsi Apr 22, 2024
86f1d22
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 23, 2024
a16b375
Optimize - don't run entire logfmt if = is not present in the log line
shantanualsi Apr 23, 2024
b01580e
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 24, 2024
4c8196e
Change detected level label
shantanualsi Apr 24, 2024
5a608a9
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 24, 2024
acd7f1f
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 25, 2024
f0f7bf8
Fix allowed labels and detected_level label
shantanualsi Apr 25, 2024
6da3509
Fix docs
shantanualsi Apr 25, 2024
8fa6d6c
Merge branch 'main' into fix-log-level-detection
shantanualsi Apr 29, 2024
9b1b91c
Add detected_level from label or existing structured metadata
shantanualsi May 6, 2024
7795d81
Merge branch 'main' into fix-log-level-detection
shantanualsi May 6, 2024
bae53e1
Do not populate detected_level when level is unknown
shantanualsi May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions docs/sources/shared/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3068,8 +3068,9 @@ The `limits_config` block configures global and per-tenant limits in Loki. The v
[discover_service_name: <list of strings> | default = [service app application name app_kubernetes_io_name container container_name component workload job]]

# Discover and add log levels during ingestion, if not present already. Levels
# would be added to Structured Metadata with name 'level' and one of the values
# from 'debug', 'info', 'warn', 'error', 'critical', 'fatal'.
# would be added to Structured Metadata with name 'level' or 'LEVEL'
# (case-sensitive) and one of the values from 'trace', 'debug', 'info', 'warn',
# 'error', 'critical', 'fatal' (case insensitive).
# CLI flag: -validation.discover-log-levels
[discover_log_levels: <boolean> | default = true]

Expand Down
67 changes: 48 additions & 19 deletions pkg/distributor/distributor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ const (
logLevelError = "error"
logLevelFatal = "fatal"
logLevelCritical = "critical"
logLevelTrace = "trace"
logLevelUnknown = "unknown"
)

var (
Expand Down Expand Up @@ -865,7 +867,11 @@ func detectLogLevelFromLogEntry(entry logproto.Entry, structuredMetadata labels.
if err != nil {
return logLevelInfo
}
if otlpSeverityNumber <= int(plog.SeverityNumberDebug4) {
if otlpSeverityNumber == int(plog.SeverityNumberUnspecified) {
return logLevelUnknown
} else if otlpSeverityNumber <= int(plog.SeverityNumberTrace4) {
return logLevelTrace
} else if otlpSeverityNumber <= int(plog.SeverityNumberDebug4) {
return logLevelDebug
} else if otlpSeverityNumber <= int(plog.SeverityNumberInfo4) {
return logLevelInfo
Expand All @@ -876,7 +882,7 @@ func detectLogLevelFromLogEntry(entry logproto.Entry, structuredMetadata labels.
} else if otlpSeverityNumber <= int(plog.SeverityNumberFatal4) {
return logLevelFatal
}
return logLevelInfo
return logLevelUnknown
}

return extractLogLevelFromLogLine(entry.Line)
Expand Down Expand Up @@ -904,44 +910,68 @@ func extractLogLevelFromLogLine(log string) string {
}

if firstNonSpaceChar == '{' && lastNonSpaceChar == '}' {
if strings.Contains(log, `:"err"`) || strings.Contains(log, `:"ERR"`) ||
strings.Contains(log, `:"error"`) || strings.Contains(log, `:"ERROR"`) {
levelIndex := strings.Index(log, `"level":`)
Copy link
Contributor

Choose a reason for hiding this comment

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

problem is that it could be Level, LEVEL, serverity, Serverity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, i've mentioned it in the caveats in notes.. can we impose this in our documentation? or make this string configurable (case sensetive)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

what if we pass a hint to this function? for example, I think we could change this code to something like the following

firstEntry := stream.Entries[0]
firtsStructuredMetadata := logproto.FromLabelAdaptersToLabels(firstEntry.StructuredMetadata)
prevTs := firstEntry.Timestamp
addLogLevel := validationContext.allowStructuredMetadata &&
				validationContext.discoverLogLevels &&
				!lbs.Has(labelLevel)

logLevelField := detectLogLevelFieldFromLogEntry(firstEntry, firtsStructuredMetadata)

this would allow detectLogLEvelFieldFromLogEntry to do a case insensitive search for a bunch of different options (ie. level, severity, etc) since it was only doing it once for the whole stream per push. We could then pass that as a hint to detectLogLevelFromLogEntry, allowing it to short-circuit when passed.

if levelIndex == -1 {
levelIndex = strings.Index(log, `"LEVEL":`)
if levelIndex == -1 {
return logLevelUnknown
}
}
compareString := log[levelIndex:min(len(log), levelIndex+20)]
cyriltovena marked this conversation as resolved.
Show resolved Hide resolved

if strings.Contains(strings.ToLower(compareString), `:"fatal"`) {
return logLevelFatal
}
if strings.Contains(strings.ToLower(compareString), `:"error"`) || strings.Contains(strings.ToLower(compareString), `:"err"`) {
return logLevelError
}
if strings.Contains(log, `:"warn"`) || strings.Contains(log, `:"WARN"`) ||
strings.Contains(log, `:"warning"`) || strings.Contains(log, `:"WARNING"`) {
if strings.Contains(strings.ToLower(compareString), `:"warning"`) || strings.Contains(strings.ToLower(compareString), `:"warn"`) || strings.Contains(strings.ToLower(compareString), `:"wrn"`) {
return logLevelWarn
}
if strings.Contains(log, `:"critical"`) || strings.Contains(log, `:"CRITICAL"`) {
if strings.Contains(strings.ToLower(compareString), `:"critical"`) {
return logLevelCritical
}
if strings.Contains(log, `:"debug"`) || strings.Contains(log, `:"DEBUG"`) {
if strings.Contains(strings.ToLower(compareString), `:"debug"`) || strings.Contains(strings.ToLower(compareString), `:"dbg"`) {
return logLevelDebug
}
if strings.Contains(log, `:"info"`) || strings.Contains(log, `:"INFO"`) {
if strings.Contains(strings.ToLower(compareString), `:"info"`) || strings.Contains(strings.ToLower(compareString), `:"inf"`) {
return logLevelInfo
}
if strings.Contains(strings.ToLower(compareString), `:"trace"`) || strings.Contains(strings.ToLower(compareString), `:"trc"`) {
return logLevelTrace
}
}

// logfmt logs:
if strings.Contains(log, "=") {
if strings.Contains(log, "=err") || strings.Contains(log, "=ERR") ||
strings.Contains(log, "=error") || strings.Contains(log, "=ERROR") {
levelIndex := strings.Index(log, "LEVEL=")
if levelIndex == -1 {
levelIndex = strings.Index(log, "level=")
}
if levelIndex != -1 {
compareString := log[levelIndex:min(len(log), levelIndex+20)]

if strings.Contains(strings.ToLower(compareString), "=fatal") {
return logLevelFatal
}
if strings.Contains(strings.ToLower(compareString), "=error") || strings.Contains(strings.ToLower(compareString), "=err") {
return logLevelError
}
if strings.Contains(log, "=warn") || strings.Contains(log, "=WARN") ||
strings.Contains(log, "=warning") || strings.Contains(log, "=WARNING") {
if strings.Contains(strings.ToLower(compareString), "=warning") || strings.Contains(strings.ToLower(compareString), "warn") || strings.Contains(strings.ToLower(compareString), "=wrn") {
return logLevelWarn
}
if strings.Contains(log, "=critical") || strings.Contains(log, "=CRITICAL") {
if strings.Contains(strings.ToLower(compareString), "=critical") {
return logLevelCritical
}
if strings.Contains(log, "=debug") || strings.Contains(log, "=DEBUG") {
if strings.Contains(strings.ToLower(compareString), "=debug") || strings.Contains(strings.ToLower(compareString), "=dbg") {
return logLevelDebug
}
if strings.Contains(log, "=info") || strings.Contains(log, "=INFO") {
if strings.Contains(strings.ToLower(compareString), "=info") || strings.Contains(strings.ToLower(compareString), "=inf") {
return logLevelInfo
}
if strings.Contains(strings.ToLower(compareString), "=trace") || strings.Contains(strings.ToLower(compareString), "=trc") {
return logLevelTrace
}

}

if strings.Contains(log, "err:") || strings.Contains(log, "ERR:") ||
Expand All @@ -959,6 +989,5 @@ func extractLogLevelFromLogLine(log string) string {
return logLevelDebug
}

// Default to info if no specific level is found
return logLevelInfo
return logLevelUnknown
}
64 changes: 60 additions & 4 deletions pkg/distributor/distributor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1548,7 +1548,7 @@ func Test_DetectLogLevels(t *testing.T) {
require.Equal(t, push.LabelsAdapter{
{
Name: labelLevel,
Value: logLevelInfo,
Value: logLevelUnknown,
},
}, topVal.Streams[0].Entries[0].StructuredMetadata)
})
Expand Down Expand Up @@ -1625,7 +1625,7 @@ func Test_detectLogLevelFromLogEntry(t *testing.T) {
entry: logproto.Entry{
Line: "foo",
},
expectedLogLevel: logLevelInfo,
expectedLogLevel: logLevelUnknown,
},
{
name: "non otlp with log level keywords in log line",
Expand All @@ -1637,17 +1637,73 @@ func Test_detectLogLevelFromLogEntry(t *testing.T) {
{
name: "json log line with an error",
entry: logproto.Entry{
Line: `{"foo":"bar",msg:"message with keyword error but it should not get picked up",level":"critical"}`,
Line: `{"foo":"bar",msg:"message with keyword error but it should not get picked up","level":"critical"}`,
},
expectedLogLevel: logLevelCritical,
},
{
name: "json log line with an error",
entry: logproto.Entry{
Line: `{"FOO":"bar",MSG:"message with keyword error but it should not get picked up","LEVEL":"Critical"}`,
},
expectedLogLevel: logLevelCritical,
},
{
name: "json log line with an warning",
entry: logproto.Entry{
Line: `{"foo":"bar",msg:"message with keyword warn but it should not get picked up","level":"warn"}`,
},
expectedLogLevel: logLevelWarn,
},
{
name: "json log line with an error in block case",
entry: logproto.Entry{
Line: `{"foo":"bar",msg:"message with keyword warn but it should not get picked up","level":"ERR"}`,
},
expectedLogLevel: logLevelError,
},
{
name: "logfmt log line with a warn",
entry: logproto.Entry{
Line: `foo=bar msg="message with keyword error but it should not get picked up" level=warn`,
},
expectedLogLevel: logLevelWarn,
},
{
name: "logfmt log line with a warn with camel case",
entry: logproto.Entry{
Line: `foo=bar msg="message with keyword error but it should not get picked up" level=Warn`,
},
expectedLogLevel: logLevelWarn,
},
{
name: "logfmt log line with a trace",
entry: logproto.Entry{
Line: `foo=bar msg="message with keyword error but it should not get picked up" level=Trace`,
},
expectedLogLevel: logLevelTrace,
},
{
name: "logfmt log line with some other level returns unknown log level",
entry: logproto.Entry{
Line: `foo=bar msg="message with keyword but it should not get picked up" level=NA`,
},
expectedLogLevel: logLevelUnknown,
},
{
name: "logfmt log line with a info with non standard case",
entry: logproto.Entry{
Line: `foo=bar msg="message with keyword error but it should not get picked up" level=inFO`,
},
expectedLogLevel: logLevelInfo,
},
{
name: "logfmt log line with a info with non block case for level",
entry: logproto.Entry{
Line: `FOO=bar MSG="message with keyword error but it should not get picked up" LEVEL=inFO`,
},
expectedLogLevel: logLevelInfo,
},
} {
t.Run(tc.name, func(t *testing.T) {
detectedLogLevel := detectLogLevelFromLogEntry(tc.entry, logproto.FromLabelAdaptersToLabels(tc.entry.StructuredMetadata))
Expand All @@ -1671,6 +1727,6 @@ func Benchmark_extractLogLevelFromLogLine(b *testing.B) {

for i := 0; i < b.N; i++ {
level := extractLogLevelFromLogLine(logLine)
require.Equal(b, logLevelInfo, level)
require.Equal(b, logLevelUnknown, level)
}
}
2 changes: 1 addition & 1 deletion pkg/validation/limits.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) {
"job",
}
f.Var((*dskit_flagext.StringSlice)(&l.DiscoverServiceName), "validation.discover-service-name", "If no service_name label exists, Loki maps a single label from the configured list to service_name. If none of the configured labels exist in the stream, label is set to unknown_service. Empty list disables setting the label.")
f.BoolVar(&l.DiscoverLogLevels, "validation.discover-log-levels", true, "Discover and add log levels during ingestion, if not present already. Levels would be added to Structured Metadata with name 'level' and one of the values from 'debug', 'info', 'warn', 'error', 'critical', 'fatal'.")
f.BoolVar(&l.DiscoverLogLevels, "validation.discover-log-levels", true, "Discover and add log levels during ingestion, if not present already. Levels would be added to Structured Metadata with name 'level' or 'LEVEL' (case-sensitive) and one of the values from 'trace', 'debug', 'info', 'warn', 'error', 'critical', 'fatal' (case insensitive).")

_ = l.RejectOldSamplesMaxAge.Set("7d")
f.Var(&l.RejectOldSamplesMaxAge, "validation.reject-old-samples.max-age", "Maximum accepted sample age before rejecting.")
Expand Down
Loading