feat: add support for discovering and adding log levels as structured metadata #12428
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
type/docs
cyriltovena
reviewed
Apr 2, 2024
| return extractLogLevelFromLogLine(entry.Line) | ||
| } | ||
|
|
||
| func extractLogLevelFromLogLine(log string) string { |
There was a problem hiding this comment.
Can you add the benchmark please this will be useful to compare changes ?
cyriltovena
approved these changes
Apr 2, 2024
svennergr
reviewed
Apr 2, 2024
| @@ -57,6 +58,13 @@ const ( | |||
|
|
|||
| labelServiceName = "service_name" | |||
| serviceUnknown = "unknown_service" | |||
| labelLevel = "level" | |||
There was a problem hiding this comment.
Should we, at least for now, rename that to something like?
Suggested change
| labelLevel = "level" | |
| labelLevel = "level_detected" |
I'm a bit afraid it could lead to false detections, and then it would even overwrite the "real" detected label from the log line and change that to level_extracted, which might break users workflows.
Take this logline as an example:
logger=tsdb.loki endpoint=queryData pluginId=loki dsName=grafanacloud-securityops-logs dsUID=grafanacloud-logs traceID=e6560d272e58b8bd94ed0d6da53c5a30 fromAlert=true end=2024-04-02T10:52:26.954369164Z step=30s query="{error=~\".+\"} " queryType=rangestatus=ok level=info
It's just an info log, but the detection would see the error=, which is part of the "parsed" query label. Leading to add the logLevelError as structured metadata, and the parsed level label renamed to level_extracted.
rhnasc
pushed a commit
to inloco/loki
that referenced
this pull request
Apr 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
The most common way to query logs from Loki is using Grafana. Grafana relies on log levels to colour-code logs and histograms to help users see the distribution of logs by various levels. It would be good to have these log levels present in a uniform way for all the logs.
This PR adds support for detecting and adding log levels as structured metadata, if it is not already present in the stream labels or structured metadata. The feature is behind per-tenant configuration and is kept disabled by default.
For otlp logs, we are using severityNumber from the otel spec to map them to a log level as defined here. For non-otel logs we detect the log level by looking for specific keywords in the log line. If we do not find any of the keywords in the log line, we default to
infolevel for that log. The initial implementation is simple and can be refined further.We are using
strings.Containswhich might seem in-optimal, but I did some benchmarks and compared it against regex and Aho–Corasick go implementations.strings.Containsoutperformed them in all the aspects. We can revisit this if we see any performance bottlenecks.Checklist
CHANGELOG.mdupdated