Skip to content

Group pseudo nodes by the node the referred to and their address in grouped topologies.#116

Merged
tomwilkie merged 4 commits intoweaveworks:masterfrom
tomwilkie:105-grouping-pseudo
May 27, 2015
Merged

Group pseudo nodes by the node the referred to and their address in grouped topologies.#116
tomwilkie merged 4 commits intoweaveworks:masterfrom
tomwilkie:105-grouping-pseudo

Conversation

@tomwilkie
Copy link
Contributor

@tomwilkie tomwilkie commented May 27, 2015

Fixes #105

@tomwilkie tomwilkie changed the title Group pseudo nodes by the node the referred to and their address in g… Group pseudo nodes by the node the referred to and their address in grouped topologies. May 27, 2015
@peterbourgon
Copy link
Contributor

peterbourgon commented May 27, 2015

I don't think it's OK to have this behavior be default. Again, I want to see pseudonodes in the general case. The case where Chrome is opening a bunch of short-lived connections is an outlier.

Oops, misread.

peterbourgon
peterbourgon reviewed May 27, 2015
View reviewed changes
report/topology_test.go Outdated

This comment was marked as abuse.

Sign in to view

@tomwilkie
Copy link
Contributor Author

tomwilkie commented May 27, 2015

This change is for grouped topologies, and stops all pseudo nodes being merged into one. So it adds more pseudo nodes....

@tomwilkie
Copy link
Contributor Author

tomwilkie commented May 27, 2015

And yes, in general I'm factoring out the pseudo node handling to be per-topology, so different topologies can have different concepts for pseudo nodes. So for the pid topology, we can show more, for containers, less, and hosts is pretty obvious.

@peterbourgon
Copy link
Contributor

peterbourgon commented May 27, 2015

Great! I noticed the formatLabel thing is basically dead code at this point, I'll remove that in another PR. 👍

@tomwilkie tomwilkie force-pushed the 105-grouping-pseudo branch from 7184c09 to 5a8a76e Compare May 27, 2015 09:19
@tomwilkie
Copy link
Contributor Author

tomwilkie commented May 27, 2015

PTAL; I've factored out the pseudo node handling (hopefully with no semantic change)

peterbourgon
peterbourgon reviewed May 27, 2015
View reviewed changes
report/mapping_functions.go Outdated

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@peterbourgon
Copy link
Contributor

peterbourgon commented May 27, 2015

Yes, this is a brilliant change. 💯

@tomwilkie tomwilkie force-pushed the 105-grouping-pseudo branch from d14a9af to f4dcf93 Compare May 27, 2015 09:41
@tomwilkie
Copy link
Contributor Author

tomwilkie commented May 27, 2015

Okay one more look pls.

@tomwilkie
Copy link
Contributor Author

tomwilkie commented May 27, 2015

Also fixes #113

@tomwilkie tomwilkie force-pushed the 105-grouping-pseudo branch from f4dcf93 to 3e96983 Compare May 27, 2015 09:56
@tomwilkie tomwilkie mentioned this pull request May 27, 2015
Closed
peterbourgon
peterbourgon reviewed May 27, 2015
View reviewed changes
report/mapping_functions.go Outdated

This comment was marked as abuse.

Sign in to view

@peterbourgon
Copy link
Contributor

peterbourgon commented May 27, 2015

👍

@tomwilkie tomwilkie force-pushed the 105-grouping-pseudo branch from 3e96983 to 0b786ae Compare May 27, 2015 10:05
tomwilkie added a commit that referenced this pull request May 27, 2015
@tomwilkie
Merge pull request #116 from tomwilkie/105-grouping-pseudo
68a11db 
Group pseudo nodes by the node the referred to and their address in grouped topologies.
@tomwilkie tomwilkie merged commit 68a11db into weaveworks:master May 27, 2015
@tomwilkie tomwilkie deleted the 105-grouping-pseudo branch May 27, 2015 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@peterbourgon peterbourgon

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

All pseudo nodes are grouped together in the "grouped" view

2 participants

@tomwilkie @peterbourgon