Skip to content
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

Merged
merged 4 commits into from
Jun 15, 2017

Conversation

exekias
Copy link
Contributor

@exekias exekias commented Jun 13, 2017

This change adds match_source setting to automatically match source field from filebeat events. Default settings take care of most common scenario. It will match /var/lib/docker/containers/<container_id>/*.log. The match_source_index setting allows changing this to match other position in the path. Just use:

processors:
   - add_docker_metadata: ~

This should be useful to enrich events coming from docker logs, as it
will parse the container id from the source path.
Copy link
Member

@andrewkroh andrewkroh left a 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

parts := strings.Split(value, f.Separator)
parts = deleteEmpty(parts)
Copy link
Member

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?

Copy link
Contributor Author

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/...

Copy link
Member

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)
Copy link
Member

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".

@exekias
Copy link
Contributor Author

exekias commented Jun 15, 2017

jenkins retest this

@andrewkroh andrewkroh merged commit 6bc3d4b into elastic:master Jun 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants