-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[pkg/stanza] Improve error logs produced by transformer processors #37285
base: main
Are you sure you want to change the base?
[pkg/stanza] Improve error logs produced by transformer processors #37285
Conversation
…tor-contrib into improve-log-transformers-logs
…tor-contrib into improve-log-transformers-logs
I have no idea about why this |
…tor-contrib into improve-log-transformers-logs
zap.Any("action", t.OnError), | ||
} | ||
if entry != nil { | ||
toAddFields := []zap.Field{zap.Any(attrs.LogRecordOriginal, entry.Body)} |
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.
I don't agree we should add the body. The body could contain sensitive information and is not necessarily the reason for the failure. Maybe the timestamp is enough?
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.
I wanted to make the Collector's log useful enough so that we could avoid looking at the real log file.
I had totally overlooked the fact that the body can contain sensitive information. So I agree that we can't simply include it there. :/
Originally I wanted to include the line number of the entry, but because of the way the file scanning is working it got complicated (lots of wrapping of the scanner's splitfunc and some file position tracking is byte based instead of line based).
I think the timestamp will be a good substitute for now and I can try to get the line number working later. 👍
…tor-contrib into improve-log-transformers-logs
…tor-contrib into improve-log-transformers-logs
@djaglowski do you have some time to have another look at this, please? |
Description
This improves the error messages of the log transformers processors to include the log file path and the original log record, effectively allowing users to quickly debug and investigate deeper the reason for the problem without having to use the debug exporter or using the highest verbosity level.
Here's an example of one of these failures:
This log line could be even more helpful if the it included the regex pattern that was matched against the entry. I'm not sure about adding it now and it could be added in the future if desired.
A small note on the name of the log keys: they could have shorter names, like
body
andfile_path
but I think we could also use the attribute names from the semantic conventions for logs. I have no preference though and I'm happy to change it if needed.Testing
log.record.original
key is present in the logs.