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

Address issue #10505 (add_kubernetes_metadata processor matcher not working) #10506

Merged
merged 5 commits into from
Feb 4, 2019
Merged

Conversation

alakahakai
Copy link

add_kubernetes_metadata processor is broken because it uses old field source for lookup. Use log.file.path for matcher.

@alakahakai alakahakai requested a review from andrewkroh February 2, 2019 17:53
@alakahakai alakahakai requested a review from a team as a code owner February 2, 2019 17:53
@alakahakai alakahakai changed the title Address issue #10505 Address issue #10505 (add_kubernetes_metadata processor matcher not working) Feb 2, 2019
@alakahakai alakahakai requested a review from tsg February 4, 2019 06:39
@ruflin ruflin requested a review from jsoriano February 4, 2019 09:09
@ruflin ruflin added the Team:Integrations Label for the Integrations team label Feb 4, 2019
@alakahakai alakahakai requested review from andrewkroh and tsg and removed request for tsg and andrewkroh February 4, 2019 16:54
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.

@alakahakai
Copy link
Author

alakahakai commented Feb 4, 2019

I think there are some test case(s) that need updated in

beats/filebeat/processor/add_kubernetes_metadata/matchers_test.go

Line 111 in 106eb3b
"source": source,

Good catch. fixed this.

CHANGELOG.asciidoc Outdated Show resolved Hide resolved
@alakahakai alakahakai requested review from a team as code owners February 4, 2019 21:22
@alakahakai alakahakai removed request for a team February 4, 2019 21:45
Copy link
Contributor

@tsg tsg left a comment

Choose a reason for hiding this comment

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

LGTM.

@alakahakai alakahakai merged commit 8b520c5 into elastic:master Feb 4, 2019
@ruflin
Copy link
Contributor

ruflin commented Feb 5, 2019

@alakahakai Thank you for the fix.

@@ -77,9 +77,9 @@ const containerIdLen = 64
const podUIDPos = 5

func (f *LogPathMatcher) MetadataIndex(event common.MapStr) string {
if value, ok := event["source"]; ok {
if value, ok := event["log"].(common.MapStr)["file"].(common.MapStr)["path"]; ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

This line probably breaks with the redis input as it does not have the fields in here.

Copy link
Member

Choose a reason for hiding this comment

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

@alakahakai I think using event.Get("log.file.path") should be safe w.r.t. #11543 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants