-
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
Add support for podman metrics in docker module #41889
Add support for podman metrics in docker module #41889
Conversation
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
The relevant docker integration should also be updated with the new variable podman, right? |
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
@elastic/elastic-agent-data-plane could I get a review here as you are the codeowners? |
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.
Only one suggestion but can be merged in the current state too.
metricbeat/module/docker/docker.go
Outdated
if !stream { | ||
if err := decoder.Decode(&event.Stats); err != nil { | ||
return event | ||
} | ||
} else { |
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 like in either case (error or not) we return the content of the event
variable here. Does it even make sense to check for the error in this case?
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 are right, I was just receiving a linter error. I decided to add an debug log message.
* Add support for podman metrics (cherry picked from commit 1fefdbb)
* Add support for podman metrics (cherry picked from commit 1fefdbb)
* Add support for podman metrics (cherry picked from commit 1fefdbb)
* Add support for podman metrics (cherry picked from commit 1fefdbb) Co-authored-by: Michael Katsoulis <[email protected]>
* Add support for podman metrics (cherry picked from commit 1fefdbb) Co-authored-by: Michael Katsoulis <[email protected]>
#41967) * Add support for podman metrics in docker module (#41889) --------- Co-authored-by: Michael Katsoulis <[email protected]>
* Add support for podman metrics
Proposed commit message
Details
Podman offers a docker compatible API for metrics collection. So using docker module to collect podman metrics should be working out of the box.
In reality the memory metrics were not published at all and cpu usage percentage calculation was incorrect.
The reason is that the podman api returns zero values for
precpu_stats
. These stats refer to the latest's read cpu statistics, needed for cpu percentage calculation. Also due to these stats being zero, the memory metrics were not populated because of a sanity check in the code.beats/metricbeat/module/docker/memory/helper.go
Line 55 in 3f51793
For docker, the
precpu_stats
are returned every time.The solution in the Podman's case is to stream the api response. By default we had set the stream option to false as there was no need in case of docker.
As part of this PR we introduce a new configuration parameter named
podman
(by default false). If set to true, indicating that podman is used, then the stream parameter is set to true, only for the collection of cpu and memory stats.The reason is that for cpu and memory stats the
precpu_stats
are needed.From the streamed response, we get the second response as it was noticed during testing that the
precpu_stats
of the first response were incorrect and the cpu was miscalculated.Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Disruptive User Impact
There is no disruptive user impact.
How to test this PR locally
podman run -d -p 8080:80 --name mynginx nginx
a.
podman exec -ti mynginx bash
b.
while true; do :; done &
podman
parameter set to truepodman stats
command results.Related issues
Use cases
Screenshots
Podman Stats command (check for mynginx container)