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

Render: add ContainerName into output node when rendering processes #3212

Closed
wants to merge 1 commit into from

Conversation

bboreham
Copy link
Collaborator

@bboreham bboreham commented Jun 4, 2018

This was a bug - we spend eight lines fetching the name, then didn't assign the new node into the variable.

Extracted from #3137

This was a bug - we spend eight lines fetching the name, then didn't
assign the new node into the variable.
@rade
Copy link
Member

rade commented Jun 4, 2018

What user-visible change does this entail? A 'Container Name' item in the 'Info' section of a process detail panel? Seems kinda superfluous - we already have a link to the container in the parents section. Also, we don't have an equivalent field for the container-to-pod relationship.

@bboreham
Copy link
Collaborator Author

bboreham commented Jun 4, 2018

The feature was added in response to a user request - #331.
I noticed there is also a parent relationship, but I haven't looked into how that gets there.

I guess since it's been broken for five months we could just delete the label (and rename ProcessWithContainerNameRenderer).

@rade
Copy link
Member

rade commented Jun 4, 2018

I see; the container name is incorporated in the minor label. Usually it will be hidden and only visible on hover, since the host name, which precedes it in the label, takes up all the space. In fact it looks like there is a rendering bug and the minor label isn't rendered fully even on hover. Anyway, I'd be in favour of ditching the container name from the metadata and hence label.

@2opremio
Copy link
Contributor

Closing in favor of #3263

@2opremio 2opremio closed this Jul 11, 2018
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