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

show more details of a node's internet connections #1875

Merged
merged 2 commits into from Sep 19, 2016

Conversation

rade
Copy link
Member

@rade rade commented Sep 19, 2016

in the details panel, instead of showing connections to/from the internet as "TheInternet ", we now show "(<ip_addr>) " (or just "<ip_addr> " if we don't have a dns name).

Fixes #1713.

in the details panel, instead of showing connections to/from the
internet as "TheInternet <port>", we now show "<dns-name>(<ip_addr>)
<port>" (or just "<ip_addr> <port>" if we don't have a dns name).

Fixes #1713.
@rade
Copy link
Member Author

rade commented Sep 19, 2016

before:
screenshot_2016-09-19_14-36-36
after:
screenshot_2016-09-19_14-33-25

I chose to denote the source/destination simply by the name/IP, and make them linkable - to TheInternet node. Is this too subtle/confusing? i.e. will users struggle to tell the difference in the list between these links and links to a "real" node? Do we care? NB: this will be made worse with #1863, which presumably denotes pseudo nodes very similar to the nodes here.

So...

  1. leave as is
  2. choose a different style (italics?)
  3. drop the linking, under the assumption that users can always navigate to The Internet node via the topology
  4. render as "TheInternet - name (addr)"
  5. render the "name (addr)" in a separate column

I have pretty much ruled out 4/5 since they eat too much into the horizontal space we have available for display the info that actually matters to users, i.e. the "name (addr)".

@rade
Copy link
Member Author

rade commented Sep 19, 2016

@foot @2opremio any thoughts on the "design" concern above?

return "", false
}
if set, ok := ep.Sets.Lookup(endpoint.ReverseDNSNames); ok && len(set) > 0 {
addr = fmt.Sprintf("%s (%s)", set[0], addr)

This comment was marked as abuse.

This comment was marked as abuse.

@foot
Copy link
Contributor

foot commented Sep 19, 2016

Tough call, I think I'd go for dropping the link.

Unless you feel there's a possible user flow there? But there doesn't seem to be too much utility in clicking through.

I dunno what would be ideal, perhaps all the internet rows grouped under a clickable "the internet" row, sort of tree style.

@foot
Copy link
Contributor

foot commented Sep 19, 2016

I guess you might wanna see what else that aws server is talking too in our scoped world.

Which is a bit rough:

  • click dns/ip label
  • opens "the internet" card.
  • (probably click show-more first) and try and remember what you just clicked, find other things its talking to too.

I guess in that case you'd want individual pseudo node cards. Which could all be grouped on the topo into a super "The Internet" node, as discussed in some other ticket/feature I can't find right now...

In conclusion leaving/removing the link both sound okay for now.

@rade
Copy link
Member Author

rade commented Sep 19, 2016

Unless you feel there's a possible user flow there

Well, it's the same user flow that is there already, i.e. being able to use the detail panel to navigate to adjacent nodes, which, in this case, is the internet node.

How useful that is I do not know.

In conclusion leaving/removing the link both sound okay for now.

ok. Let's leave it for now, and perhaps revisit in #1863.

@rade rade merged commit d008919 into master Sep 19, 2016
@rade rade mentioned this pull request Sep 19, 2016
@rade rade deleted the 1713-show-internet-connections branch July 5, 2017 13:09
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.

None yet

3 participants