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

win_docker_daemon: initial release #538

Closed

Conversation

Dan70402
Copy link

What does this PR do?

A plugin that gets docker stats from the docker api. This is useful if running windows containers where the existing docker integration cannot provide stats. This module borrows heavily from the old docker_daemon module.

Motivation

To get similar performance stats from windows docker containers as are provided from the linux docker integration.

DataDog/docker-dd-agent#175

Review checklist

  • PR has a meaningful title or PR has the no-changelog label attached
  • Feature or bugfix has tests
  • Git history is clean
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo

Additional Notes

None

A plugin that gets docker stats from the docker api.  This is useful if running windows containers where the existing docker integration cannot provide stats.  This module borrows heavily from the old docker_daemon module.
@Dan70402 Dan70402 requested review from a team as code owners February 10, 2020 19:50
## Data Collected

### Metrics
| name | type | description |
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to inline the metrics in the README here, just adding:

See [metadata.csv](<LINK_TO_METADATA.CSV_FILE>) for a full list of provided metrics by this integration.

will inline automatically the metrics table in the public Datadog documentation.


To install the win_docker_daemon check on your host:

1. Install the [developer toolkit](https://docs.datadoghq.com/developers/integrations/new_check_howto/#developer-toolkit) on any machine.
Copy link
Contributor

Choose a reason for hiding this comment

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

Link should be set as reference in the whole README.md


## Overview

This check monitors [win_docker_daemon][1] through the Datadog Agent. This integration targets gathering docker stats from the daemon api for Windows containers. The core docker integration uses the filesystem\cgroups to gather performance stats. This approach does not work with Windows. This integration attempts to create the same metrics as the docker integration. It currently targets:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This check monitors [win_docker_daemon][1] through the Datadog Agent. This integration targets gathering docker stats from the daemon api for Windows containers. The core docker integration uses the filesystem\cgroups to gather performance stats. This approach does not work with Windows. This integration attempts to create the same metrics as the docker integration. It currently targets:
This check monitors [win_docker_daemon][1] through the Datadog Agent. This integration targets gathering docker stats from the daemon api for Windows containers. The core docker integration uses the filesystem\cgroups to gather performance stats which is not applicable on Windows. This integration attempts to collect the same metrics as the docker integration. It currently targets:

We should add a link to the docker integration that is mentioned.

- memory
- network

This module borrows heavily from the old docker_daemon plugin.
Copy link
Contributor

Choose a reason for hiding this comment

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

Link missing


Need help? Contact [Datadog support][6].

[1]: **LINK_TO_INTEGRATION_SITE**
Copy link
Contributor

Choose a reason for hiding this comment

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

Link missing



### Events
- Die
Copy link
Contributor

Choose a reason for hiding this comment

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

Event description is missing



### Service Checks
- docker.exit
Copy link
Contributor

Choose a reason for hiding this comment

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

Service check description is missing

[6]: https://docs.datadoghq.com/help


### TODO's
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be removed in the final version of the README.md

@hithwen hithwen requested a review from a team February 11, 2020 13:01
Copy link
Contributor

@hithwen hithwen left a comment

Choose a reason for hiding this comment

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

Hi, thanks for your submission. To add a new integration in extras you need to agree to be a maintainer. This includes checking externals PRs you may get in the integration.

win_docker_daemon/assets/service_checks.json Show resolved Hide resolved
win_docker_daemon/datadog_checks/__init__.py Show resolved Hide resolved
@hithwen hithwen dismissed their stale review February 11, 2020 14:14

not relevant

@hithwen
Copy link
Contributor

hithwen commented Feb 11, 2020

Hi @Dan70402, It seems we're planning to include this functionality out fo the box soon so it's probably better to wait a bit and see if what we provide covers your needs.

@Dan70402
Copy link
Author

Thanks for that info @hithwen. From what I could gather in some threads was windows container monitoring was a small use-case and less likely to be implemented. Do you have any idea what "soon" will be? weeks, months, year? If this PR is going to overlap with an upcoming feature it can be closed.

@hithwen
Copy link
Contributor

hithwen commented Feb 12, 2020

Hi @Dan70402 the team is currently working on this, we expect it to be ready in a couple of months.

@Dan70402
Copy link
Author

That's good to hear. No need for this PR then. Feel free to close.

@hithwen hithwen closed this Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants