-
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 JSON options to autodiscover hints #14208
Conversation
Signed-off-by: chrismark <[email protected]>
740f98c
to
084ccc5
Compare
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Great addition! |
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.
Looking good!
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.
sorry didn't notice failing tests, this will require a make update
, and probably a rebase
Also, outside of the scope of this PR (maybe a followup). What would you think about simplifying these parameters for the user? I'm thinking something like this would make things better:
This somehow would be an alias for:
which is probably what most users want |
Signed-off-by: chrismark <[email protected]>
🤔 could you elaborate more? |
Sure! sorry for my short explanation 🤕 I think that current config keys are not very straight forward. Having that in most cases what users want is probably to just decode the This would be the same behavior as using |
@ChrsMark had a chat and we decided it probably makes more sense to have the |
jenkins, test this again please |
Merging this since Travis went green and failing tests of Jenkins look irrelevant to this patch. |
Assigning myself for manually testing this PR for 7.6.0 release per |
Tested manually. Works as expected! |
Removed |
Tested with BC1 and this works as expected! |
Closes #12634.
cc: @odacremolbap, @exekias
Manual testing
Configure filebeat autodiscover
Having
filebeat
running, launch containers with the proper labels each time.Make sure that Filebeat has permissions to read from
/var/lib/docker/containers/
, if not a quick option could be to run with sudo likesudo -E ./filebeat -e -d "*" --strict.perms=false
. Note that for testing this a Linux host is needed so as Filebeat to be able to locate and read log files properly.Test that keys are not copied under top level in the output document
docker run -l co.elastic.logs/json.keys_under_root=false -it busybox echo '{"foo":"bar", "error_msg": "error opening file"}'
You should see in the event the following:
Test keys are copied under top level in the output document
docker run -l co.elastic.logs/json.keys_under_root=true -it busybox echo '{"foo":"bar", "error_msg": "error opening file"}'
You should see the
foo
anderror_msg
in top level.Test that error is reported
docker run -l co.elastic.logs/json.keys_under_root=true -l co.elastic.logs/json.add_error_key=true -it busybox echo '{"foo":"bar", "error_msg": "error opening file" and some buggy stuff}'
Event should be enriched with: