From 7a8e61454f842f3c986ff40d3f92be24c519e356 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Tue, 10 May 2016 14:14:15 +0000 Subject: [PATCH 1/4] Limit tables to 20 rows --- report/table.go | 43 ++++++++++++++++++++++++++++++++----------- report/table_test.go | 6 +++++- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/report/table.go b/report/table.go index 25052640b8..964da308dd 100644 --- a/report/table.go +++ b/report/table.go @@ -7,31 +7,51 @@ import ( "github.com/weaveworks/scope/common/mtime" ) +// MaxTableRows sets the limit on the table size to render +// TODO: this won't be needed once we send reports incrementally +const MaxTableRows = 20 + // AddTable appends arbirary key-value pairs to the Node, returning a new node. func (node Node) AddTable(prefix string, labels map[string]string) Node { + count := 0 for key, value := range labels { + // It's enough to only include MaxTableRows+1 + // since they won't be rendered anyhow + if count > MaxTableRows { + break + } node = node.WithLatest(prefix+key, mtime.Now(), value) + count++ + } return node } // ExtractTable returns the key-value pairs with the given prefix from this Node, -func (node Node) ExtractTable(prefix string) map[string]string { - result := map[string]string{} +func (node Node) ExtractTable(prefix string) (rows map[string]string, truncated bool) { + rows = map[string]string{} + truncated = false + count := 0 node.Latest.ForEach(func(key, value string) { if strings.HasPrefix(key, prefix) { + if count >= MaxTableRows { + truncated = true + return + } label := key[len(prefix):] - result[label] = value + rows[label] = value + count++ } }) - return result + return rows, truncated } // Table is the type for a table in the UI. type Table struct { - ID string `json:"id"` - Label string `json:"label"` - Rows []MetadataRow `json:"rows"` + ID string `json:"id"` + Label string `json:"label"` + Rows []MetadataRow `json:"rows"` + WasTruncated bool `json:"was_truncated"` } type tablesByID []Table @@ -90,12 +110,13 @@ type TableTemplates map[string]TableTemplate func (t TableTemplates) Tables(node Node) []Table { var result []Table for _, template := range t { + rows, truncated := node.ExtractTable(template.Prefix) table := Table{ - ID: template.ID, - Label: template.Label, - Rows: []MetadataRow{}, + ID: template.ID, + Label: template.Label, + Rows: []MetadataRow{}, + WasTruncated: truncated, } - rows := node.ExtractTable(template.Prefix) keys := make([]string, 0, len(rows)) for k := range rows { keys = append(keys, k) diff --git a/report/table_test.go b/report/table_test.go index b1866b072a..7102995f88 100644 --- a/report/table_test.go +++ b/report/table_test.go @@ -16,7 +16,11 @@ func TestTables(t *testing.T) { nmd := report.MakeNode("foo1") nmd = nmd.AddTable("foo_", want) - have := nmd.ExtractTable("foo_") + have, truncated := nmd.ExtractTable("foo_") + + if truncated { + t.Error("Table shouldn't had been truncated") + } if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) From 100f78d77166e5c377bd588445d2b072defb3a44 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Tue, 10 May 2016 16:35:35 +0000 Subject: [PATCH 2/4] Provide truncation counts to the UI --- report/table.go | 51 ++++++++++++++++++++++++-------------------- report/table_test.go | 29 +++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 25 deletions(-) diff --git a/report/table.go b/report/table.go index 964da308dd..6b5346d908 100644 --- a/report/table.go +++ b/report/table.go @@ -1,57 +1,62 @@ package report import ( + "fmt" "sort" "strings" + log "github.com/Sirupsen/logrus" "github.com/weaveworks/scope/common/mtime" ) // MaxTableRows sets the limit on the table size to render // TODO: this won't be needed once we send reports incrementally -const MaxTableRows = 20 +const ( + MaxTableRows = 20 + TruncationCountPrefix = "table_truncation_count_" +) // AddTable appends arbirary key-value pairs to the Node, returning a new node. func (node Node) AddTable(prefix string, labels map[string]string) Node { count := 0 for key, value := range labels { - // It's enough to only include MaxTableRows+1 - // since they won't be rendered anyhow - if count > MaxTableRows { + if count >= MaxTableRows { break } node = node.WithLatest(prefix+key, mtime.Now(), value) count++ - + } + if len(labels) > MaxTableRows { + truncationCount := fmt.Sprintf("%d", len(labels)-MaxTableRows) + node = node.WithLatest(TruncationCountPrefix+prefix, mtime.Now(), truncationCount) } return node } // ExtractTable returns the key-value pairs with the given prefix from this Node, -func (node Node) ExtractTable(prefix string) (rows map[string]string, truncated bool) { +func (node Node) ExtractTable(prefix string) (rows map[string]string, truncationCount int) { rows = map[string]string{} - truncated = false - count := 0 + truncationCount = 0 node.Latest.ForEach(func(key, value string) { if strings.HasPrefix(key, prefix) { - if count >= MaxTableRows { - truncated = true - return - } label := key[len(prefix):] rows[label] = value - count++ } }) - return rows, truncated + if str, ok := node.Latest.Lookup(TruncationCountPrefix + prefix); ok { + if n, err := fmt.Sscanf(str, "%d", &truncationCount); n != 1 || err != nil { + log.Warn("Unexpected truncation count format %q", str) + } + } + return rows, truncationCount } // Table is the type for a table in the UI. type Table struct { - ID string `json:"id"` - Label string `json:"label"` - Rows []MetadataRow `json:"rows"` - WasTruncated bool `json:"was_truncated"` + ID string `json:"id"` + Label string `json:"label"` + Rows []MetadataRow `json:"rows"` + TruncationCount int `json:"truncation_count,omitempty"` } type tablesByID []Table @@ -110,12 +115,12 @@ type TableTemplates map[string]TableTemplate func (t TableTemplates) Tables(node Node) []Table { var result []Table for _, template := range t { - rows, truncated := node.ExtractTable(template.Prefix) + rows, truncationCount := node.ExtractTable(template.Prefix) table := Table{ - ID: template.ID, - Label: template.Label, - Rows: []MetadataRow{}, - WasTruncated: truncated, + ID: template.ID, + Label: template.Label, + Rows: []MetadataRow{}, + TruncationCount: truncationCount, } keys := make([]string, 0, len(rows)) for k := range rows { diff --git a/report/table_test.go b/report/table_test.go index 7102995f88..cf276ba75c 100644 --- a/report/table_test.go +++ b/report/table_test.go @@ -1,6 +1,7 @@ package report_test import ( + "fmt" "reflect" "testing" @@ -16,9 +17,9 @@ func TestTables(t *testing.T) { nmd := report.MakeNode("foo1") nmd = nmd.AddTable("foo_", want) - have, truncated := nmd.ExtractTable("foo_") + have, truncationCount := nmd.ExtractTable("foo_") - if truncated { + if truncationCount != 0 { t.Error("Table shouldn't had been truncated") } @@ -26,3 +27,27 @@ func TestTables(t *testing.T) { t.Error(test.Diff(want, have)) } } + +func TestTruncation(t *testing.T) { + wantTruncationCount := 1 + want := map[string]string{} + for i := 0; i < report.MaxTableRows+wantTruncationCount; i++ { + key := fmt.Sprintf("key%d", i) + value := fmt.Sprintf("value%d", i) + want[key] = value + } + + nmd := report.MakeNode("foo1") + + nmd = nmd.AddTable("foo_", want) + _, truncationCount := nmd.ExtractTable("foo_") + + if truncationCount != wantTruncationCount { + t.Error( + "Table should had been truncated by", + wantTruncationCount, + "and not", + truncationCount, + ) + } +} From 645f2ca9f4c7a29935537239e0ababd3745684ec Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Wed, 11 May 2016 13:05:19 +0200 Subject: [PATCH 3/4] Add hint that a section may be truncated --- client/app/scripts/components/node-details.js | 22 +++++++++++++++---- client/app/scripts/components/warning.js | 7 ++++++ client/app/styles/main.less | 8 +++++++ report/table.go | 2 +- 4 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 client/app/scripts/components/warning.js diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index 6c566c3722..a9d6cbcc3a 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -2,15 +2,23 @@ import _ from 'lodash'; import React from 'react'; import { connect } from 'react-redux'; +import { clickCloseDetails, clickShowTopologyForNode } from '../actions/app-actions'; +import { brightenColor, getNeutralColor, getNodeColorDark } from '../utils/color-utils'; +import { resetDocumentTitle, setDocumentTitle } from '../utils/title-utils'; + import NodeDetailsControls from './node-details/node-details-controls'; import NodeDetailsHealth from './node-details/node-details-health'; import NodeDetailsInfo from './node-details/node-details-info'; import NodeDetailsLabels from './node-details/node-details-labels'; import NodeDetailsRelatives from './node-details/node-details-relatives'; import NodeDetailsTable from './node-details/node-details-table'; -import { clickCloseDetails, clickShowTopologyForNode } from '../actions/app-actions'; -import { brightenColor, getNeutralColor, getNodeColorDark } from '../utils/color-utils'; -import { resetDocumentTitle, setDocumentTitle } from '../utils/title-utils'; +import Warning from './warning'; + +function getTruncationText(label, count) { + return `The section ${label} has been truncated because of too many entries. +${count} entries are not shown. +We are working on making this better.`; +} export class NodeDetails extends React.Component { @@ -196,7 +204,13 @@ export class NodeDetails extends React.Component { if (table.rows.length > 0) { return (
-
{table.label}
+
+ {table.label} + {table.truncationCount > 0 && + + } +
); diff --git a/client/app/scripts/components/warning.js b/client/app/scripts/components/warning.js new file mode 100644 index 0000000000..d79731a918 --- /dev/null +++ b/client/app/scripts/components/warning.js @@ -0,0 +1,7 @@ +import React from 'react'; + +export default function Warning({text}) { + return ( + + ); +} diff --git a/client/app/styles/main.less b/client/app/styles/main.less index 321468f475..a54b4e2d58 100644 --- a/client/app/styles/main.less +++ b/client/app/styles/main.less @@ -649,6 +649,10 @@ h2 { font-size: 90%; color: @text-tertiary-color; padding: 4px 0; + + &-warning { + padding: 0 0.5em; + } } } } @@ -1098,6 +1102,10 @@ h2 { } } +.warning { + .btn-opacity; +} + .sidebar { position: fixed; bottom: 16px; diff --git a/report/table.go b/report/table.go index 6b5346d908..6ab4f5a20e 100644 --- a/report/table.go +++ b/report/table.go @@ -56,7 +56,7 @@ type Table struct { ID string `json:"id"` Label string `json:"label"` Rows []MetadataRow `json:"rows"` - TruncationCount int `json:"truncation_count,omitempty"` + TruncationCount int `json:"truncationCount,omitempty"` } type tablesByID []Table From c265a57672664146ba411596947f92a17c75b97a Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Wed, 11 May 2016 16:55:57 +0200 Subject: [PATCH 4/4] Show truncation warning on click, reworded --- client/app/scripts/components/node-details.js | 9 ++--- client/app/scripts/components/warning.js | 40 +++++++++++++++++-- client/app/styles/main.less | 38 +++++++++++++++--- 3 files changed, 73 insertions(+), 14 deletions(-) diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index a9d6cbcc3a..6bd6017e32 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -14,10 +14,9 @@ import NodeDetailsRelatives from './node-details/node-details-relatives'; import NodeDetailsTable from './node-details/node-details-table'; import Warning from './warning'; -function getTruncationText(label, count) { - return `The section ${label} has been truncated because of too many entries. -${count} entries are not shown. -We are working on making this better.`; +function getTruncationText(count) { + return 'This section was too long to be handled efficiently and has been truncated' + + ` (${count} extra entries not included). We are working to remove this limitation.`; } export class NodeDetails extends React.Component { @@ -208,7 +207,7 @@ export class NodeDetails extends React.Component { {table.label} {table.truncationCount > 0 && - + } diff --git a/client/app/scripts/components/warning.js b/client/app/scripts/components/warning.js index d79731a918..161c68d5ab 100644 --- a/client/app/scripts/components/warning.js +++ b/client/app/scripts/components/warning.js @@ -1,7 +1,39 @@ import React from 'react'; +import classnames from 'classnames'; -export default function Warning({text}) { - return ( - - ); + +class Warning extends React.Component { + + constructor(props, context) { + super(props, context); + this.handleClick = this.handleClick.bind(this); + this.state = { + expanded: false + }; + } + + handleClick() { + const expanded = !this.state.expanded; + this.setState({ expanded }); + } + + render() { + const { text } = this.props; + const { expanded } = this.state; + + const className = classnames('warning', { + 'warning-expanded': expanded + }); + + return ( +
+
+ + {expanded && {text}} +
+
+ ); + } } + +export default Warning; diff --git a/client/app/styles/main.less b/client/app/styles/main.less index a54b4e2d58..43132ba893 100644 --- a/client/app/styles/main.less +++ b/client/app/styles/main.less @@ -649,10 +649,6 @@ h2 { font-size: 90%; color: @text-tertiary-color; padding: 4px 0; - - &-warning { - padding: 0 0.5em; - } } } } @@ -1103,7 +1099,39 @@ h2 { } .warning { - .btn-opacity; + display: inline-block; + cursor: pointer; + border: 1px dashed transparent; + text-transform: none; + border-radius: @border-radius; + margin-left: 4px; + + &-wrapper { + display: flex; + } + + &-text { + display: inline-block; + color: @text-secondary-color; + padding-left: 0.5em; + } + + &-icon { + .btn-opacity; + } + + &-expanded { + margin-left: 0; + padding: 2px 4px; + border-color: @text-tertiary-color; + } + + &-expanded &-icon { + position: relative; + top: 4px; + left: 2px; + } + } .sidebar {