-
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
Refactor kubelet metricsets to share response from endpoint #25782
Conversation
Signed-off-by: chrismark <chrismarkou92@gmail.com>
💔 Build Failed
Expand to view the summary
Build stats
Trends 🧪Steps errorsExpand to view the steps failures
|
Pinging @elastic/integrations (Team:Integrations) |
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
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.
Looks good, only some nitpicking.
GetSharedFamilies(prometheus p.Prometheus) ([]*dto.MetricFamily, error) | ||
GetSharedKubeletStats(http *helper.HTTP) ([]byte, error) |
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. I am thinking now that this being shared is an implementation detail, interface and consumers don't care if the response is shared or cached.
GetSharedFamilies(prometheus p.Prometheus) ([]*dto.MetricFamily, error) | |
GetSharedKubeletStats(http *helper.HTTP) ([]byte, error) | |
GetFamilies(prometheus p.Prometheus) ([]*dto.MetricFamily, error) | |
GetKubeletStats(http *helper.HTTP) ([]byte, error) |
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
/package |
Failing tests seem to be unrelated (hitting issue reported at #25956). Merging this one. |
What does this PR do?
Follow up of #25640. This PR changes how kubernetes module handle metricsets which collect metrics from kubelet's API and which share same target endpoint.
Metricsets affected:
system
pod
node
volume
container
Why is it important?
To improve the performance of the module by avoid fetching same content multiple times.
How to test this PR locally
Related issues
Closes #24869
Screenshots