[metricbeat]kubernetes: add persistentvolumeclaim from kube-state-metrics#15066
Conversation
odacremolbap
added
Metricbeat
odacremolbap
added
review
and removed
in progress
| - name: storage_class | ||
| type: keyword | ||
| description: Storage class for the PVC. | ||
| - name: labels.* |
There was a problem hiding this comment.
do you know if this labels are exactly Kubernetes labels? If that's the case we could map them under the common kubernetes.labels
There was a problem hiding this comment.
It is.
I moved it to the module location.
Will need to do so with PV and Services (on a different PR)
exekias
left a comment
exekias
left a comment
There was a problem hiding this comment.
This is looking good, just left a question on labels
| func (m *persistentvolumeclaimMetricSet) Fetch(reporter mb.ReporterV2) { | ||
| events, err := m.prometheus.GetProcessedMetrics(m.mapping) | ||
| if err != nil { | ||
| m.Logger().Error(err) |
There was a problem hiding this comment.
You can write the signature of the method like func (m *persistentvolumeclaimMetricSet) Fetch(reporter mb.ReporterV2) error and return err here directly. The under the hood implementation will do the m.Logger().Error(err) and the reporter.Error(err).
There was a problem hiding this comment.
thanks a lot @sayden
will use that func signature from now on.
sayden
left a comment
sayden
left a comment
There was a problem hiding this comment.
LGTM. If you want to change the thing on the comment is also fine :)
| description: > | ||
| Information and statistics of pods managed by kubernetes. | ||
| fields: | ||
| - name: labels.* |
There was a problem hiding this comment.
There was a problem hiding this comment.
to clarify, that fields.yml is included in any beat using the processor, including Metricbeat
There was a problem hiding this comment.
oh, for some reason beyond my comprehension I tested without adding that field and it appeared as non mapped at kibana.
I'll re-check and remove if the field is already there.
thanks!
There was a problem hiding this comment.
done @exekias
please, re-review if you will
| kube_persistentvolumeclaim_status_phase{namespace="default",persistentvolumeclaim="mysql-data",phase="Lost"} 0 | ||
| kube_persistentvolumeclaim_status_phase{namespace="default",persistentvolumeclaim="mysql-data",phase="Pending"} 0 | ||
| kube_persistentvolumeclaim_resource_requests_storage_bytes{namespace="default",persistentvolumeclaim="mysql-data"} 1.073741824e+09 | ||
| kube_persistentvolumeclaim_labels{label_app="mysql-server",namespace="default",persistentvolumeclaim="mysql-data"} 1 |
There was a problem hiding this comment.
it's interesting how they included labels here when it's normally not part of the info we get. Do you know why?
There was a problem hiding this comment.
(not sure if I follow)
that labels pattern seems to be common to all main kube-state-metrics objects. Not seeing it anywhere else, nor for annotations, but it looks like the label_keyname="value" is here to stay.
exekias
left a comment
exekias
left a comment
There was a problem hiding this comment.
LGTM, could you remove the labels field? It should not be needed and can cause issues
|
jenkins test this |
|
jenkins test this failed tests look unrelated |
…rics (elastic#15066) kubernetes.persistentvolumeclaim from kube-state-metrics (cherry picked from commit 9b48f2b)
5 participants
kubernetes persistent volume claim metrics for kube-state-metrics.
refer to:
https://github.com/kubernetes/kube-state-metrics/blob/release-1.8/docs/persistentvolumeclaim-metrics.md
Note: master branch ksm document above adds a metric family that is not supported yet, and won't be until next ksm is released.
related to #14821