-
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
[metricbeat] Add creation timestamp to state_persistentvolumeclaim metricset #37108
Conversation
This pull request doesn't have a |
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
@@ -6,6 +6,9 @@ | |||
}, | |||
"MetricSetFields": { | |||
"access_mode": "ReadWriteOnce", | |||
"created": { |
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.
Isn't this file created automatically ? I wonder why indentation is wrong.
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.
Isn't this file created automatically ? I wonder why indentation is wrong.
Sorry, It is very likely that I did something wrong during my tests, I'm still trying to familiarize myself with the unit tests in this package. Could you explain me what command is supposed to generate this file? 🙇
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.
You can go to the metricbeat/module/kubernetes/state_persistentvolumeclaim
directory and just run on the terminal:
go test -data
That should generate all the expected files.
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.
Thanks a lot @constanca-m , last commit includes the files generated by go test -data
@@ -29,6 +29,7 @@ var mapping = &p.MetricsMapping{ | |||
Metrics: map[string]p.MetricMap{ | |||
|
|||
"kube_persistentvolumeclaim_access_mode": p.LabelMetric("access_mode", "access_mode"), | |||
"kube_persistentvolumeclaim_created": p.Metric("created.sec"), |
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.
We collect the creation time in other metricsets of kubernetes module as well. See state_service
, state_job
and state_storageclass
. In all those cases we use
"kube_service_created": p.Metric("created", p.OpUnixTimestampValue()),
which transforms the seconds in a unix timestamp and declare the field as date. See
- name: created |
I would suggest to do the same here and name the new filed created
instead of created.sec
I haven't looked at how to test it yet. I'm using Agent, not sure about the simplest way to do that 😕
Let me know which one should be updated. |
We have a README file on how to test this @barkbay |
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.
Thanks @barkbay for adding this. We should add the new field in the integration data stream as well. But it should happen after the FF of beats.
…tricset (elastic#37108) * [metricbeat] Add creation timestamp to state_persistentvolumeclaim metricset
This PR is an attempt to include the creation timestamp for
PersistentVolumeClaim
resources, as introduced in kubernetes/kube-state-metrics#1741:This is my very first PR to add a field in Metricbeat, so please be indulgent 😄
Proposed commit message
[metricbeat] Add creation timestamp to state_persistentvolumeclaim metricset
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
n/a
Use cases
We need the creation timestamp in order to trigger alerts for Pending volumes bases on their creation timestamps.