Skip to content

[docker_daemon] Add 5 opt-in checks for container & volume counts#2740

Closed
parkr wants to merge 3 commits intoDataDog:masterfrom
parkr:docker-daemon-track-containers-volumes
Closed

[docker_daemon] Add 5 opt-in checks for container & volume counts#2740
parkr wants to merge 3 commits intoDataDog:masterfrom
parkr:docker-daemon-track-containers-volumes

Conversation

@parkr
Copy link
Contributor

@parkr parkr commented Aug 9, 2016

What does this PR do?

This pull request adds 5 new metrics:

  • docker.containers.count – equivalent to docker ps -a | wc -l
  • docker.containers.state_dead.count – count of all dead containers
  • docker.containers.state_exited.count – count of all exited containers
  • docker.volumes.count – count of all volumes
  • docker.volumes.dangling.count – count of all dangling volumes

Each metric has its own toggle in the settings so you can opt in to each on its own.

Motivation

We've been battling a bit of weirdness with containers and volumes lying around. We use ephemeral docker containers for some operations but found that volumes would be lying around in the "dangling" state, and that some containers weren't properly cleaned up.

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Additional Notes

I went with one monolithic commit here because the code was too close together for Git to play nicely with cherry-picking the commits. Let me know if that wasn't the right way to go and I can make 5 distinct commits.

@parkr
Copy link
Contributor Author

parkr commented Aug 9, 2016

/cc @ross

@olivielpeau
Copy link
Member

olivielpeau commented Aug 10, 2016

Thanks @parkr! We'll review your changes in details soon.

No problem at all about the monolithic commit, it looks clean enough as it is 😃

We might want to add some tests on this in test_docker_daemon.py, we'll keep you updated

@parkr
Copy link
Contributor Author

parkr commented Aug 11, 2016

@olivielpeau Thanks! It's been a while since I have done any Python work so pardon the lack of idiomatic Python. I tried to fix it up by using filter and so forth where I can, but still wearing my Ruby hat. 😄 I wasn't able to get the tests fixed up yet, but will try to take a look soon. I'll be on vacation next week so it may have to wait until after that.

@remh remh self-assigned this Aug 12, 2016
m_func = FUNC_MAP[GAUGE][self.use_histogram]
# Report docker.container.count if state is "Any", otherwise
# report docker.container.state_STATE.count
suffix = ".state_{}".format(state.lower()) if state is not "Any" else ""
Copy link

Choose a reason for hiding this comment

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

any reason not to use a tag instead for the state ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @remh! No particular reason! I am used to a much more basic version of graphite and tags are new. How would you like to proceed here?

Copy link
Member

Choose a reason for hiding this comment

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

Hi @parkr
The way we usually do this is to report docker.containers.count several times, each time with a different state tag. This could look like:

# const
STATES = ['created', 'restarting', 'running', 'paused', 'exited', 'dead']
...
def _report_container_count_by_state(self, containers_by_id):
    tags = self._get_tags(filtered[0], PERFORMANCE)
    m_func = FUNC_MAP[GAUGE][self.use_histogram]
    for state in STATES:
        # the filter changes for each state
        filterlambda = lambda x: not self._is_container_excluded(x) and container["State"] is state
        filtered = list(filter(filterlambda, containers_by_id))
        # add the state tag
        tags.append('state:{}'.format(state)
        m_func(self, 'docker.container.count'.format(suffix), len(filtered), tags=tags)

This way you can sum the reported values of docker.container.count on a graph to get the total count, or split the graph over the state tag and have the detailed count.

Let me know if it's still unclear.

(this snippet is not tested and could be optimized)

@remh
Copy link

remh commented Aug 12, 2016

Thanks a lot @parkr ! That's pretty cool! Could you also add tests for the metrics you are collecting please ?

@parkr
Copy link
Contributor Author

parkr commented Sep 2, 2016

Could you also add tests for the metrics you are collecting please ?

I just went looking through other PR's to see how this is best accomplished and didn't find recent examples of PR's which add functionality to this check and add tests. I'll take a stab but it might be terrible.

- collect_exited_container_count: gauge of how many containers have State=Exited
- collect_dead_container_count: gauge of how many containers have State=Dead
- collect_container_count: gauge of how many containers there are
- collect_volume_count: gauge of how many volumes there are
- collect_dangling_volume_count: gauge of how many dangling volumes there are
tags=tags)

def _report_container_count_by_state(self, containers_by_id, state="Any"):
count = 0
Copy link
Member

Choose a reason for hiding this comment

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

looks like it's not used, you can remove it safely

@hkaj
Copy link
Member

hkaj commented Oct 19, 2016

Hi @parkr
Do you have the bandwidth to finish this up? Otherwise I can take over. Not rushing you, I just want to make sure this PR eventually gets all the ❤️ it deserves (these are some interesting metrics!).

@remh remh modified the milestones: 5.11.0, Triage Oct 24, 2016
@parkr
Copy link
Contributor Author

parkr commented Oct 25, 2016

Do you have the bandwidth to finish this up?

I got stuck trying to write tests for this. Couldn't seem to mock the response from the docker API.

@parkr parkr closed this Jan 9, 2017
@parkr parkr deleted the docker-daemon-track-containers-volumes branch January 9, 2017 17:26
@masci masci modified the milestones: 5.11.0, 5.12.0 Jan 24, 2017
hkaj added a commit that referenced this pull request Feb 7, 2017
…iners-volumes

[docker] container & volume metrics (wraps #2740)
@parkr
Copy link
Contributor Author

parkr commented Feb 7, 2017

#3077, which supersedes this PR, was merged.

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