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

Ensure Pod UID is always present in pod metricset #12345

Merged
merged 2 commits into from
Jun 4, 2019

Conversation

exekias
Copy link
Contributor

@exekias exekias commented May 29, 2019

Pod UID field is currently retrieved as part of the metadata enrichment
present for all Kubernetes module metricsets. In case of errors or
missconfiguration, pod UID won't be present in resulting documents.

Since this field is available in the pod metricset, this change adds
it to Pod events, avoiding issues when enrichment fails.

@exekias exekias added enhancement Metricbeat Metricbeat containers Related to containers use case labels May 29, 2019
@exekias exekias requested a review from a team as a code owner May 29, 2019 13:51
Pod UID field is currently retrieved as part of the metadata enrichment
present for  all Kubernetes module metricsets. In case of errors or
missconfiguration, pod UID won't be present in resulting documents.

Since this field is available in the `pod` metricset, this change adds
it to Pod events, avoiding issues when enrichment fails.
@exekias exekias force-pushed the hardcode-pod-uid branch from 8a6df66 to 0e1f402 Compare May 29, 2019 13:52
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

Do we need to update metricbeat/module/kubernetes/pod/_meta/data.json as well?

@fearful-symmetry
Copy link
Contributor

Forgive me if I'm a little behind on the k8s metricsets, is there a reason uid isn't in fields.yml? I don't see it.

@exekias
Copy link
Contributor Author

exekias commented May 29, 2019

Forgive me if I'm a little behind on the k8s metricsets, is there a reason uid isn't in fields.yml? I don't see it.

This is a good point, happy to explain: add_kubernetes_metadata processor already configures some of the fields that this module reports. We avoid double-registering them, as that could cause problems:

. But the actual result is that this field was already registered.

Do we need to update metricbeat/module/kubernetes/pod/_meta/data.json as well?

This is mostly defensive code, as this module was already reporting this field through add_metadata enrichment, that's why current data.json already has the field:

"uid": "b89a812e-18cd-11e9-b333-080027190d51",

@exekias exekias added needs_backport PR is waiting to be backported to other branches. review labels May 29, 2019
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

Good to know! Thanks for answering. Besides the merge conflict, it looks good.

@exekias exekias merged commit db51085 into elastic:master Jun 4, 2019
@exekias exekias self-assigned this Jun 6, 2019
andrewvc pushed a commit to andrewvc/beats that referenced this pull request Jun 12, 2019
Pod UID field is currently retrieved as part of the metadata enrichment
present for  all Kubernetes module metricsets. In case of errors or
missconfiguration, pod UID won't be present in resulting documents.

Since this field is available in the `pod` metricset, this change adds
it to Pod events, avoiding issues when enrichment fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
containers Related to containers use case enhancement Metricbeat Metricbeat needs_backport PR is waiting to be backported to other branches. review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants