-
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
Support kube_state_metrics v2.0.0 #27552
Support kube_state_metrics v2.0.0 #27552
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
Pinging @elastic/integrations (Team:Integrations) |
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.
Addition looks good to me! I left a minor code related question/suggestion.
I'm not sure if we miss anything else in order to support v2.0.0 but I guess we have already done our research and we concluded to the fields that we are handling here, right?
Also I would suggest to list in the PR's description the fields that we handle in this PR for v2.0.0. so as to have clear/quick access to them in the future if we have questions about v2 support.
@@ -130,18 +142,29 @@ func (m *MetricSet) Fetch(reporter mb.ReporterV2) error { | |||
} | |||
|
|||
m.enricher.Enrich(events) | |||
|
|||
//m.Logger().Infof("Events are %+v", events) |
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.
leftover?
if limit, ok := event["cpu.limit.cores"]; ok { | ||
if limitCores, ok := limit.(float64); ok { | ||
event["cpu.limit.nanocores"] = limitCores * nanocores | ||
if cpuFields, ok := event["cpu"]; ok { |
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.
Why not to convert the event
to common.MapStr
and then access the cpu.limit.cores
field directly, so as to avoid all these if
s? With the methods that MapStr
provides you should be ok.
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.
Good catch. event
is already a common.MapStr
"kube_pod_container_info": p.InfoMetric(), | ||
"kube_pod_info": p.InfoMetric(), | ||
"kube_pod_container_info": p.InfoMetric(), | ||
"kube_pod_container_resource_requests": p.Metric("", p.OpFilterMap( |
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.
Great that you were able to re-use our existing OpFilter to handle this!
@@ -21,7 +21,8 @@ | |||
"MetricSetFields": { | |||
"cpu": { | |||
"request": { | |||
"cores": 0.15 |
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.
Also I think that we need to cover ksm.v2.0.0
in our tests, right?
@ChrsMark It is updated and ready for review |
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.
lgtm, left some nits
@@ -130,21 +142,19 @@ func (m *MetricSet) Fetch(reporter mb.ReporterV2) error { | |||
} | |||
|
|||
m.enricher.Enrich(events) | |||
|
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.
nit: consider not removing lines if there is no specific reason
} | ||
} | ||
|
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.
nit: same here, do not remove lines if it's not intentional
* Support kube_pod_container_resource_requests and kube_pod_container_resource_limits * Add kube_node_status_capacity and kube_node_status_allocatable support * Add tests for ksm v2 for all state data streams (cherry picked from commit 1596d64)
* Support kube_pod_container_resource_requests and kube_pod_container_resource_limits * Add kube_node_status_capacity and kube_node_status_allocatable support * Add tests for ksm v2 for all state data streams (cherry picked from commit 1596d64) Co-authored-by: Michael Katsoulis <[email protected]>
* master: Forward port 7.14.1 changelog to master (elastic#27687) Addressing multiple dashboard issues: deps loading once, field conversion, etc. (elastic#27669) Remove adaptive queue sizes from agent's spec files (elastic#27653) Osquerybeat: Improve testability and unit test coverage (elastic#27591) Osquerybeat: lockdown flagsfile, prevent global defaults (elastic#27611) Import the references of dashboard assets using the Saved Objects API (elastic#27647) Fix bug with override path in cgroups (elastic#27620) Allow Kibana client to authorize with Elasticsearch API key (elastic#27540) Filebeat auditd: Fix Top Exec Commands dashboard visualization (elastic#27638) [elastic-agent] Fix docker tar.gz generation for complete image (elastic#27621) Follow up changes in dashboards in mage check && fix minor issue (elastic#27553) [Heartbeat] Fix bug where `enabled: false` is ignored. (elastic#27615) Support kube_state_metrics v2.0.0 (elastic#27552)
* Support kube_pod_container_resource_requests and kube_pod_container_resource_limits * Add kube_node_status_capacity and kube_node_status_allocatable support * Add tests for ksm v2 for all state data streams (cherry picked from commit 1596d64)
* Support kube_state_metrics v2.0.0 (#27552) * Support kube_pod_container_resource_requests and kube_pod_container_resource_limits * Add kube_node_status_capacity and kube_node_status_allocatable support * Add tests for ksm v2 for all state data streams (cherry picked from commit 1596d64) * Update CHANGELOG.next.asciidoc
* Support kube_pod_container_resource_requests and kube_pod_container_resource_limits * Add kube_node_status_capacity and kube_node_status_allocatable support * Add tests for ksm v2 for all state data streams
What does this PR do?
This PR enables
Metricbeat
to supportkube-state-metrics
version 2.0.0 for kubernetes.In v2.0.0 some breaking changes where introduced.
Following fields where removed and replaced by
kube_pod_container_resource_requests
andkube_pod_container_resource_limits
.Following fields where removed and replaced by
kube_node_status_capacity
andkube_node_status_allocatable
.Metricbeat was updated to support both new and old format of those fields.
Why is it important?
After version 2.0.0 of kube-state-metrics some fields where deprecated and replaced by others.
Without this PR fields
where missing when
kube-state-metrics v2.0.0
and higher was present in the Kubernetescluster.
Details can be found here.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally
Run metricbeat as daemonset in a Kubernetes cluster where
kube-state-metrics v2.0.0
or higher is deployed and watch those fields getting populated in Elasticsearch.Related issues
Screenshots