Skip to content
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 docker.kmem.usage #3339

Merged
merged 1 commit into from
May 2, 2019
Merged

Support docker.kmem.usage #3339

merged 1 commit into from
May 2, 2019

Conversation

wolf31o2
Copy link
Contributor

Some recent versions of Docker perform Kernel Memory accounting. Because
of this, it's necessary to monitor a container's Kernel Memory usage.
This gives us a GAUGE counter to determine kernel memory. This has been
in use in production on our site for 6 months.

Signed-off-by: Chris Gianelloni [email protected]

What does this PR do?

Adds metrics collection for Docker container Kernel memory usage.

Motivation

Running Docker versions > 1.13 on RHEL-based kernels enabled kernel memory process accounting. However, this used the old cgroup v1 interface, which was essentially broken. To properly size container limits, accounting of the container's kernel memory usage was required.

Additional Notes

This is fairly simple and doesn't touch existing functionality.

Review checklist (to be filled by reviewers)

  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • Feature or bugfix must have tests
  • Git history must be clean
  • If PR adds a configuration option, it must be added to the configuration file.

@wolf31o2 wolf31o2 requested review from a team as code owners March 20, 2019 18:43
agaffney
agaffney previously approved these changes Mar 20, 2019
Copy link
Contributor

@gzussa gzussa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank a lot for this contribution 💯. It is really appreciated!
Looks good to me but I would like @DataDog/container-integrations to approve as well.

@Simwar
Copy link
Contributor

Simwar commented Mar 26, 2019

Hi @wolf31o2 ,

Thanks again for the contribution!
This PR adds the metric to the docker_daemon check, which is the check used for the Agent 5.
For the Agent 6, the check has been reworked in Go and added in the agent directly.

So, if you are using the Agent 6, you won't get this metric if we merge it right now.

You can take a look at this PR: DataDog/datadog-agent#2952 this is a good example on how to add a docker metric for the agent 6.

Also, please add your new metric to the metadata file, it will be used in our documentation:
https://github.com/DataDog/integrations-core/blob/master/docker_daemon/metadata.csv

Thanks!

@stale
Copy link

stale bot commented Apr 25, 2019

This issue has been automatically marked as stale because it has not had activity in the last 30 days. Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity. Thank you for participating in the Datadog open source community.

@stale stale bot added the stale label Apr 25, 2019
@hkaj
Copy link
Member

hkaj commented Apr 26, 2019

Hi @wolf31o2
Circling back on this, do you need it in agent 5, or are you using agent 6? If you are pulling the image from datadog/docker-dd-agent it's agent 5 (and we can merge this PR), if you're using datadog/agent this PR won't help and you'll need to add it in https://github.com/DataDog/datadog-agent/ as @Simwar mentioned. Or you can open an issue and we'll triage it.

Thanks

@stale stale bot removed the stale label Apr 26, 2019
@wolf31o2
Copy link
Contributor Author

Sorry, I'm aware that this is for agent 5. As I said, we're using this in production.

Copy link
Member

@hkaj hkaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying @wolf31o2
Could you update the metadata.csv file in the same folder with this new metric then? This will make it easily discoverable in the Datadog UI, add a description for everyone to see, and make the unit scale nicely on graphs.

@wolf31o2
Copy link
Contributor Author

Sure. :-)

Some recent versions of Docker perform Kernel Memory accounting. Because
of this, it's necessary to monitor a container's Kernel Memory usage.
This gives us a GAUGE counter to determine kernel memory. This has been
in use in production on our site for 6 months.

Signed-off-by: Chris Gianelloni <[email protected]>
Copy link
Member

@hkaj hkaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brilliant, thanks @wolf31o2 !

@hkaj hkaj merged commit 9cf23af into DataDog:master May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants