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

Windows: Add support for docker stats #25737

Merged
merged 1 commit into from
Sep 17, 2016
Merged

Conversation

lowenna
Copy link
Member

@lowenna lowenna commented Aug 15, 2016

Signed-off-by: John Howard jhoward@microsoft.com

This PR adds container stats for Windows. It supports both Windows Server and Hyper-V containers.

Stats exposed from the platform are fundamentally quite different to those exposed by Linux. Where possible (and sensible), I have re-utilised Linux fields in the REST structure returned from the daemon to the client. However, in other cases, noticeably storage, the platforms are significantly different enough to warrant a different structure.

The REST structure has been enhanced, the same as for docker build and other calls, so that the client is aware of the platform of the daemon. In that way, the client can display appropriate information (eg PIDs are not appropriate on Windows). As the client only knows the daemon platform once there is at least one ContainerStat returned from the daemon, rather than just printing the headers immediately (which could be for the wrong platform), it will indicate that it is waiting for statistics, and only print the right header once at least one ContainerStat has been retrieved.

Memory management is sufficiently different between Windows and Linux, and indeed different enough between Windows Server containers and Hyper-V containers, that the only field which makes sense for client display purposes is the private working set. This is the same as task manager in Windows using the default settings.

CPU usage calculation is also sufficiently different, but generally the same fields can be utilised. The main difference is that for each interval to calculate percentage, the previous read time and number of processors are required from the daemon. In that way it can calculate the available number of 100ns ticks on the host between time samples, and from that calculate percentage utilisation of each container.

The final note worth mentioning on this PR is that all stats in the platform are returned via HCS. This is by design of the Windows API surface. HCS will indirectly get the networking stats from HNS is that same call. Hence there is no call into libnetwork to obtain networking stats as is done on Linux.

The net result of the change is that the client does "the right thing" regardless of whether it's Linux to Linux, Windows to Linux, Linux to Windows or any other platform combination you choose.

Here's an example of running Windows to Windows docker stats where there are a variety of Windows Server and Hyper-V containers running.

image

And a similar example on Linux to Windows. First before any stats are available (just running docker stats when there are no running containers)

image

And with a bunch of containers running on the target Windows machine:

image

And the corresponding examples using the updated client pointing to a Linux daemon to demonstrate that the familiar column set for Linux daemons is still displayed and hasn't regressed

image

image

Finally an example of the full REST output to query the stats of a single (Windows Server) container against a Windows daemon.

{"read":"2016-08-15T21:42:53.3872243Z","preread":"2016-08-15T21:42:52.3881218Z","pids_stats":{},"blkio_stats":{"io_service_bytes_recursive":null,"io_serviced_recursive":null,"io_queue_recursive":null,"io_service_time_recursive":null,"io_wait_time_recursive":null,"io_merged_recursive":null,"io_time_recursive":null,"sectors_recursive":null},"num_procs":8,"storage_stats":{"read_count_normalized":9309,"read_size_bytes":69064192,"write_count_normalized":1203177,"write_size_bytes":9854512640},"cpu_stats":{"cpu_usage":{"total_usage":158125000,"usage_in_kernelmode":102968750,"usage_in_usermode":102968750},"throttling_data":{"periods":0,"throttled_periods":0,"throttled_time":0}},"precpu_stats":{"cpu_usage":{"total_usage":158125000,"usage_in_kernelmode":102968750,"usage_in_usermode":102968750},"throttling_data":{"periods":0,"throttled_periods":0,"throttled_time":0}},"memory_stats":{"commitbytes":70619136,"commitpeakbytes":114601984,"privateworkingset":52281344},"networks":{"faed4256-741a-4492-ba5b-98da170c647c":{"rx_bytes":10670115968,"rx_packets":7120183,"rx_dropped":0,"tx_bytes":205687305,"tx_packets":3703891,"tx_dropped":0}}}

~~
This PR is in multiple commits to break down the changes into reasonable sized chunks to ease code review and make the logical parts easy to understand. Otherwise this would be a difficult PR to understand.

1. API and matching client changes. One change is for the fields in the REST structure, the other  for returning the daemon platform (`OSType`).
2. Daemon libcontainerd changes to call into HCS to retrieve the stats from the platform
3. Daemon refactoring of stats_collector to be present on Windows, and to populate the stats in a REST call
4. Client display changes depending on platform
5. Integration test changes

~~
Squashed 9/16/2016 for merge

@crosbymichael
Copy link
Contributor

Why is it priv working set and not just memory?

@lowenna
Copy link
Member Author

lowenna commented Aug 15, 2016

"Memory" is a very overloaded term in Windows. Private working set is what is displayed in task manager.

image

@thaJeztah thaJeztah added this to the 1.13.0 milestone Aug 17, 2016
@lowenna lowenna force-pushed the jjh-statistics branch 2 times, most recently from a376279 to bfe49c1 Compare August 31, 2016 20:34
@lowenna lowenna added the status/failing-ci Indicates that the PR in its current state fails the test suite label Aug 31, 2016
@lowenna lowenna force-pushed the jjh-statistics branch 4 times, most recently from 9a3c15d to c852667 Compare September 1, 2016 18:23
@lowenna lowenna changed the title Windows: Add support for docker stats [needs revendor] Windows: Add support for docker stats Sep 7, 2016
@lowenna lowenna added status/2-code-review and removed status/1-design-review status/failing-ci Indicates that the PR in its current state fails the test suite labels Sep 7, 2016
@lowenna
Copy link
Member Author

lowenna commented Sep 7, 2016

Rebased and updated now engine-api is back in docker/docker.

@lowenna lowenna closed this Sep 7, 2016
@lowenna
Copy link
Member Author

lowenna commented Sep 7, 2016

@vdemeester as you reviewed what was the engine-api PR, but no longer needed. 😸

@vdemeester
Copy link
Member

LGTM 👼
/cc @thaJeztah does this need docs-review ?

@@ -141,7 +163,7 @@ func (s *DockerSuite) TestApiStatsNetworkStats(c *check.C) {

func (s *DockerSuite) TestApiStatsNetworkStatsVersioning(c *check.C) {
testRequires(c, SameHostDaemon)
testRequires(c, DaemonIsLinux)
//testRequires(c, DaemonIsLinux)
Copy link
Member

Choose a reason for hiding this comment

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

Oh, looks like this can be removed

Copy link
Member

Choose a reason for hiding this comment

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

❤️

Copy link
Member Author

Choose a reason for hiding this comment

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

Doh. Yes, removed and pushed.

@thaJeztah
Copy link
Member

Given that the output is slightly different on Windows, perhaps it's worth adding an example in the docs https://docs.docker.com/engine/reference/commandline/stats/#/examples

@lowenna
Copy link
Member Author

lowenna commented Sep 15, 2016

@thaJeztah Sure, will add an example.

@lowenna
Copy link
Member Author

lowenna commented Sep 15, 2016

@thaJeztah Done 😄

@cpuguy83
Copy link
Member

Can you squash before merge?

@lowenna
Copy link
Member Author

lowenna commented Sep 16, 2016

Sure. @thaJeztah Docs ok? If so will squash ready for merge.

@vdemeester
Copy link
Member

docs LGTM 🐸

@lowenna
Copy link
Member Author

lowenna commented Sep 16, 2016

OK, squashing...

Signed-off-by: John Howard <jhoward@microsoft.com>
@lowenna
Copy link
Member Author

lowenna commented Sep 16, 2016

Squashed. Just need CI to be 💚 now 😄

@thaJeztah
Copy link
Member

Yes, docs LGTM, thanks! And a pretty cool feature, really nice to see this coming to Windows

(sorry for the delay 😄)

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.

6 participants