-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[exporter/loki] Automatically maps severity value to "level" attribute #14560
[exporter/loki] Automatically maps severity value to "level" attribute #14560
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you maybe add one sentence in the readme of the Loki exporter mentioning this automatic mapping?
pkg/translator/loki/convert.go
Outdated
@@ -59,7 +58,15 @@ func convertAttributesToLabels(attributes pcommon.Map, attrsToSelect pcommon.Val | |||
attr = strings.TrimSpace(attr) | |||
av, ok := attributes.Get(attr) // do we need to trim this? | |||
if ok { | |||
out[model.LabelName(attr)] = model.LabelValue(av.AsString()) | |||
if attr == levelAttributeName { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets revert this changes and do the conversion in logs_to_loki#handleSeverity
method, smth like
level, _ := severityNumberToLevel[log.SeverityNumber().String()]
...
log.Attributes().PutStr(levelAttributeName, level)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
linting error: coz of _
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@jpkrohling do you want to take another look at the PR? |
I'll take another look. |
@jpkrohling Does it looks good to you ? |
Could you please fix the conflicts? This PR will then be ready to be merged. Thank you for your patience during this PR! |
If you prefer to have the simple version of the code (the original one you wrote), that's fine as well. We can optimize it later if we find it to be a problem. |
Done. Also backported the code to |
The tests seem to be failing:
|
@jpkrohling, @kovrus Is it good for you ? |
Why is this blocked ? It would be much simpler for me to have this feature ;) |
@jtama I think it is good to be merged. @jpkrohling can you please merge it? |
open-telemetry#14560) Automatically maps severity value to "level" attribute
@jtama Is there any guidance on how to extract the severity field into a log attribute using the OTEL Collector? |
Hi, |
Description: Loki follows severity using the "level" attribute value. This allow automatic mapping from the log record "severity" value to the "attribute.level" value
Link to tracking Issue: Closes #14313
Testing: Test added in the
pkg/translator/loki/convert_test.go#TestConvertAttributesAndMerge
Documentation: Not done for the moment