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

[Metricbeat]Kubernetes: make state_container metricset ECS compliant #13884

Merged

Conversation

odacremolbap
Copy link
Contributor

Move existing field kubernetes.container.id, at state_container kubernetes metricset, to the ECS compliant fields.

Origin field contains a string prefixed by the container runtime, and suffixed with the container ID
New fields populated are

  • container.runtime
  • container.id

Previous field won't be removed until 8.0 elastic stack release.

Fixes #10926

@odacremolbap odacremolbap requested a review from a team as a code owner October 2, 2019 18:42
@odacremolbap odacremolbap self-assigned this Oct 2, 2019
@odacremolbap odacremolbap added Metricbeat Metricbeat Team:Integrations Label for the Integrations team containers Related to containers use case v7.5.0 test-plan Add this PR to be manual test plan labels Oct 2, 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.

Do the new fields need to be added to fields.yml?

@odacremolbap
Copy link
Contributor Author

@kaiyan-sheng thanks for taking a look.
new fields are ECS, so I guess they don't

@odacremolbap
Copy link
Contributor Author

jenkins test this

@odacremolbap
Copy link
Contributor Author

jenkins, test this

@odacremolbap odacremolbap mentioned this pull request Oct 4, 2019
19 tasks
@odacremolbap
Copy link
Contributor Author

jenkins, test this

@odacremolbap
Copy link
Contributor Author

jenkins, test this

@odacremolbap odacremolbap merged commit 932dd3c into elastic:master Oct 7, 2019
@odacremolbap odacremolbap deleted the task/kubernetes-containerid-ecs branch October 7, 2019 10:21
@odacremolbap
Copy link
Contributor Author

manual testing:

  • run ES + optionally kibana
  • deploy kubernetes (could be minikube) and kube-state-metrics as stated here
  • deploy metricbeat, you can either use an build candidate image, or use the release branch and the playground pod.
  • configure state_container metricset:
- module: kubernetes
  enabled: true
  metricsets:
    - state_container
  period: 10s
  hosts: ["kube-state-metrics.kube-system:8080"]
  in_cluster: true

Expected behaviour:

  • Events must still contain kubernetes.container.id
  • Field container.runtime exists, and contains the container runtime used
  • Field container.id exists, and contains the container unique identifier
  • All new fields are already mapped by ECS (no ? shown at kibana

@tbragin
Copy link
Contributor

tbragin commented Oct 14, 2019

@sorantis @sgrodzicki @jasonrhodes Do we need any special handling for this change on the Metrics UI front, especially as we move toward 8.0 where the old fields are going to be removed?

@sgrodzicki
Copy link

@simianhacker probably knows best

@kaiyan-sheng
Copy link
Contributor

We are looking at removing old fields here in 8.0. @simianhacker Could you confirm here that metrics UI is already switched to use the new field names? TIA!!

@MichaelKatsoulis
Copy link
Contributor

@kaiyan-sheng after discussions in #23585 (comment) we will remove only kubernetes.container.image in 8.0 as its value is duplicated to ECS field container.image.name.
@simianhacker can you confirm that this field is not used in Metrics UI ?

@mukeshelastic
Copy link

@elastic/infra-monitoring-ui can someone confirm whether removal of this field will be a problem in kibana metrics UI?

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 Metricbeat Metricbeat review Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan v7.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ECS container fields for kubernetes
8 participants