-
Notifications
You must be signed in to change notification settings - Fork 3.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
Promtail: Autodetect log format #380
Conversation
4246b47
to
7716f9b
Compare
Not sure why it failed linting. Looks to be complaining about code in vendor. Can someone re-trigger? |
These tests are pretty flaky. lint was failing, now it is successful. Tests were successful, now they're failing. Tests succeed locally. 😞 |
yeah i fixed one intermittent test problem yesterday and there seems to be another yet. additionally the linter has always seemed flaky. I kicked it again and hopefully it passes this time. I may be working on something else today and won't get to dig into this, but at first glance my only feedback would be to add some kind of persistence or caching to the detected format so that we don't have to iterate through the formats for every log entry? Maybe there is a way to store what parser worked and just continue to use that for the duration? |
Failed again. 😢
The current implementation does cache it for the remainder of the log file. It stores the autodetected parser as a variable in the closure. |
@steven-sheehy would like to chat with you about this and some other ongoing work to try to coordinate things a little, would you be able to join our public slack grafana.slack.com and ping me @ewelch ? |
7716f9b
to
9a4f2c9
Compare
Tests finally successful! 🎉 |
Signed-off-by: Steven Sheehy <[email protected]>
9a4f2c9
to
cc96ca4
Compare
@@ -201,7 +203,7 @@ func (s *syncer) sync(groups []*targetgroup.Group) { | |||
} | |||
|
|||
func (s *syncer) newTarget(path string, labels model.LabelSet) (*FileTarget, error) { | |||
return NewFileTarget(s.log, s.entryHandler, s.positions, path, labels, s.targetConfig) | |||
return NewFileTarget(s.log, s.entryParser.Wrap(s.entryHandler), s.positions, path, labels, s.targetConfig) |
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.
Should I re-detect at a deeper level than FileTarget?
Closing due to the complexity needed to support this for pipeline stages and the improvements made since this in configuring the default parser. |
[release-5.9] Backport PR grafana#14526
Fixes #357
cc @slim-bean @tomwilkie