Add support for Names and ID in stats format by vdemeester · Pull Request #27797 · moby/moby

vdemeester · 2016-10-26T18:58:45Z

This adds support to display names or id of container instead of what was provided in the request.

This keeps the default behavior (docker stats byname will display byname in the CONTAINER colmun and docker stats byid will display the id in the CONTAINER column) but adds two new format directive.

$ docker stats 7c3
CONTAINER           CPU %               MEM USAGE / LIMIT       MEM %               NET I/O              BLOCK I/O             PIDS
7c3                 15.02%              666.5 MiB / 15.56 GiB   4.18%               22.46 MB / 3.56 MB   405.5 MB / 162.1 MB   71
$ docker stats --format="table {{.Names}}\t{{.ID}}\t{{.Container}}" 7c3
NAME                CONTAINER ID                                                       CONTAINER
elastic_agnesi      7c32fcc1c619123926f2c3cbc2356375b1887196b6b0cf3ed682973916bbb74e   7c3

/cc @thaJeztah @dnephin

🐸

Signed-off-by: Vincent Demeester [email protected]

MHBauer

Code looks right. Not sure I understand the use case.

MHBauer · 2016-10-28T20:12:19Z

Why here vs on the Stats struct? I don't have any preference. I'm just curious how we decide to put it on one or the other.

Mainly because I wanted to keep Stats struct with only metrics field and no metadata.

MHBauer · 2016-10-28T20:14:58Z

Check my understanding.

Container has the user input name.

Name has the full name, and used to have the user input name.

ID has the full ID.

yep, that's it. I kept Container as the user input as this is what it's doing now.

Could you put ID at the top here? (ID, Name, Container)

No reason to nit pick when its still in design review @thaJeztah. be nice to @vdemeester

crosbymichael · 2016-10-28T20:21:26Z

Why is this called Names with an s?

@crosbymichael I asked myself the same question 😅. I did it to mimic the docker ps --format which has Names too, I'm fine to rename it to Name 👼

@vdemeester docker ps used to show all the things a container could be referenced as which includes names, as well as link aliases. That's why it is plural.... it might still do that if you use --no-trunc come to think of it.

Definitely singular here is good.

I'll update 👼

boaz0 · 2016-10-31T22:27:28Z

Can't we display both the container name and the container ID?
I believe the container name is more readable than its iD.
Thought? 😊

@ripcurld00d On the debug log ? This change is here to keep the same behavior.

@vdemeester yes, now I see it. s.Container can be the ID or the Name depends on what the user enters.

boaz0 · 2016-10-31T22:30:32Z

@vdemeester @thaJeztah
I think it closes #20973, right?

boaz0 · 2016-10-31T22:34:23Z

If I'm not wrong, in case there is a container called 18f132d776 and another container with ID 18f132d776 then both of them will be displayed.

Is that OK? Maybe adding --container-id can solve this edge-case?
Feedback? 😺

vdemeester · 2016-11-01T02:28:07Z

@ripcurld00d yes you're right, I should update the description. For the name/id collision, I don't think we should do anything 👼.

coolljt0725 · 2016-11-01T07:36:36Z

During review this PR, I found there would be an issue with docker stats byname if user call docker rename to rename the container, the name display in CONTAINER colmun does not change. I think this PR could fix this issue with more changes since this PR return the container name to client(https://github.com/docker/docker/pull/27797/files#diff-ad75d4bbd2897e0abd0207e0f2608bf6R47).

EDIT: open an issue to track this #27937

vdemeester · 2016-11-02T21:27:12Z

@coolljt0725 yep, I'm not really sure how to fix it the best way possible 👼. At some point we should make it so that Container and ID are the same (or Name), and thus not display the user input as Container. But not sure how we should change that (in terms or retro-compat, etc…)…

This adds support to display names or id of container instead of what was provided in the request. This keeps the default behavior (`docker stats byname` will display `byname` in the `CONTAINER` colmun and `docker stats byid` will display the id in the `CONTAINER` column) but adds two new format directive. Signed-off-by: Vincent Demeester <[email protected]>

crosbymichael · 2016-11-09T18:59:48Z

What is the use case for this because adding three different ways have identifying the container is pretty confusing for me and hard to reason about that cons for this.

vdemeester · 2016-11-09T21:08:35Z

@crosbymichael The use case is to be able to display the actual name or id of the container instead of the user input. Right now there is no way to get the name of a container for stats if you don't already know it and pass it to the command (and thus, stats on all containers, you can't, at all 😓).

Keeping Container for retro-compatibility but I really wish I wouldn't have too…

crosbymichael · 2016-11-10T17:19:20Z

@vdemeester is this just an api issue or does the cli format have to change as well? Could we get away with just returning the full container id and not this name stuff since names can change and its weird. Most of the time the name will be the Continaer part anyways right?

colinmollenhour · 2017-03-30T17:40:46Z

This shows merged into 1.13 milestone but it does not appear to be present in 17.03.0-ce. Also, shouldn't "byname" and "byid" be "--byname" and "--byid" since they are options?

Add support for Names and ID in stats format

GordonTheTurtle added the status/0-triage label Oct 26, 2016

vdemeester added status/1-design-review and removed status/0-triage labels Oct 26, 2016

vdemeester added this to the 1.13.0 milestone Oct 26, 2016

MHBauer reviewed Oct 28, 2016

View reviewed changes

crosbymichael reviewed Oct 28, 2016

View reviewed changes

boaz0 reviewed Oct 31, 2016

View reviewed changes

vdemeester force-pushed the stats-format-names-support branch from e44cf35 to 7a1a9c5 Compare November 2, 2016 21:25

vdemeester added the status/failing-ci Indicates that the PR in its current state fails the test suite label Nov 2, 2016

vdemeester force-pushed the stats-format-names-support branch from 7a1a9c5 to ef915fd Compare November 3, 2016 06:20

vdemeester removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Nov 3, 2016

crosbymichael self-assigned this Nov 8, 2016

crosbymichael merged commit 801230c into moby:master Nov 10, 2016

vdemeester deleted the stats-format-names-support branch November 10, 2016 21:26

boaz0 mentioned this pull request Nov 16, 2016

Proposal: Docker stats to show names instead of IDs #20973

Closed

WeiZhang555 mentioned this pull request Dec 26, 2016

Send "Name" and "ID" when stating stopped containers #29702

Merged

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#27797 from vdemeester/stats-format-names-support

6bf79de

Add support for Names and ID in stats format

thaJeztah mentioned this pull request Sep 11, 2025

cli/command/container: prevent panic during stats on empty event Actor.ID docker/cli#6451

Merged

thaJeztah mentioned this pull request Sep 24, 2025

[28.x backport] cli/command/container: prevent panic during stats on empty event Actor.ID docker/cli#6471

Merged

Conversation

vdemeester commented Oct 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MHBauer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boaz0 commented Oct 31, 2016

Uh oh!

boaz0 commented Oct 31, 2016

Uh oh!

vdemeester commented Nov 1, 2016

Uh oh!

coolljt0725 commented Nov 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vdemeester commented Nov 2, 2016

Uh oh!

crosbymichael commented Nov 9, 2016

Uh oh!

vdemeester commented Nov 9, 2016

Uh oh!

crosbymichael commented Nov 10, 2016

Uh oh!

colinmollenhour commented Mar 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

vdemeester commented Oct 26, 2016 •

edited

Loading

coolljt0725 commented Nov 1, 2016 •

edited

Loading