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

Index pod IP too in ip_port kubernetes indexer #5721

Merged
merged 1 commit into from
Nov 29, 2017

Conversation

exekias
Copy link
Contributor

@exekias exekias commented Nov 27, 2017

Before this change, it only indexed ip:port combination, but in some cases, we want to have IP only, as source port may not be exported

@@ -0,0 +1,22 @@
package add_kubernetes_metadata

Choose a reason for hiding this comment

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

don't use an underscore in package name

@@ -0,0 +1,22 @@
package add_kubernetes_metadata

Choose a reason for hiding this comment

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

don't use an underscore in package name

@exekias exekias added the review label Nov 27, 2017
@exekias exekias changed the title Index pod IP too in ip_port Index pod IP too in ip_port kubernetes indexer Nov 27, 2017
@ruflin
Copy link
Member

ruflin commented Nov 28, 2017

@exekias Can you rebase this one?

Before this change it only indexed ip:port combination, but in some
cases we want to have ip only, as source port may not be exported
@exekias
Copy link
Contributor Author

exekias commented Nov 28, 2017

done!

Data: commonMeta,
})

for i := 1; i < len(hostPorts); i++ {
Copy link
Member

@ruflin ruflin Nov 28, 2017

Choose a reason for hiding this comment

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

Should this be i := 0? Or are you skipping the first one because it is the podIP? If yes, perhaps add a comment here otherwise I think it's likely to break if this gets refactored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, as the first item in the list will be the ip, this filters it to range on ip:port pairs

@ruflin ruflin merged commit fca031f into elastic:master Nov 29, 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.

3 participants