-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add source path matching to add_docker_metadata
processor
#4495
Conversation
353b7a1
to
c8c8f93
Compare
This should be useful to enrich events coming from docker logs, as it will parse the container id from the source path.
c8c8f93
to
cf3d393
Compare
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.
Nice work. (Reviewed from mobile so let me know if any of my short comments aren't clear.)
func (f extract_field) Run(event common.MapStr) (common.MapStr, error) { | ||
fieldValue, err := event.GetValue(f.Field) | ||
if err != nil { | ||
return nil, fmt.Errorf("Error getting field '%s' from event", f.Field) |
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.
Use lower case error strings. https://github.com/golang/go/wiki/CodeReviewComments#error-strings
} | ||
|
||
parts := strings.Split(value, f.Separator) | ||
parts = deleteEmpty(parts) |
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.
Doesn't this create is disconnect between the configured index and the index into parts?
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 fully get this question, but will explain why I had to delete empty parts. When you split a string like /var/lib/docker/containers/foo
you get [ '', 'var', 'docker', 'containers', 'foo' ]
. It's counter intuitive to ask for index 4 and get containers
(I would expect foo
) because of the empty string at the beginning. Things get worse if you have things like /var//lib/...
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.
That answers my question (I was thinking of the /var//lib/...
case).
|
||
result, err := actual.GetValue(test.Target) | ||
assert.NoError(t, err) | ||
assert.Equal(t, result.(string), test.Result) |
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.
Does this type assertion panic if there was an err? I'm not a fan of assert.NoError due to it continuing on error. Maybe use "if assert.NoError { assert.Equal }". Check other uses and possibly replace with an "if err then t.Fatal".
9fd07ef
to
9f5b969
Compare
9f5b969
to
0df8b11
Compare
jenkins retest this |
This change adds
match_source
setting to automatically matchsource
field from filebeat events. Default settings take care of most common scenario. It will match/var/lib/docker/containers/<container_id>/*.log
. Thematch_source_index
setting allows changing this to match other position in the path. Just use: