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

Add docker metric "docker.cpu.shares" #1358

Merged
merged 7 commits into from Apr 16, 2018

Conversation

Projects
None yet
5 participants
@winebarrel
Copy link
Contributor

winebarrel commented Apr 7, 2018

What does this PR do?

Adds docker.cpu.shares metric to the datadog agent.
It is the value of /sys/fs/cgroup/cpu/cpu.shares.

2018-04-07 21 39 22

Motivation

from https://help.datadoghq.com/hc/en-us/requests/138307 .

I want normalized CPU usage for container CPU monitoring.

docker.cpu.usage: 100% has a different meaning depending on the value of cpu.shares (=ECS task definition "cpu" parameter).
If cps.shares is 1024, it reaches the limit.

If the agent can get cpu.shares, I can make a monitor like this:

avg:docker.cpu.usage{*} by {container_name} * 1024 / avg:docker.cpu.shares{*} by {container_name}

2018-04-07 21 50 09

Versioning

  • Bumped the check version in manifest.json
  • Bumped the check version in datadog_checks/{integration}/__init__.py
  • Updated CHANGELOG.md. Please use Unreleased as the date in the title
    for the new section.
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo

Additional Notes

Pull request for version 6: DataDog/datadog-agent#1562

@masci masci added the community label Apr 9, 2018

@xvello
Copy link
Member

xvello left a comment

Thanks @winebarrel for your contribution, the extraction logic for both versions of the agent LGTM,

Could you please:

I suggest docker.cpu.shares,gauge,,,,"Shares of CPU usage allocated to the container",0,docker,cpu shares

@xvello xvello self-assigned this Apr 12, 2018

@hkaj
Copy link
Member

hkaj left a comment

Thanks for the PR @winebarrel !
I left one minor comment, the code will be ready to merge once it's fixed. Can you also take care of updating manifest.json, the changelog, and the check version as explained here?
Take a look at other PRs to see examples of that.

Thanks again :)

@@ -1026,6 +1033,9 @@ def _parse_cgroup_file(self, stat_file):
# 2 ** 60 is kept for consistency of other cgroups metrics
if value < 2 ** 60:
return dict({'softlimit': value})
elif 'cpu.shares' in stat_file:
value = int(fp.read())
return dict({'shares': value})

This comment has been minimized.

@hkaj

hkaj Apr 12, 2018

Member

i don't think we need the dict()?

This comment has been minimized.

@winebarrel

winebarrel Apr 16, 2018

Author Contributor

thanks.
fixed 455a38d

@winebarrel

This comment has been minimized.

Copy link
Contributor Author

winebarrel commented Apr 16, 2018

@xvello @hkaj Thank you for your comment!
I fixed the pull request.

@hkaj

hkaj approved these changes Apr 16, 2018

Copy link
Member

hkaj left a comment

thanks @winebarrel

@hkaj hkaj referenced this pull request Apr 16, 2018

Merged

Add docker CPU shares gauge #1597

@hkaj hkaj merged commit b9c3fee into DataDog:master Apr 16, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@winebarrel winebarrel deleted the winebarrel:support_cpu_shares branch Apr 16, 2018

@winebarrel

This comment has been minimized.

Copy link
Contributor Author

winebarrel commented Apr 16, 2018

Thank you for merging PR!!

@cmatteri

This comment has been minimized.

Copy link
Contributor

cmatteri commented May 10, 2018

@winebarrel thanks for the PR! docker.cpu.usage should be a percentage of a single core (it's not capped at 100), not a fraction, though the metadata for this metric was incorrect at one point. Thus, I believe the query should be:

avg:docker.cpu.usage{*} by {container_name} * 1024 / 100 / avg:docker.cpu.shares{*} by {container_name}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.