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

Ensure k8s views are correctly hidden. #1241

Merged
merged 4 commits into from
Apr 7, 2016

Conversation

tomwilkie
Copy link
Contributor

  • Filter unconnected psuedo nodes from the Pods view
  • Don't report these filtered nodes in stats
  • Fix typo in logic for hiding views

Fixes #1214

- Filter unconnected psuedo nodes from the Pods view
- Don't report these filtered nodes in stats
- Fix typo in logic for hiding views
@@ -58,12 +58,26 @@ func ColorConnected(r Renderer) Renderer {
// Filter removes nodes from a view based on a predicate.
type Filter struct {
Renderer
FilterFunc func(report.Node) bool
FilterFunc func(report.Node) bool
ReportFiltered bool // false means we don't report stats for how many are filtered

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor Author

@paulbellamy PTAL

@paulbellamy
Copy link
Contributor

The number of nodes filtered still seem odd.
screen shot 2016-04-07 at 12 38 23

what 32 nodes are being filtered?

Edit: attached report
report.json.zip

@paulbellamy
Copy link
Contributor

Also, I still think the SilentFilter thing seems overcomplicated and error-prone. e.g. It will still count the internet node being filtered out as part of the container renderer.

@tomwilkie
Copy link
Contributor Author

The number of nodes filtered still seem odd.

I think the stats in the bottom left lag slightly; when I do this I get 39 NODES (7 FILTERED) (which is still wrong).

@paulbellamy
Copy link
Contributor

Mine's been showing 12 nodes (34 filtered) for 28 minutes. When there are 39 nodes visible...

@tomwilkie
Copy link
Contributor Author

Try refreshing? Otherwise its a UI bug.

@paulbellamy
Copy link
Contributor

Ah yeah. Now I get 39 nodes (6 filtered), which is still wrong. It occurs to me that maybe any filters used in render/topologies.go should be silent, and only the ones applied (at the end) by the UI widget should count towards the (X filtered) statistic.

@tomwilkie
Copy link
Contributor Author

@paulbellamy yup, I agree. Have made them all silent now; which also means we need to silently filter some non-pseudo nodes. WDYT?

@paulbellamy
Copy link
Contributor

LGTM. I take back what I said about SilentFilter not being the right abstraction. When applied to all the internal filters, it's great.

@paulbellamy paulbellamy merged commit 201c290 into master Apr 7, 2016
@paulbellamy paulbellamy deleted the 1214-hide-views-proper-like branch April 7, 2016 12:45
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.

2 participants