From 31be525bd2ad19b9d490d972d819fb576f6d5f3d Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Thu, 22 Dec 2016 17:40:42 +0100 Subject: [PATCH 1/4] Created generic table model on backend Replaced MetadataRow with generic Row in Table model Sending through multicolumn tables from the backend --- .../node-details/node-details-labels.js | 9 +- .../utils/__tests__/search-utils-test.js | 14 +- client/app/scripts/utils/search-utils.js | 5 +- probe/docker/container.go | 4 +- probe/docker/reporter.go | 2 +- probe/kubernetes/meta.go | 2 +- probe/overlay/weave.go | 103 +++++++----- render/detailed/tables_test.go | 14 +- report/table.go | 159 +++++++++++++----- report/table_test.go | 4 +- 10 files changed, 208 insertions(+), 108 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-labels.js b/client/app/scripts/components/node-details/node-details-labels.js index 6823f1330a..2fd01a4829 100644 --- a/client/app/scripts/components/node-details/node-details-labels.js +++ b/client/app/scripts/components/node-details/node-details-labels.js @@ -6,7 +6,6 @@ import MatchedText from '../matched-text'; import NodeDetailsControlButton from './node-details-control-button'; import ShowMore from '../show-more'; - const Controls = controls => (
{sortBy(controls, 'rank').map(control =>
- {field.label} + title={field.entries.label} key={field.id}> + {field.entries.label}
-
- +
+
))} diff --git a/client/app/scripts/utils/__tests__/search-utils-test.js b/client/app/scripts/utils/__tests__/search-utils-test.js index 301b431a2c..1596e3d626 100644 --- a/client/app/scripts/utils/__tests__/search-utils-test.js +++ b/client/app/scripts/utils/__tests__/search-utils-test.js @@ -30,9 +30,17 @@ describe('SearchUtils', () => { tables: [{ id: 'metric1', rows: [{ - id: 'row1', - label: 'Row 1', - value: 'Row Value 1' + entries: { + id: 'row1', + label: 'Row 1', + value: 'Row Value 1' + } + }, { + entries: { + id: 'row2', + label: 'Row 2', + value: 'Row Value 2' + } }] }], }, diff --git a/client/app/scripts/utils/search-utils.js b/client/app/scripts/utils/search-utils.js index b8f74d0471..c297ae96a9 100644 --- a/client/app/scripts/utils/search-utils.js +++ b/client/app/scripts/utils/search-utils.js @@ -154,9 +154,10 @@ export function searchTopology(nodes, { prefix, query, metric, comp, value }) { tables.forEach((table) => { if (table.get('rows')) { table.get('rows').forEach((field) => { + const entries = field.get('entries'); const keyPath = [nodeId, 'tables', field.get('id')]; - nodeMatches = findNodeMatch(nodeMatches, keyPath, field.get('value'), - query, prefix, field.get('label')); + nodeMatches = findNodeMatch(nodeMatches, keyPath, entries.get('value'), + query, prefix, entries.get('label')); }); } }); diff --git a/probe/docker/container.go b/probe/docker/container.go index 50856ee0fa..f23ed71fd3 100644 --- a/probe/docker/container.go +++ b/probe/docker/container.go @@ -379,8 +379,8 @@ func (c *container) getBaseNode() report.Node { }).WithParents(report.EmptySets. Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(c.Image()))), ) - result = result.AddPrefixTable(LabelPrefix, c.container.Config.Labels) - result = result.AddPrefixTable(EnvPrefix, c.env()) + result = result.AddPrefixLabels(LabelPrefix, c.container.Config.Labels) + result = result.AddPrefixLabels(EnvPrefix, c.env()) return result } diff --git a/probe/docker/reporter.go b/probe/docker/reporter.go index b1e2eb75c8..accd183015 100644 --- a/probe/docker/reporter.go +++ b/probe/docker/reporter.go @@ -258,7 +258,7 @@ func (r *Reporter) containerImageTopology() report.Topology { } nodeID := report.MakeContainerImageNodeID(imageID) node := report.MakeNodeWith(nodeID, latests) - node = node.AddPrefixTable(ImageLabelPrefix, image.Labels) + node = node.AddPrefixLabels(ImageLabelPrefix, image.Labels) result.AddNode(node) }) diff --git a/probe/kubernetes/meta.go b/probe/kubernetes/meta.go index c7db23d0c7..20fb213bb5 100644 --- a/probe/kubernetes/meta.go +++ b/probe/kubernetes/meta.go @@ -56,5 +56,5 @@ func (m meta) MetaNode(id string) report.Node { Name: m.Name(), Namespace: m.Namespace(), Created: m.Created(), - }).AddPrefixTable(LabelPrefix, m.Labels()) + }).AddPrefixLabels(LabelPrefix, m.Labels()) } diff --git a/probe/overlay/weave.go b/probe/overlay/weave.go index 819d88eb5a..13e34412ce 100644 --- a/probe/overlay/weave.go +++ b/probe/overlay/weave.go @@ -19,34 +19,35 @@ import ( // Keys for use in Node const ( - WeavePeerName = "weave_peer_name" - WeavePeerNickName = "weave_peer_nick_name" - WeaveDNSHostname = "weave_dns_hostname" - WeaveMACAddress = "weave_mac_address" - WeaveVersion = "weave_version" - WeaveEncryption = "weave_encryption" - WeaveProtocol = "weave_protocol" - WeavePeerDiscovery = "weave_peer_discovery" - WeaveTargetCount = "weave_target_count" - WeaveConnectionCount = "weave_connection_count" - WeavePeerCount = "weave_peer_count" - WeaveTrustedSubnets = "weave_trusted_subnet_count" - WeaveIPAMTableID = "weave_ipam_table" - WeaveIPAMStatus = "weave_ipam_status" - WeaveIPAMRange = "weave_ipam_range" - WeaveIPAMDefaultSubnet = "weave_ipam_default_subnet" - WeaveDNSTableID = "weave_dns_table" - WeaveDNSDomain = "weave_dns_domain" - WeaveDNSUpstream = "weave_dns_upstream" - WeaveDNSTTL = "weave_dns_ttl" - WeaveDNSEntryCount = "weave_dns_entry_count" - WeaveProxyTableID = "weave_proxy_table" - WeaveProxyStatus = "weave_proxy_status" - WeaveProxyAddress = "weave_proxy_address" - WeavePluginTableID = "weave_plugin_table" - WeavePluginStatus = "weave_plugin_status" - WeavePluginDriver = "weave_plugin_driver" - WeaveConnectionsTablePrefix = "weave_connections_table_" + WeavePeerName = "weave_peer_name" + WeavePeerNickName = "weave_peer_nick_name" + WeaveDNSHostname = "weave_dns_hostname" + WeaveMACAddress = "weave_mac_address" + WeaveVersion = "weave_version" + WeaveEncryption = "weave_encryption" + WeaveProtocol = "weave_protocol" + WeavePeerDiscovery = "weave_peer_discovery" + WeaveTargetCount = "weave_target_count" + WeaveConnectionCount = "weave_connection_count" + WeavePeerCount = "weave_peer_count" + WeaveTrustedSubnets = "weave_trusted_subnet_count" + WeaveIPAMTableID = "weave_ipam_table" + WeaveIPAMStatus = "weave_ipam_status" + WeaveIPAMRange = "weave_ipam_range" + WeaveIPAMDefaultSubnet = "weave_ipam_default_subnet" + WeaveDNSTableID = "weave_dns_table" + WeaveDNSDomain = "weave_dns_domain" + WeaveDNSUpstream = "weave_dns_upstream" + WeaveDNSTTL = "weave_dns_ttl" + WeaveDNSEntryCount = "weave_dns_entry_count" + WeaveProxyTableID = "weave_proxy_table" + WeaveProxyStatus = "weave_proxy_status" + WeaveProxyAddress = "weave_proxy_address" + WeavePluginTableID = "weave_plugin_table" + WeavePluginStatus = "weave_plugin_status" + WeavePluginDriver = "weave_plugin_driver" + WeaveConnectionsTablePrefix = "weave_connections_table_" + WeaveConnectionsMulticolumnTablePrefix = "weave_connections_multicolumn_table_" ) var ( @@ -73,14 +74,20 @@ var ( } weaveTableTemplates = report.TableTemplates{ - WeaveIPAMTableID: {ID: WeaveIPAMTableID, Label: "IPAM", + WeaveIPAMTableID: { + ID: WeaveIPAMTableID, + Label: "IPAM", + Type: report.PropertyListType, FixedRows: map[string]string{ WeaveIPAMStatus: "Status", WeaveIPAMRange: "Range", WeaveIPAMDefaultSubnet: "Default Subnet", }, }, - WeaveDNSTableID: {ID: WeaveDNSTableID, Label: "DNS", + WeaveDNSTableID: { + ID: WeaveDNSTableID, + Label: "DNS", + Type: report.PropertyListType, FixedRows: map[string]string{ WeaveDNSDomain: "Domain", WeaveDNSUpstream: "Upstream", @@ -88,13 +95,19 @@ var ( WeaveDNSEntryCount: "Entries", }, }, - WeaveProxyTableID: {ID: WeaveProxyTableID, Label: "Proxy", + WeaveProxyTableID: { + ID: WeaveProxyTableID, + Label: "Proxy", + Type: report.PropertyListType, FixedRows: map[string]string{ WeaveProxyStatus: "Status", WeaveProxyAddress: "Address", }, }, - WeavePluginTableID: {ID: WeavePluginTableID, Label: "Plugin", + WeavePluginTableID: { + ID: WeavePluginTableID, + Label: "Plugin", + Type: report.PropertyListType, FixedRows: map[string]string{ WeavePluginStatus: "Status", WeavePluginDriver: "Driver Name", @@ -103,8 +116,15 @@ var ( WeaveConnectionsTablePrefix: { ID: WeaveConnectionsTablePrefix, Label: "Connections", + Type: report.PropertyListType, Prefix: WeaveConnectionsTablePrefix, }, + WeaveConnectionsMulticolumnTablePrefix: { + ID: WeaveConnectionsMulticolumnTablePrefix, + Label: "Connections", + Type: report.MulticolumnTableType, + Prefix: WeaveConnectionsMulticolumnTablePrefix, + }, } ) @@ -434,28 +454,31 @@ func (w *Weave) addCurrentPeerInfo(latests map[string]string, node report.Node) latests[WeavePluginStatus] = "running" latests[WeavePluginDriver] = "weave" } - node = node.AddPrefixTable(WeaveConnectionsTablePrefix, getConnectionsTable(w.statusCache.Router)) + node = node.AddPrefixTable(WeaveConnectionsMulticolumnTablePrefix, getConnectionsTable(w.statusCache.Router)) node = node.WithParents(report.EmptySets.Add(report.Host, report.MakeStringSet(w.hostID))) return latests, node } -func getConnectionsTable(router weave.Router) map[string]string { +func getConnectionsTable(router weave.Router) []report.Row { const ( outboundArrow = "->" inboundArrow = "<-" ) - table := make(map[string]string, len(router.Connections)) + table := make([]report.Row, len(router.Connections)) for _, conn := range router.Connections { arrow := inboundArrow if conn.Outbound { arrow = outboundArrow } - // TODO: we should probably use a multicolumn table for this - // but there is no mechanism to support it yet. - key := fmt.Sprintf("%s %s", arrow, conn.Address) - value := fmt.Sprintf("%s, %s", conn.State, conn.Info) - table[key] = value + table = append(table, report.Row{ + ID: conn.Address, + Entries: map[string]string{ + "ip": fmt.Sprintf("%s %s", arrow, conn.Address), + "state": conn.State, + "info": conn.Info, + }, + }) } return table } diff --git a/render/detailed/tables_test.go b/render/detailed/tables_test.go index 02f723391f..634755c545 100644 --- a/render/detailed/tables_test.go +++ b/render/detailed/tables_test.go @@ -35,23 +35,25 @@ func TestNodeTables(t *testing.T) { { ID: docker.EnvPrefix, Label: "Environment Variables", - Rows: []report.MetadataRow{}, + Rows: []report.Row{}, }, { ID: docker.LabelPrefix, Label: "Docker Labels", - Rows: []report.MetadataRow{ + Rows: []report.Row{ { - ID: "label_label1", - Label: "label1", - Value: "label1value", + Entries: map[string]string{ + "id": "label_label1", + "label": "label1", + "value": "label1value", + }, }, }, }, { ID: docker.ImageTableID, Label: "Image", - Rows: []report.MetadataRow{}, + Rows: []report.Row{}, }, }, }, diff --git a/report/table.go b/report/table.go index 74cbc1a71f..791fe62eaf 100644 --- a/report/table.go +++ b/report/table.go @@ -15,10 +15,33 @@ import ( const ( MaxTableRows = 20 TruncationCountPrefix = "table_truncation_count_" + MulticolumnTableType = "multicolumn-table" + PropertyListType = "property-list" ) -// AddPrefixTable appends arbitrary key-value pairs to the Node, returning a new node. -func (node Node) AddPrefixTable(prefix string, labels map[string]string) Node { +// AddPrefixTable appends arbitrary rows to the Node, returning a new node. +func (node Node) AddPrefixTable(prefix string, rows []Row) Node { + count := 0 + for _, row := range rows { + if count >= MaxTableRows { + break + } + // TODO: Figure a more natural way of storing rows + for column, value := range row.Entries { + key := fmt.Sprintf("%s %s", row.ID, column) + node = node.WithLatest(prefix+key, mtime.Now(), value) + } + count++ + } + if len(rows) > MaxTableRows { + truncationCount := fmt.Sprintf("%d", len(rows)-MaxTableRows) + node = node.WithLatest(TruncationCountPrefix+prefix, mtime.Now(), truncationCount) + } + return node +} + +// AddPrefixLabels appends arbitrary key-value pairs to the Node, returning a new node. +func (node Node) AddPrefixLabels(prefix string, labels map[string]string) Node { count := 0 for key, value := range labels { if count >= MaxTableRows { @@ -34,33 +57,86 @@ func (node Node) AddPrefixTable(prefix string, labels map[string]string) Node { return node } -// ExtractTable returns the key-value pairs to build a table from this node -func (node Node) ExtractTable(template TableTemplate) (rows map[string]string, truncationCount int) { - rows = map[string]string{} - truncationCount = 0 - node.Latest.ForEach(func(key string, _ time.Time, value string) { - if label, ok := template.FixedRows[key]; ok { - rows[label] = value +// ExtractTable returns the rows to build a table from this node +func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCount int) { + rows = []Row{} + switch template.Type { + case MulticolumnTableType: + keyRows := map[string]Row{} + node.Latest.ForEach(func(key string, _ time.Time, value string) { + if len(template.Prefix) > 0 && strings.HasPrefix(key, template.Prefix) { + rowID, column := "", "" + fmt.Sscanf(key[len(template.Prefix):], "%s %s", &rowID, &column) + if _, ok := keyRows[rowID]; !ok { + keyRows[rowID] = Row{ + ID: rowID, + Entries: map[string]string{}, + } + } + keyRows[rowID].Entries[column] = value + } + }) + for _, row := range keyRows { + rows = append(rows, row) + } + // By default assume it's a property list (for backward compatibility) + default: + keyValues := map[string]string{} + node.Latest.ForEach(func(key string, _ time.Time, value string) { + if label, ok := template.FixedRows[key]; ok { + keyValues[label] = value + } + if len(template.Prefix) > 0 && strings.HasPrefix(key, template.Prefix) { + label := key[len(template.Prefix):] + keyValues[label] = value + } + }) + labels := make([]string, 0, len(rows)) + for label := range keyValues { + labels = append(labels, label) } - if len(template.Prefix) > 0 && strings.HasPrefix(key, template.Prefix) { - label := key[len(template.Prefix):] - rows[label] = value + sort.Strings(labels) + for _, label := range labels { + rows = append(rows, Row{ + ID: "label_" + label, + Entries: map[string]string{ + "label": label, + "value": keyValues[label], + }, + }) } - }) + } + + truncationCount = 0 if str, ok := node.Latest.Lookup(TruncationCountPrefix + template.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 } +type Column struct { + ID string `json:"id"` + Label string `json:"label"` + DataType string `json:"dataType"` + Alignment string `json:"alignment"` +} + +type Row struct { + ID string `json:"id"` + Entries map[string]string `json:"entries"` +} + // 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"` - TruncationCount int `json:"truncationCount,omitempty"` + ID string `json:"id"` + Label string `json:"label"` + Type string `json:"type"` + Columns []Column `json:"columns"` + Rows []Row `json:"rows"` + TruncationCount int `json:"truncationCount,omitempty"` } type tablesByID []Table @@ -72,9 +148,14 @@ func (t tablesByID) Less(i, j int) bool { return t[i].ID < t[j].ID } // Copy returns a copy of the Table. func (t Table) Copy() Table { result := Table{ - ID: t.ID, - Label: t.Label, - Rows: make([]MetadataRow, 0, len(t.Rows)), + ID: t.ID, + Label: t.Label, + Type: t.Type, + Columns: make([]Column, 0, len(t.Columns)), + Rows: make([]Row, 0, len(t.Rows)), + } + for _, column := range t.Columns { + result.Columns = append(result.Columns, column) } for _, row := range t.Rows { result.Rows = append(result.Rows, row) @@ -82,18 +163,13 @@ func (t Table) Copy() Table { return result } -// FixedRow describes a row which is part of a TableTemplate and whose value is extracted -// from a predetermined key -type FixedRow struct { - Label string `json:"label"` - Key string `json:"key"` -} - // TableTemplate describes how to render a table for the UI. type TableTemplate struct { - ID string `json:"id"` - Label string `json:"label"` - Prefix string `json:"prefix"` + ID string `json:"id"` + Label string `json:"label"` + Prefix string `json:"prefix"` + Type string `json:"type"` + Columns []Column `json:"columns"` // FixedRows indicates what predetermined rows to render each entry is // indexed by the key to extract the row value is mapped to the row // label @@ -126,10 +202,13 @@ func (t TableTemplate) Merge(other TableTemplate) TableTemplate { fixedRows = other.FixedRows } + // TODO: Refactor the merging logic, as mixing + // the types now might result in invalid tables. return TableTemplate{ ID: max(t.ID, other.ID), Label: max(t.Label, other.Label), Prefix: max(t.Prefix, other.Prefix), + Type: max(t.Type, other.Type), FixedRows: fixedRows, } } @@ -142,25 +221,13 @@ func (t TableTemplates) Tables(node Node) []Table { var result []Table for _, template := range t { rows, truncationCount := node.ExtractTable(template) - table := Table{ + result = append(result, Table{ ID: template.ID, Label: template.Label, - Rows: []MetadataRow{}, + Type: template.Type, + Rows: rows, TruncationCount: truncationCount, - } - keys := make([]string, 0, len(rows)) - for k := range rows { - keys = append(keys, k) - } - sort.Strings(keys) - for _, key := range keys { - table.Rows = append(table.Rows, MetadataRow{ - ID: "label_" + key, - Label: key, - Value: rows[key], - }) - } - result = append(result, table) + }) } sort.Sort(tablesByID(result)) return result diff --git a/report/table_test.go b/report/table_test.go index 72b6d94cd4..017762a7a5 100644 --- a/report/table_test.go +++ b/report/table_test.go @@ -16,7 +16,7 @@ func TestPrefixTables(t *testing.T) { } nmd := report.MakeNode("foo1") - nmd = nmd.AddPrefixTable("foo_", want) + nmd = nmd.AddPrefixLabels("foo_", want) have, truncationCount := nmd.ExtractTable(report.TableTemplate{Prefix: "foo_"}) if truncationCount != 0 { @@ -62,7 +62,7 @@ func TestTruncation(t *testing.T) { nmd := report.MakeNode("foo1") - nmd = nmd.AddPrefixTable("foo_", want) + nmd = nmd.AddPrefixLabels("foo_", want) _, truncationCount := nmd.ExtractTable(report.TableTemplate{Prefix: "foo_"}) if truncationCount != wantTruncationCount { From e475a09ee6e9a58beb15344198486cf8f7f228fb Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Fri, 23 Dec 2016 17:00:24 +0100 Subject: [PATCH 2/4] Rendering sortable generic tables in the UI Rendering generic table columns Made Type a required attribute for TableTemplate Made generic table sortable on the UI --- client/app/scripts/components/node-details.js | 36 ++++- .../node-details-generic-table.js | 129 ++++++++++++++++++ probe/docker/reporter.go | 26 +++- probe/kubernetes/reporter.go | 7 +- probe/overlay/weave.go | 16 ++- report/table.go | 24 +++- 6 files changed, 224 insertions(+), 14 deletions(-) create mode 100644 client/app/scripts/components/node-details/node-details-generic-table.js diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index b8abffd1b7..d376947297 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -1,3 +1,4 @@ +import debug from 'debug'; import React from 'react'; import { connect } from 'react-redux'; import { Map as makeMap } from 'immutable'; @@ -8,6 +9,7 @@ import { resetDocumentTitle, setDocumentTitle } from '../utils/title-utils'; import MatchedText from './matched-text'; import NodeDetailsControls from './node-details/node-details-controls'; +import NodeDetailsGenericTable from './node-details/node-details-generic-table'; import NodeDetailsHealth from './node-details/node-details-health'; import NodeDetailsInfo from './node-details/node-details-info'; import NodeDetailsLabels from './node-details/node-details-labels'; @@ -15,13 +17,18 @@ import NodeDetailsRelatives from './node-details/node-details-relatives'; import NodeDetailsTable from './node-details/node-details-table'; import Warning from './warning'; + +const logError = debug('scope:error'); + 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.`; } -class NodeDetails extends React.Component { +const TABLE_TYPE_PROPERTY_LIST = 'property-list'; +const TABLE_TYPE_GENERIC = 'multicolumn-table'; +class NodeDetails extends React.Component { constructor(props, context) { super(props, context); this.handleClickClose = this.handleClickClose.bind(this); @@ -214,9 +221,7 @@ class NodeDetails extends React.Component { }
- + {this.renderTable(table)} ); } @@ -227,6 +232,29 @@ class NodeDetails extends React.Component { ); } + renderTable(table) { + const { nodeMatches = makeMap() } = this.props; + switch (table.type) { + case TABLE_TYPE_GENERIC: + return ( + + ); + case TABLE_TYPE_PROPERTY_LIST: + return ( + + ); + default: + logError(`Undefined type '${table.type}' for table ${table.id}`); + return null; + } + } + componentDidUpdate() { this.updateTitle(); } diff --git a/client/app/scripts/components/node-details/node-details-generic-table.js b/client/app/scripts/components/node-details/node-details-generic-table.js new file mode 100644 index 0000000000..ad37ee44a7 --- /dev/null +++ b/client/app/scripts/components/node-details/node-details-generic-table.js @@ -0,0 +1,129 @@ +import React from 'react'; +import { Map as makeMap } from 'immutable'; +import { sortBy } from 'lodash'; + +import MatchedText from '../matched-text'; +import ShowMore from '../show-more'; + + +function columnStyle(column) { + return { + textAlign: column.dataType === 'number' ? 'right' : 'left', + paddingRight: '10px', + maxWidth: '140px' + }; +} + +function sortedRows(rows, sortedByColumn, sortedDesc) { + const orderedRows = sortBy(rows, row => row.id); + const sorted = sortBy(orderedRows, (row) => { + let value = row.entries[sortedByColumn.id]; + if (sortedByColumn.dataType === 'number') { + value = parseFloat(value); + } + return value; + }); + if (!sortedDesc) { + sorted.reverse(); + } + return sorted; +} + +export default class NodeDetailsGenericTable extends React.Component { + constructor(props, context) { + super(props, context); + this.DEFAULT_LIMIT = 5; + this.state = { + limit: this.DEFAULT_LIMIT, + sortedByColumn: props.columns[0], + sortedDesc: true + }; + this.handleLimitClick = this.handleLimitClick.bind(this); + } + + handleHeaderClick(ev, column) { + ev.preventDefault(); + this.setState({ + sortedByColumn: column, + sortedDesc: this.state.sortedByColumn.id === column.id + ? !this.state.sortedDesc : true + }); + } + + handleLimitClick() { + const limit = this.state.limit ? 0 : this.DEFAULT_LIMIT; + this.setState({limit}); + } + + render() { + const { sortedByColumn, sortedDesc } = this.state; + const { columns, matches = makeMap() } = this.props; + let rows = this.props.rows; + let notShown = 0; + const limited = rows && this.state.limit > 0 && rows.length > this.state.limit; + const expanded = this.state.limit === 0; + if (rows && limited) { + const hasNotShownMatch = rows.filter((row, index) => index >= this.state.limit + && matches.has(row.id)).length > 0; + if (!hasNotShownMatch) { + notShown = rows.length - this.DEFAULT_LIMIT; + rows = rows.slice(0, this.state.limit); + } + } + + return ( +
+ + + + {columns.map((column) => { + const onHeaderClick = (ev) => { + this.handleHeaderClick(ev, column); + }; + const isSorted = column.id === this.state.sortedByColumn.id; + const isSortedDesc = isSorted && this.state.sortedDesc; + const isSortedAsc = isSorted && !isSortedDesc; + const style = Object.assign(columnStyle(column), { + cursor: 'pointer', + fontSize: '11px' + }); + return ( + + ); + })} + + + + {sortedRows(rows, sortedByColumn, sortedDesc).map(row => ( + + {columns.map((column) => { + const value = row.entries[column.id]; + const match = matches.get(column.id); + return ( + + ); + })} + + ))} + +
+ {isSortedAsc + && } + {isSortedDesc + && } + {column.label} +
+ +
+ +
+ ); + } +} diff --git a/probe/docker/reporter.go b/probe/docker/reporter.go index accd183015..4aacaca2da 100644 --- a/probe/docker/reporter.go +++ b/probe/docker/reporter.go @@ -48,7 +48,10 @@ var ( } ContainerTableTemplates = report.TableTemplates{ - ImageTableID: {ID: ImageTableID, Label: "Image", + ImageTableID: { + ID: ImageTableID, + Label: "Image", + Type: report.PropertyListType, FixedRows: map[string]string{ ImageID: "ID", ImageName: "Name", @@ -56,12 +59,27 @@ var ( ImageVirtualSize: "Virtual Size", }, }, - LabelPrefix: {ID: LabelPrefix, Label: "Docker Labels", Prefix: LabelPrefix}, - EnvPrefix: {ID: EnvPrefix, Label: "Environment Variables", Prefix: EnvPrefix}, + LabelPrefix: { + ID: LabelPrefix, + Label: "Docker Labels", + Type: report.PropertyListType, + Prefix: LabelPrefix, + }, + EnvPrefix: { + ID: EnvPrefix, + Label: "Environment Variables", + Type: report.PropertyListType, + Prefix: EnvPrefix, + }, } ContainerImageTableTemplates = report.TableTemplates{ - ImageLabelPrefix: {ID: ImageLabelPrefix, Label: "Docker Labels", Prefix: ImageLabelPrefix}, + ImageLabelPrefix: { + ID: ImageLabelPrefix, + Label: "Docker Labels", + Type: report.PropertyListType, + Prefix: ImageLabelPrefix, + }, } ContainerControls = []report.Control{ diff --git a/probe/kubernetes/reporter.go b/probe/kubernetes/reporter.go index 44c7c564a3..dfebc1a399 100644 --- a/probe/kubernetes/reporter.go +++ b/probe/kubernetes/reporter.go @@ -68,7 +68,12 @@ var ( ReplicaSetMetricTemplates = PodMetricTemplates TableTemplates = report.TableTemplates{ - LabelPrefix: {ID: LabelPrefix, Label: "Kubernetes Labels", Prefix: LabelPrefix}, + LabelPrefix: { + ID: LabelPrefix, + Label: "Kubernetes Labels", + Type: report.PropertyListType, + Prefix: LabelPrefix, + }, } ScalingControls = []report.Control{ diff --git a/probe/overlay/weave.go b/probe/overlay/weave.go index 13e34412ce..2fb3aea18b 100644 --- a/probe/overlay/weave.go +++ b/probe/overlay/weave.go @@ -121,9 +121,23 @@ var ( }, WeaveConnectionsMulticolumnTablePrefix: { ID: WeaveConnectionsMulticolumnTablePrefix, - Label: "Connections", + Label: "Connections (new)", Type: report.MulticolumnTableType, Prefix: WeaveConnectionsMulticolumnTablePrefix, + Columns: []report.Column{ + report.Column{ + ID: "ip", + Label: "IP", + }, + report.Column{ + ID: "state", + Label: "State", + }, + report.Column{ + ID: "info", + Label: "Info", + }, + }, }, } ) diff --git a/report/table.go b/report/table.go index 791fe62eaf..e8e0f8297e 100644 --- a/report/table.go +++ b/report/table.go @@ -118,10 +118,9 @@ func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCou } type Column struct { - ID string `json:"id"` - Label string `json:"label"` - DataType string `json:"dataType"` - Alignment string `json:"alignment"` + ID string `json:"id"` + Label string `json:"label"` + DataType string `json:"dataType"` } type Row struct { @@ -129,6 +128,16 @@ type Row struct { Entries map[string]string `json:"entries"` } +// Copy returns a copy of the Row. +func (r Row) Copy() Row { + entriesCopy := make(map[string]string, len(r.Entries)) + for key, value := range r.Entries { + entriesCopy[key] = value + } + r.Entries = entriesCopy + return r +} + // Table is the type for a table in the UI. type Table struct { ID string `json:"id"` @@ -202,6 +211,11 @@ func (t TableTemplate) Merge(other TableTemplate) TableTemplate { fixedRows = other.FixedRows } + columns := t.Columns + if len(other.Columns) > len(columns) { + columns = other.Columns + } + // TODO: Refactor the merging logic, as mixing // the types now might result in invalid tables. return TableTemplate{ @@ -209,6 +223,7 @@ func (t TableTemplate) Merge(other TableTemplate) TableTemplate { Label: max(t.Label, other.Label), Prefix: max(t.Prefix, other.Prefix), Type: max(t.Type, other.Type), + Columns: columns, FixedRows: fixedRows, } } @@ -225,6 +240,7 @@ func (t TableTemplates) Tables(node Node) []Table { ID: template.ID, Label: template.Label, Type: template.Type, + Columns: template.Columns, Rows: rows, TruncationCount: truncationCount, }) From 6888108b83453e1e19d1069056c59497f7851002 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Tue, 3 Jan 2017 16:43:41 +0100 Subject: [PATCH 3/4] Made the searching of generic tables work on the UI Extracted table headers common code on the frontend Fixed the search matching and extracted further common code in the UI --- client/app/scripts/components/node-details.js | 42 +++-- .../node-details-generic-table.js | 110 +++++-------- .../node-details/node-details-labels.js | 10 +- .../node-details/node-details-relatives.js | 8 +- .../node-details-table-headers.js | 55 +++++++ .../node-details/node-details-table.js | 155 +++--------------- client/app/scripts/constants/limits.js | 2 + client/app/scripts/constants/styles.js | 44 +++++ .../app/scripts/utils/node-details-utils.js | 36 ++++ client/app/scripts/utils/search-utils.js | 33 ++-- client/app/styles/main.less | 11 ++ probe/overlay/weave.go | 20 ++- 12 files changed, 275 insertions(+), 251 deletions(-) create mode 100644 client/app/scripts/components/node-details/node-details-table-headers.js create mode 100644 client/app/scripts/constants/limits.js create mode 100644 client/app/scripts/utils/node-details-utils.js diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index d376947297..387565ea68 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -5,6 +5,7 @@ import { Map as makeMap } from 'immutable'; import { clickCloseDetails, clickShowTopologyForNode } from '../actions/app-actions'; import { brightenColor, getNeutralColor, getNodeColorDark } from '../utils/color-utils'; +import { isGenericTable, isPropertyList } from '../utils/node-details-utils'; import { resetDocumentTitle, setDocumentTitle } from '../utils/title-utils'; import MatchedText from './matched-text'; @@ -25,9 +26,6 @@ function getTruncationText(count) { + ` (${count} extra entries not included). We are working to remove this limitation.`; } -const TABLE_TYPE_PROPERTY_LIST = 'property-list'; -const TABLE_TYPE_GENERIC = 'multicolumn-table'; - class NodeDetails extends React.Component { constructor(props, context) { super(props, context); @@ -215,7 +213,7 @@ class NodeDetails extends React.Component { return (
- {table.label} + {table.label.length > 0 && table.label} {table.truncationCount > 0 && @@ -234,25 +232,25 @@ class NodeDetails extends React.Component { renderTable(table) { const { nodeMatches = makeMap() } = this.props; - switch (table.type) { - case TABLE_TYPE_GENERIC: - return ( - - ); - case TABLE_TYPE_PROPERTY_LIST: - return ( - - ); - default: - logError(`Undefined type '${table.type}' for table ${table.id}`); - return null; + + if (isGenericTable(table)) { + return ( + + ); + } else if (isPropertyList(table)) { + return ( + + ); } + + logError(`Undefined type '${table.type}' for table ${table.id}`); + return null; } componentDidUpdate() { diff --git a/client/app/scripts/components/node-details/node-details-generic-table.js b/client/app/scripts/components/node-details/node-details-generic-table.js index ad37ee44a7..3478078bea 100644 --- a/client/app/scripts/components/node-details/node-details-generic-table.js +++ b/client/app/scripts/components/node-details/node-details-generic-table.js @@ -2,28 +2,25 @@ import React from 'react'; import { Map as makeMap } from 'immutable'; import { sortBy } from 'lodash'; +import { NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT } from '../../constants/limits'; +import { + isNumber, getTableColumnsStyles, genericTableEntryKey +} from '../../utils/node-details-utils'; +import NodeDetailsTableHeaders from './node-details-table-headers'; import MatchedText from '../matched-text'; import ShowMore from '../show-more'; -function columnStyle(column) { - return { - textAlign: column.dataType === 'number' ? 'right' : 'left', - paddingRight: '10px', - maxWidth: '140px' - }; -} - -function sortedRows(rows, sortedByColumn, sortedDesc) { - const orderedRows = sortBy(rows, row => row.id); - const sorted = sortBy(orderedRows, (row) => { - let value = row.entries[sortedByColumn.id]; - if (sortedByColumn.dataType === 'number') { +function sortedRows(rows, columns, sortedBy, sortedDesc) { + const column = columns.find(c => c.id === sortedBy); + const sorted = sortBy(rows, (row) => { + let value = row.entries[sortedBy]; + if (isNumber(column)) { value = parseFloat(value); } return value; }); - if (!sortedDesc) { + if (sortedDesc) { sorted.reverse(); } return sorted; @@ -32,85 +29,68 @@ function sortedRows(rows, sortedByColumn, sortedDesc) { export default class NodeDetailsGenericTable extends React.Component { constructor(props, context) { super(props, context); - this.DEFAULT_LIMIT = 5; this.state = { - limit: this.DEFAULT_LIMIT, - sortedByColumn: props.columns[0], + limit: NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT, + sortedBy: props.columns[0].id, sortedDesc: true }; this.handleLimitClick = this.handleLimitClick.bind(this); + this.updateSorted = this.updateSorted.bind(this); } - handleHeaderClick(ev, column) { - ev.preventDefault(); - this.setState({ - sortedByColumn: column, - sortedDesc: this.state.sortedByColumn.id === column.id - ? !this.state.sortedDesc : true - }); + updateSorted(sortedBy, sortedDesc) { + this.setState({ sortedBy, sortedDesc }); } handleLimitClick() { - const limit = this.state.limit ? 0 : this.DEFAULT_LIMIT; - this.setState({limit}); + this.setState({ + limit: this.state.limit ? 0 : NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT + }); } render() { - const { sortedByColumn, sortedDesc } = this.state; + const { sortedBy, sortedDesc } = this.state; const { columns, matches = makeMap() } = this.props; - let rows = this.props.rows; - let notShown = 0; - const limited = rows && this.state.limit > 0 && rows.length > this.state.limit; const expanded = this.state.limit === 0; - if (rows && limited) { - const hasNotShownMatch = rows.filter((row, index) => index >= this.state.limit - && matches.has(row.id)).length > 0; - if (!hasNotShownMatch) { - notShown = rows.length - this.DEFAULT_LIMIT; + + // Stabilize the order of rows + let rows = sortBy(this.props.rows || [], row => row.id); + let notShown = 0; + + // If there are rows that would be hidden behind 'show more', keep them + // expanded if any of them match the search query; otherwise hide them. + if (this.state.limit > 0 && rows.length > this.state.limit) { + const hasHiddenMatch = rows.slice(this.state.limit).some(row => + columns.some(column => matches.has(genericTableEntryKey(row, column))) + ); + if (!hasHiddenMatch) { + notShown = rows.length - NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT; rows = rows.slice(0, this.state.limit); } } + const styles = getTableColumnsStyles(columns); return (
- - {columns.map((column) => { - const onHeaderClick = (ev) => { - this.handleHeaderClick(ev, column); - }; - const isSorted = column.id === this.state.sortedByColumn.id; - const isSortedDesc = isSorted && this.state.sortedDesc; - const isSortedAsc = isSorted && !isSortedDesc; - const style = Object.assign(columnStyle(column), { - cursor: 'pointer', - fontSize: '11px' - }); - return ( - - ); - })} - + - {sortedRows(rows, sortedByColumn, sortedDesc).map(row => ( + {sortedRows(rows, columns, sortedBy, sortedDesc).map(row => ( - {columns.map((column) => { + {columns.map((column, index) => { + const match = matches.get(genericTableEntryKey(row, column)); const value = row.entries[column.id]; - const match = matches.get(column.id); return ( ); diff --git a/client/app/scripts/components/node-details/node-details-labels.js b/client/app/scripts/components/node-details/node-details-labels.js index 2fd01a4829..e25ec4ccd8 100644 --- a/client/app/scripts/components/node-details/node-details-labels.js +++ b/client/app/scripts/components/node-details/node-details-labels.js @@ -2,8 +2,9 @@ import React from 'react'; import { Map as makeMap } from 'immutable'; import sortBy from 'lodash/sortBy'; -import MatchedText from '../matched-text'; +import { NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT } from '../../constants/limits'; import NodeDetailsControlButton from './node-details-control-button'; +import MatchedText from '../matched-text'; import ShowMore from '../show-more'; const Controls = controls => ( @@ -17,15 +18,14 @@ export default class NodeDetailsLabels extends React.Component { constructor(props, context) { super(props, context); - this.DEFAULT_LIMIT = 5; this.state = { - limit: this.DEFAULT_LIMIT, + limit: NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT, }; this.handleLimitClick = this.handleLimitClick.bind(this); } handleLimitClick() { - const limit = this.state.limit ? 0 : this.DEFAULT_LIMIT; + const limit = this.state.limit ? 0 : NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT; this.setState({limit}); } @@ -39,7 +39,7 @@ export default class NodeDetailsLabels extends React.Component { const hasNotShownMatch = rows.filter((row, index) => index >= this.state.limit && matches.has(row.id)).length > 0; if (!hasNotShownMatch) { - notShown = rows.length - this.DEFAULT_LIMIT; + notShown = rows.length - NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT; rows = rows.slice(0, this.state.limit); } } diff --git a/client/app/scripts/components/node-details/node-details-relatives.js b/client/app/scripts/components/node-details/node-details-relatives.js index 26cef8a78a..3004011fd5 100644 --- a/client/app/scripts/components/node-details/node-details-relatives.js +++ b/client/app/scripts/components/node-details/node-details-relatives.js @@ -1,22 +1,22 @@ import React from 'react'; import { Map as makeMap } from 'immutable'; +import { NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT } from '../../constants/limits'; import NodeDetailsRelativesLink from './node-details-relatives-link'; export default class NodeDetailsRelatives extends React.Component { constructor(props, context) { super(props, context); - this.DEFAULT_LIMIT = 5; this.state = { - limit: this.DEFAULT_LIMIT + limit: NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT }; this.handleLimitClick = this.handleLimitClick.bind(this); } handleLimitClick(ev) { ev.preventDefault(); - const limit = this.state.limit ? 0 : this.DEFAULT_LIMIT; + const limit = this.state.limit ? 0 : NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT; this.setState({limit}); } @@ -26,7 +26,7 @@ export default class NodeDetailsRelatives extends React.Component { const limited = this.state.limit > 0 && relatives.length > this.state.limit; const showLimitAction = limited || (this.state.limit === 0 - && relatives.length > this.DEFAULT_LIMIT); + && relatives.length > NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT); const limitActionText = limited ? 'Show more' : 'Show less'; if (limited) { relatives = relatives.slice(0, this.state.limit); diff --git a/client/app/scripts/components/node-details/node-details-table-headers.js b/client/app/scripts/components/node-details/node-details-table-headers.js new file mode 100644 index 0000000000..9d72a2ce4a --- /dev/null +++ b/client/app/scripts/components/node-details/node-details-table-headers.js @@ -0,0 +1,55 @@ +import React from 'react'; +import { defaultSortDesc, getTableColumnsStyles } from '../../utils/node-details-utils'; +import { NODE_DETAILS_TABLE_CW, NODE_DETAILS_TABLE_XS_LABEL } from '../../constants/styles'; + + +export default class NodeDetailsTableHeaders extends React.Component { + handleClick(ev, headerId, currentSortedBy, currentSortedDesc) { + ev.preventDefault(); + const header = this.props.headers.find(h => h.id === headerId); + const sortedBy = header.id; + const sortedDesc = sortedBy === currentSortedBy + ? !currentSortedDesc : defaultSortDesc(header); + this.props.onClick(sortedBy, sortedDesc); + } + + render() { + const { headers, sortedBy, sortedDesc } = this.props; + const colStyles = getTableColumnsStyles(headers); + return ( + + {headers.map((header, index) => { + const headerClasses = ['node-details-table-header', 'truncate']; + const onClick = (ev) => { + this.handleClick(ev, header.id, sortedBy, sortedDesc); + }; + // sort by first metric by default + const isSorted = header.id === sortedBy; + const isSortedDesc = isSorted && sortedDesc; + const isSortedAsc = isSorted && !isSortedDesc; + + if (isSorted) { + headerClasses.push('node-details-table-header-sorted'); + } + + const style = colStyles[index]; + const label = + (style.width === NODE_DETAILS_TABLE_CW.XS && NODE_DETAILS_TABLE_XS_LABEL[header.id]) ? + NODE_DETAILS_TABLE_XS_LABEL[header.id] : header.label; + + return ( + + ); + })} + + ); + } +} diff --git a/client/app/scripts/components/node-details/node-details-table.js b/client/app/scripts/components/node-details/node-details-table.js index a7c820fc10..2cd8f9ebab 100644 --- a/client/app/scripts/components/node-details/node-details-table.js +++ b/client/app/scripts/components/node-details/node-details-table.js @@ -2,59 +2,15 @@ import React from 'react'; import classNames from 'classnames'; import { find, get, union, sortBy, groupBy, concat } from 'lodash'; +import { NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT } from '../../constants/limits'; + import ShowMore from '../show-more'; import NodeDetailsTableRow from './node-details-table-row'; +import NodeDetailsTableHeaders from './node-details-table-headers'; import { ipToPaddedString } from '../../utils/string-utils'; - - -function isNumber(data) { - return data.dataType && data.dataType === 'number'; -} - - -function isIP(data) { - return data.dataType && data.dataType === 'ip'; -} - - -const CW = { - XS: '32px', - S: '50px', - M: '70px', - L: '120px', - XL: '140px', - XXL: '170px', -}; - - -const XS_LABEL = { - count: '#', - // TODO: consider changing the name of this field on the BE - container: '#', -}; - - -const COLUMN_WIDTHS = { - count: CW.XS, - container: CW.XS, - docker_container_created: CW.XXL, - docker_container_restart_count: CW.M, - docker_container_state_human: CW.XXL, - docker_container_uptime: '85px', - docker_cpu_total_usage: CW.M, - docker_memory_usage: CW.M, - open_files_count: CW.M, - pid: CW.S, - port: CW.S, - ppid: CW.S, - process_cpu_usage_percent: CW.M, - process_memory_usage_bytes: CW.M, - threads: CW.M, - - // e.g. details panel > pods - kubernetes_ip: CW.L, - kubernetes_state: CW.M, -}; +import { + isIP, isNumber, defaultSortDesc, getTableColumnsStyles +} from '../../utils/node-details-utils'; function getDefaultSortedBy(columns, nodes) { @@ -158,54 +114,27 @@ function getSortedNodes(nodes, sortedByHeader, sortedDesc) { } -function getColumnWidth(headers, h) { - // - // More beauty hacking, ports and counts can only get so big, free up WS for other longer - // fields like IPs! - // - return COLUMN_WIDTHS[h.id]; -} - - -function getColumnsStyles(headers) { - return headers.map((h, i) => ({ - width: getColumnWidth(headers, h, i), - textAlign: h.dataType === 'number' ? 'right' : 'left', - })); -} - - -function defaultSortDesc(header) { - return header && header.dataType === 'number'; -} - - export default class NodeDetailsTable extends React.Component { constructor(props, context) { super(props, context); - this.DEFAULT_LIMIT = 5; this.state = { - limit: props.limit || this.DEFAULT_LIMIT, + limit: props.limit || NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT, sortedDesc: this.props.sortedDesc, sortedBy: this.props.sortedBy }; this.handleLimitClick = this.handleLimitClick.bind(this); + this.updateSorted = this.updateSorted.bind(this); } - handleHeaderClick(ev, headerId, currentSortedBy, currentSortedDesc) { - ev.preventDefault(); - const header = this.getColumnHeaders().find(h => h.id === headerId); - const sortedBy = header.id; - const sortedDesc = header.id === currentSortedBy - ? !currentSortedDesc : defaultSortDesc(header); - this.setState({sortedBy, sortedDesc}); + updateSorted(sortedBy, sortedDesc) { + this.setState({ sortedBy, sortedDesc }); this.props.onSortChange(sortedBy, sortedDesc); } handleLimitClick() { - const limit = this.state.limit ? 0 : this.DEFAULT_LIMIT; - this.setState({limit}); + const limit = this.state.limit ? 0 : NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT; + this.setState({ limit }); } getColumnHeaders() { @@ -213,60 +142,13 @@ export default class NodeDetailsTable extends React.Component { return [{id: 'label', label: this.props.label}].concat(columns); } - renderHeaders(sortedBy, sortedDesc) { - if (!this.props.nodes || this.props.nodes.length === 0) { - return null; - } - - const headers = this.getColumnHeaders(); - const colStyles = getColumnsStyles(headers); - - return ( - - {headers.map((header, i) => { - const headerClasses = ['node-details-table-header', 'truncate']; - const onHeaderClick = (ev) => { - this.handleHeaderClick(ev, header.id, sortedBy, sortedDesc); - }; - // sort by first metric by default - const isSorted = header.id === sortedBy; - const isSortedDesc = isSorted && sortedDesc; - const isSortedAsc = isSorted && !isSortedDesc; - - if (isSorted) { - headerClasses.push('node-details-table-header-sorted'); - } - - const style = colStyles[i]; - const label = (style.width === CW.XS && XS_LABEL[header.id]) ? - XS_LABEL[header.id] : - header.label; - - return ( - - ); - })} - - ); - } - render() { const { nodeIdKey, columns, topologyId, onClickRow, onMouseEnter, onMouseLeave, onMouseEnterRow, onMouseLeaveRow } = this.props; const sortedBy = this.state.sortedBy || getDefaultSortedBy(columns, this.props.nodes); const sortedByHeader = this.getColumnHeaders().find(h => h.id === sortedBy); - const sortedDesc = this.state.sortedDesc !== null ? - this.state.sortedDesc : - defaultSortDesc(sortedByHeader); + const sortedDesc = this.state.sortedDesc || defaultSortDesc(sortedByHeader); let nodes = getSortedNodes(this.props.nodes, sortedByHeader, sortedDesc); const limited = nodes && this.state.limit > 0 && nodes.length > this.state.limit; @@ -277,13 +159,20 @@ export default class NodeDetailsTable extends React.Component { } const className = classNames('node-details-table-wrapper-wrapper', this.props.className); + const headers = this.getColumnHeaders(); + const styles = getTableColumnsStyles(headers); return (
- {isSortedAsc - && } - {isSortedDesc - && } - {column.label} -
+ className="node-details-generic-table-value truncate" + title={value} key={column.id} style={styles[index]}>
+ {isSortedAsc + && } + {isSortedDesc + && } + {label} +
- {isSortedAsc - && } - {isSortedDesc - && } - {label} -
- {this.renderHeaders(sortedBy, sortedDesc)} + {this.props.nodes && this.props.nodes.length > 0 && } pods + kubernetes_ip: NODE_DETAILS_TABLE_CW.XL, + kubernetes_state: NODE_DETAILS_TABLE_CW.M, + + // weave connections + weave_connection_connection: NODE_DETAILS_TABLE_CW.XXL, + weave_connection_state: NODE_DETAILS_TABLE_CW.L, + weave_connection_info: NODE_DETAILS_TABLE_CW.XL, +}; + +export const NODE_DETAILS_TABLE_XS_LABEL = { + count: '#', + // TODO: consider changing the name of this field on the BE + container: '#', +}; diff --git a/client/app/scripts/utils/node-details-utils.js b/client/app/scripts/utils/node-details-utils.js new file mode 100644 index 0000000000..7d9c3d2aa4 --- /dev/null +++ b/client/app/scripts/utils/node-details-utils.js @@ -0,0 +1,36 @@ +import { NODE_DETAILS_TABLE_COLUMN_WIDTHS } from '../constants/styles'; + +export function isGenericTable(table) { + return (table.type || (table.get && table.get('type'))) === 'multicolumn-table'; +} + +export function isPropertyList(table) { + return (table.type || (table.get && table.get('type'))) === 'property-list'; +} + +export function isNumber(data) { + return data.dataType && data.dataType === 'number'; +} + +export function isIP(data) { + return data.dataType && data.dataType === 'ip'; +} + +export function genericTableEntryKey(row, column) { + const columnId = column.id || column.get('id'); + const rowId = row.id || row.get('id'); + return `${rowId}_${columnId}`; +} + +export function defaultSortDesc(header) { + return header && isNumber(header); +} + +export function getTableColumnsStyles(headers) { + return headers.map(header => ({ + // More beauty hacking, ports and counts can only get + // so big, free up WS for other longer fields like IPs! + width: NODE_DETAILS_TABLE_COLUMN_WIDTHS[header.id], + textAlign: isNumber(header) ? 'right' : 'left' + })); +} diff --git a/client/app/scripts/utils/search-utils.js b/client/app/scripts/utils/search-utils.js index c297ae96a9..2ea844c230 100644 --- a/client/app/scripts/utils/search-utils.js +++ b/client/app/scripts/utils/search-utils.js @@ -1,6 +1,7 @@ import { Map as makeMap, Set as makeSet, List as makeList } from 'immutable'; import { escapeRegExp } from 'lodash'; +import { isGenericTable, isPropertyList, genericTableEntryKey } from './node-details-utils'; import { slugify } from './string-utils'; // topolevel search fields @@ -148,20 +149,26 @@ export function searchTopology(nodes, { prefix, query, metric, comp, value }) { }); } - // tables (envvars and labels) - const tables = node.get('tables'); - if (tables) { - tables.forEach((table) => { - if (table.get('rows')) { - table.get('rows').forEach((field) => { - const entries = field.get('entries'); - const keyPath = [nodeId, 'tables', field.get('id')]; - nodeMatches = findNodeMatch(nodeMatches, keyPath, entries.get('value'), - query, prefix, entries.get('label')); - }); - } + // property lists + (node.get('tables') || []).filter(isPropertyList).forEach((propertyList) => { + (propertyList.get('rows') || []).forEach((row) => { + const entries = row.get('entries'); + const keyPath = [nodeId, 'labels', row.get('id')]; + nodeMatches = findNodeMatch(nodeMatches, keyPath, entries.get('value'), + query, prefix, entries.get('label')); }); - } + }); + + // generic tables + (node.get('tables') || []).filter(isGenericTable).forEach((table) => { + (table.get('rows') || []).forEach((row) => { + table.get('columns').forEach((column) => { + const val = row.get('entries').get(column.get('id')); + const keyPath = [nodeId, 'tables', genericTableEntryKey(row, column)]; + nodeMatches = findNodeMatch(nodeMatches, keyPath, val, query); + }); + }); + }); } else if (metric) { const metrics = node.get('metrics'); if (metrics) { diff --git a/client/app/styles/main.less b/client/app/styles/main.less index e3e8d75cb9..801ad53db0 100644 --- a/client/app/styles/main.less +++ b/client/app/styles/main.less @@ -897,6 +897,17 @@ h2 { } } + &-generic-table { + width: 100%; + + tr { + display: flex; + th, td { + padding: 0 5px; + } + } + } + &-table { width: 100%; border-spacing: 0; diff --git a/probe/overlay/weave.go b/probe/overlay/weave.go index 2fb3aea18b..fde8ccf869 100644 --- a/probe/overlay/weave.go +++ b/probe/overlay/weave.go @@ -46,6 +46,9 @@ const ( WeavePluginTableID = "weave_plugin_table" WeavePluginStatus = "weave_plugin_status" WeavePluginDriver = "weave_plugin_driver" + WeaveConnectionsConnection = "weave_connection_connection" + WeaveConnectionsState = "weave_connection_state" + WeaveConnectionsInfo = "weave_connection_info" WeaveConnectionsTablePrefix = "weave_connections_table_" WeaveConnectionsMulticolumnTablePrefix = "weave_connections_multicolumn_table_" ) @@ -115,26 +118,25 @@ var ( }, WeaveConnectionsTablePrefix: { ID: WeaveConnectionsTablePrefix, - Label: "Connections", + Label: "Connections (old)", Type: report.PropertyListType, Prefix: WeaveConnectionsTablePrefix, }, WeaveConnectionsMulticolumnTablePrefix: { ID: WeaveConnectionsMulticolumnTablePrefix, - Label: "Connections (new)", Type: report.MulticolumnTableType, Prefix: WeaveConnectionsMulticolumnTablePrefix, Columns: []report.Column{ report.Column{ - ID: "ip", - Label: "IP", + ID: WeaveConnectionsConnection, + Label: "Connections", }, report.Column{ - ID: "state", + ID: WeaveConnectionsState, Label: "State", }, report.Column{ - ID: "info", + ID: WeaveConnectionsInfo, Label: "Info", }, }, @@ -488,9 +490,9 @@ func getConnectionsTable(router weave.Router) []report.Row { table = append(table, report.Row{ ID: conn.Address, Entries: map[string]string{ - "ip": fmt.Sprintf("%s %s", arrow, conn.Address), - "state": conn.State, - "info": conn.Info, + WeaveConnectionsConnection: fmt.Sprintf("%s %s", arrow, conn.Address), + WeaveConnectionsState: conn.State, + WeaveConnectionsInfo: conn.Info, }, }) } From d3466b54543b70d8c3500fc894604bc51fd43e49 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Wed, 4 Jan 2017 15:54:22 +0100 Subject: [PATCH 4/4] Refactored the table component/model and wrote the tests Backward-compatibility fix --- client/app/scripts/components/node-details.js | 12 +- .../node-details-generic-table.js | 12 +- ...abels.js => node-details-property-list.js} | 15 +- .../utils/__tests__/search-utils-test.js | 75 ++++- .../app/scripts/utils/node-details-utils.js | 4 +- client/app/scripts/utils/search-utils.js | 2 +- client/app/styles/main.less | 2 +- probe/docker/container.go | 4 +- probe/docker/reporter.go | 2 +- probe/kubernetes/meta.go | 2 +- probe/overlay/weave.go | 21 +- render/detailed/tables_test.go | 5 +- report/table.go | 205 +++++++++----- report/table_test.go | 268 ++++++++++++++++-- 14 files changed, 484 insertions(+), 145 deletions(-) rename client/app/scripts/components/node-details/{node-details-labels.js => node-details-property-list.js} (80%) diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index 387565ea68..ffd99e6a17 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -11,15 +11,15 @@ import { resetDocumentTitle, setDocumentTitle } from '../utils/title-utils'; import MatchedText from './matched-text'; import NodeDetailsControls from './node-details/node-details-controls'; import NodeDetailsGenericTable from './node-details/node-details-generic-table'; +import NodeDetailsPropertyList from './node-details/node-details-property-list'; 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 Warning from './warning'; -const logError = debug('scope:error'); +const log = debug('scope:node-details'); function getTruncationText(count) { return 'This section was too long to be handled efficiently and has been truncated' @@ -213,7 +213,7 @@ class NodeDetails extends React.Component { return (
- {table.label.length > 0 && table.label} + {table.label && table.label.length > 0 && table.label} {table.truncationCount > 0 && @@ -242,14 +242,14 @@ class NodeDetails extends React.Component { ); } else if (isPropertyList(table)) { return ( - ); } - logError(`Undefined type '${table.type}' for table ${table.id}`); + log(`Undefined type '${table.type}' for table ${table.id}`); return null; } diff --git a/client/app/scripts/components/node-details/node-details-generic-table.js b/client/app/scripts/components/node-details/node-details-generic-table.js index 3478078bea..54e295ad80 100644 --- a/client/app/scripts/components/node-details/node-details-generic-table.js +++ b/client/app/scripts/components/node-details/node-details-generic-table.js @@ -1,10 +1,13 @@ import React from 'react'; +import sortBy from 'lodash/sortBy'; import { Map as makeMap } from 'immutable'; -import { sortBy } from 'lodash'; import { NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT } from '../../constants/limits'; + import { - isNumber, getTableColumnsStyles, genericTableEntryKey + isNumber, + getTableColumnsStyles, + genericTableEntryKey } from '../../utils/node-details-utils'; import NodeDetailsTableHeaders from './node-details-table-headers'; import MatchedText from '../matched-text'; @@ -31,7 +34,7 @@ export default class NodeDetailsGenericTable extends React.Component { super(props, context); this.state = { limit: NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT, - sortedBy: props.columns[0].id, + sortedBy: props.columns && props.columns[0].id, sortedDesc: true }; this.handleLimitClick = this.handleLimitClick.bind(this); @@ -53,8 +56,7 @@ export default class NodeDetailsGenericTable extends React.Component { const { columns, matches = makeMap() } = this.props; const expanded = this.state.limit === 0; - // Stabilize the order of rows - let rows = sortBy(this.props.rows || [], row => row.id); + let rows = this.props.rows || []; let notShown = 0; // If there are rows that would be hidden behind 'show more', keep them diff --git a/client/app/scripts/components/node-details/node-details-labels.js b/client/app/scripts/components/node-details/node-details-property-list.js similarity index 80% rename from client/app/scripts/components/node-details/node-details-labels.js rename to client/app/scripts/components/node-details/node-details-property-list.js index e25ec4ccd8..4375248ca3 100644 --- a/client/app/scripts/components/node-details/node-details-labels.js +++ b/client/app/scripts/components/node-details/node-details-property-list.js @@ -8,14 +8,13 @@ import MatchedText from '../matched-text'; import ShowMore from '../show-more'; const Controls = controls => ( -
+
{sortBy(controls, 'rank').map(control => )}
); -export default class NodeDetailsLabels extends React.Component { - +export default class NodeDetailsPropertyList extends React.Component { constructor(props, context) { super(props, context); this.state = { @@ -45,16 +44,18 @@ export default class NodeDetailsLabels extends React.Component { } return ( -
+
{controls && Controls(controls)} {rows.map(field => ( -
+
{field.entries.label}
-
+
diff --git a/client/app/scripts/utils/__tests__/search-utils-test.js b/client/app/scripts/utils/__tests__/search-utils-test.js index 1596e3d626..dde59e6799 100644 --- a/client/app/scripts/utils/__tests__/search-utils-test.js +++ b/client/app/scripts/utils/__tests__/search-utils-test.js @@ -29,17 +29,50 @@ describe('SearchUtils', () => { }], tables: [{ id: 'metric1', + type: 'property-list', rows: [{ + id: 'label1', entries: { - id: 'row1', - label: 'Row 1', - value: 'Row Value 1' + label: 'Label 1', + value: 'Label Value 1' } }, { + id: 'label2', entries: { - id: 'row2', - label: 'Row 2', - value: 'Row Value 2' + label: 'Label 2', + value: 'Label Value 2' + } + }] + }, { + id: 'metric2', + type: 'multicolumn-table', + columns: [{ + id: 'a', + label: 'A' + }, { + id: 'c', + label: 'C' + }], + rows: [{ + id: 'row1', + entries: { + a: 'xxxa', + b: 'yyya', + c: 'zzz1' + } + }, { + id: 'row2', + entries: { + a: 'yyyb', + b: 'xxxb', + c: 'zzz2' + } + }, { + id: 'row3', + entries: { + a: 'Value 1', + b: 'Value 2', + c: 'Value 3' } }] }], @@ -80,7 +113,7 @@ describe('SearchUtils', () => { it('should filter nodes that do not match a pinned searches', () => { let nextState = fromJS({ nodes: nodeSets.someNodes, - pinnedSearches: ['row'] + pinnedSearches: ['Label Value 1'] }); nextState = fun(nextState); expect(nextState.get('nodes').filter(node => node.get('filtered')).size).toEqual(1); @@ -244,11 +277,31 @@ describe('SearchUtils', () => { expect(matches.getIn(['n1', 'metrics', 'metric1']).metric).toBeTruthy(); }); - it('should match on a tables field', () => { + it('should match on a property list value', () => { const nodes = nodeSets.someNodes; - const matches = fun(nodes, {query: 'Row Value 1'}); - expect(matches.size).toEqual(1); - expect(matches.getIn(['n2', 'tables', 'row1']).text).toBe('Row Value 1'); + const matches = fun(nodes, {query: 'Value 1'}); + expect(matches.size).toEqual(2); + expect(matches.getIn(['n2', 'property-lists']).size).toEqual(1); + expect(matches.getIn(['n2', 'property-lists', 'label1']).text).toBe('Label Value 1'); + }); + + it('should match on a generic table values', () => { + const nodes = nodeSets.someNodes; + const matches1 = fun(nodes, {query: 'xx'}).getIn(['n2', 'tables']); + const matches2 = fun(nodes, {query: 'yy'}).getIn(['n2', 'tables']); + const matches3 = fun(nodes, {query: 'zz'}).getIn(['n2', 'tables']); + const matches4 = fun(nodes, {query: 'a'}).getIn(['n2', 'tables']); + expect(matches1.size).toEqual(1); + expect(matches2.size).toEqual(1); + expect(matches3.size).toEqual(2); + expect(matches4.size).toEqual(3); + expect(matches1.get('row1_a').text).toBe('xxxa'); + expect(matches2.get('row2_a').text).toBe('yyyb'); + expect(matches3.get('row1_c').text).toBe('zzz1'); + expect(matches3.get('row2_c').text).toBe('zzz2'); + expect(matches4.get('row1_a').text).toBe('xxxa'); + expect(matches4.get('row3_a').text).toBe('Value 1'); + expect(matches4.get('row3_c').text).toBe('Value 3'); }); }); diff --git a/client/app/scripts/utils/node-details-utils.js b/client/app/scripts/utils/node-details-utils.js index 7d9c3d2aa4..a7c84bde08 100644 --- a/client/app/scripts/utils/node-details-utils.js +++ b/client/app/scripts/utils/node-details-utils.js @@ -9,11 +9,11 @@ export function isPropertyList(table) { } export function isNumber(data) { - return data.dataType && data.dataType === 'number'; + return data && data.dataType && data.dataType === 'number'; } export function isIP(data) { - return data.dataType && data.dataType === 'ip'; + return data && data.dataType && data.dataType === 'ip'; } export function genericTableEntryKey(row, column) { diff --git a/client/app/scripts/utils/search-utils.js b/client/app/scripts/utils/search-utils.js index 2ea844c230..aec25f632f 100644 --- a/client/app/scripts/utils/search-utils.js +++ b/client/app/scripts/utils/search-utils.js @@ -153,7 +153,7 @@ export function searchTopology(nodes, { prefix, query, metric, comp, value }) { (node.get('tables') || []).filter(isPropertyList).forEach((propertyList) => { (propertyList.get('rows') || []).forEach((row) => { const entries = row.get('entries'); - const keyPath = [nodeId, 'labels', row.get('id')]; + const keyPath = [nodeId, 'property-lists', row.get('id')]; nodeMatches = findNodeMatch(nodeMatches, keyPath, entries.get('value'), query, prefix, entries.get('label')); }); diff --git a/client/app/styles/main.less b/client/app/styles/main.less index 801ad53db0..b209661cd1 100644 --- a/client/app/styles/main.less +++ b/client/app/styles/main.less @@ -864,7 +864,7 @@ h2 { } } - &-labels { + &-property-list { &-controls { margin-left: -4px; } diff --git a/probe/docker/container.go b/probe/docker/container.go index f23ed71fd3..419fb86059 100644 --- a/probe/docker/container.go +++ b/probe/docker/container.go @@ -379,8 +379,8 @@ func (c *container) getBaseNode() report.Node { }).WithParents(report.EmptySets. Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(c.Image()))), ) - result = result.AddPrefixLabels(LabelPrefix, c.container.Config.Labels) - result = result.AddPrefixLabels(EnvPrefix, c.env()) + result = result.AddPrefixPropertyList(LabelPrefix, c.container.Config.Labels) + result = result.AddPrefixPropertyList(EnvPrefix, c.env()) return result } diff --git a/probe/docker/reporter.go b/probe/docker/reporter.go index 4aacaca2da..44efb69e41 100644 --- a/probe/docker/reporter.go +++ b/probe/docker/reporter.go @@ -276,7 +276,7 @@ func (r *Reporter) containerImageTopology() report.Topology { } nodeID := report.MakeContainerImageNodeID(imageID) node := report.MakeNodeWith(nodeID, latests) - node = node.AddPrefixLabels(ImageLabelPrefix, image.Labels) + node = node.AddPrefixPropertyList(ImageLabelPrefix, image.Labels) result.AddNode(node) }) diff --git a/probe/kubernetes/meta.go b/probe/kubernetes/meta.go index 20fb213bb5..fc8188ca3d 100644 --- a/probe/kubernetes/meta.go +++ b/probe/kubernetes/meta.go @@ -56,5 +56,5 @@ func (m meta) MetaNode(id string) report.Node { Name: m.Name(), Namespace: m.Namespace(), Created: m.Created(), - }).AddPrefixLabels(LabelPrefix, m.Labels()) + }).AddPrefixPropertyList(LabelPrefix, m.Labels()) } diff --git a/probe/overlay/weave.go b/probe/overlay/weave.go index fde8ccf869..b87cb88eb6 100644 --- a/probe/overlay/weave.go +++ b/probe/overlay/weave.go @@ -116,31 +116,32 @@ var ( WeavePluginDriver: "Driver Name", }, }, - WeaveConnectionsTablePrefix: { - ID: WeaveConnectionsTablePrefix, - Label: "Connections (old)", - Type: report.PropertyListType, - Prefix: WeaveConnectionsTablePrefix, - }, WeaveConnectionsMulticolumnTablePrefix: { ID: WeaveConnectionsMulticolumnTablePrefix, Type: report.MulticolumnTableType, Prefix: WeaveConnectionsMulticolumnTablePrefix, Columns: []report.Column{ - report.Column{ + { ID: WeaveConnectionsConnection, Label: "Connections", }, - report.Column{ + { ID: WeaveConnectionsState, Label: "State", }, - report.Column{ + { ID: WeaveConnectionsInfo, Label: "Info", }, }, }, + // Kept for backward-compatibility. + WeaveConnectionsTablePrefix: { + ID: WeaveConnectionsTablePrefix, + Label: "Connections", + Type: report.PropertyListType, + Prefix: WeaveConnectionsTablePrefix, + }, } ) @@ -470,7 +471,7 @@ func (w *Weave) addCurrentPeerInfo(latests map[string]string, node report.Node) latests[WeavePluginStatus] = "running" latests[WeavePluginDriver] = "weave" } - node = node.AddPrefixTable(WeaveConnectionsMulticolumnTablePrefix, getConnectionsTable(w.statusCache.Router)) + node = node.AddPrefixMulticolumnTable(WeaveConnectionsMulticolumnTablePrefix, getConnectionsTable(w.statusCache.Router)) node = node.WithParents(report.EmptySets.Add(report.Host, report.MakeStringSet(w.hostID))) return latests, node diff --git a/render/detailed/tables_test.go b/render/detailed/tables_test.go index 634755c545..ab847d2761 100644 --- a/render/detailed/tables_test.go +++ b/render/detailed/tables_test.go @@ -34,16 +34,18 @@ func TestNodeTables(t *testing.T) { want: []report.Table{ { ID: docker.EnvPrefix, + Type: report.PropertyListType, Label: "Environment Variables", Rows: []report.Row{}, }, { ID: docker.LabelPrefix, + Type: report.PropertyListType, Label: "Docker Labels", Rows: []report.Row{ { + ID: "label_label1", Entries: map[string]string{ - "id": "label_label1", "label": "label1", "value": "label1value", }, @@ -52,6 +54,7 @@ func TestNodeTables(t *testing.T) { }, { ID: docker.ImageTableID, + Type: report.PropertyListType, Label: "Image", Rows: []report.Row{}, }, diff --git a/report/table.go b/report/table.go index e8e0f8297e..accdd6553e 100644 --- a/report/table.go +++ b/report/table.go @@ -13,98 +13,135 @@ import ( // MaxTableRows sets the limit on the table size to render // TODO: this won't be needed once we send reports incrementally const ( - MaxTableRows = 20 - TruncationCountPrefix = "table_truncation_count_" - MulticolumnTableType = "multicolumn-table" - PropertyListType = "property-list" + MaxTableRows = 20 + TableEntryKeySeparator = "___" + TruncationCountPrefix = "table_truncation_count_" + MulticolumnTableType = "multicolumn-table" + PropertyListType = "property-list" ) -// AddPrefixTable appends arbitrary rows to the Node, returning a new node. -func (node Node) AddPrefixTable(prefix string, rows []Row) Node { - count := 0 +// WithTableTruncationInformation appends table truncation info to the node, returning the new node. +func (node Node) WithTableTruncationInformation(prefix string, totalRowsCount int) Node { + if totalRowsCount > MaxTableRows { + truncationCount := fmt.Sprintf("%d", totalRowsCount-MaxTableRows) + node = node.WithLatest(TruncationCountPrefix+prefix, mtime.Now(), truncationCount) + } + return node +} + +// AddPrefixMulticolumnTable appends arbitrary rows to the Node, returning a new node. +func (node Node) AddPrefixMulticolumnTable(prefix string, rows []Row) Node { + addedRowsCount := 0 for _, row := range rows { - if count >= MaxTableRows { + if addedRowsCount >= MaxTableRows { break } - // TODO: Figure a more natural way of storing rows - for column, value := range row.Entries { - key := fmt.Sprintf("%s %s", row.ID, column) + // Add all the row values as separate entries + for columnID, value := range row.Entries { + key := strings.Join([]string{row.ID, columnID}, TableEntryKeySeparator) node = node.WithLatest(prefix+key, mtime.Now(), value) } - count++ + addedRowsCount++ } - if len(rows) > MaxTableRows { - truncationCount := fmt.Sprintf("%d", len(rows)-MaxTableRows) - node = node.WithLatest(TruncationCountPrefix+prefix, mtime.Now(), truncationCount) - } - return node + return node.WithTableTruncationInformation(prefix, len(rows)) } -// AddPrefixLabels appends arbitrary key-value pairs to the Node, returning a new node. -func (node Node) AddPrefixLabels(prefix string, labels map[string]string) Node { - count := 0 - for key, value := range labels { - if count >= MaxTableRows { +// AddPrefixPropertyList appends arbitrary key-value pairs to the Node, returning a new node. +func (node Node) AddPrefixPropertyList(prefix string, propertyList map[string]string) Node { + addedPropertiesCount := 0 + for label, value := range propertyList { + if addedPropertiesCount >= MaxTableRows { break } - node = node.WithLatest(prefix+key, mtime.Now(), value) - count++ + node = node.WithLatest(prefix+label, mtime.Now(), value) + addedPropertiesCount++ } - if len(labels) > MaxTableRows { - truncationCount := fmt.Sprintf("%d", len(labels)-MaxTableRows) - node = node.WithLatest(TruncationCountPrefix+prefix, mtime.Now(), truncationCount) - } - return node + return node.WithTableTruncationInformation(prefix, len(propertyList)) } -// ExtractTable returns the rows to build a table from this node -func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCount int) { - rows = []Row{} - switch template.Type { - case MulticolumnTableType: - keyRows := map[string]Row{} - node.Latest.ForEach(func(key string, _ time.Time, value string) { - if len(template.Prefix) > 0 && strings.HasPrefix(key, template.Prefix) { - rowID, column := "", "" - fmt.Sscanf(key[len(template.Prefix):], "%s %s", &rowID, &column) - if _, ok := keyRows[rowID]; !ok { - keyRows[rowID] = Row{ - ID: rowID, - Entries: map[string]string{}, - } +// WithoutPrefix returns the string with trimmed prefix and a +// boolean information of whether that prefix was really there. +// NOTE: Consider moving this function to utilities. +func WithoutPrefix(s string, prefix string) (string, bool) { + return strings.TrimPrefix(s, prefix), len(prefix) > 0 && strings.HasPrefix(s, prefix) +} + +// ExtractMulticolumnTable returns the rows to build a multicolumn table from this node +func (node Node) ExtractMulticolumnTable(template TableTemplate) (rows []Row) { + rowsMapByID := map[string]Row{} + + // Itearate through the whole of our map to extract all the values with the key + // with the given prefix. Since multicolumn tables don't support fixed rows (yet), + // all the table values will be stored under the table prefix. + // NOTE: It would be nice to optimize this part by only iterating through the keys + // with the given prefix. If it is possible to traverse the keys in the Latest map + // in a sorted order, then having LowerBoundEntry(key) and UpperBoundEntry(key) + // methods should be enough to implement ForEachWithPrefix(prefix) straightforwardly. + node.Latest.ForEach(func(key string, _ time.Time, value string) { + if keyWithoutPrefix, ok := WithoutPrefix(key, template.Prefix); ok { + ids := strings.Split(keyWithoutPrefix, TableEntryKeySeparator) + rowID, columnID := ids[0], ids[1] + // If the row with the given ID doesn't yet exist, we create an empty one. + if _, ok := rowsMapByID[rowID]; !ok { + rowsMapByID[rowID] = Row{ + ID: rowID, + Entries: map[string]string{}, } - keyRows[rowID].Entries[column] = value } - }) - for _, row := range keyRows { - rows = append(rows, row) + // At this point, the row with that ID always exists, so we just update the value. + rowsMapByID[rowID].Entries[columnID] = value } - // By default assume it's a property list (for backward compatibility) - default: - keyValues := map[string]string{} - node.Latest.ForEach(func(key string, _ time.Time, value string) { - if label, ok := template.FixedRows[key]; ok { - keyValues[label] = value - } - if len(template.Prefix) > 0 && strings.HasPrefix(key, template.Prefix) { - label := key[len(template.Prefix):] - keyValues[label] = value - } - }) - labels := make([]string, 0, len(rows)) - for label := range keyValues { - labels = append(labels, label) - } - sort.Strings(labels) - for _, label := range labels { - rows = append(rows, Row{ - ID: "label_" + label, - Entries: map[string]string{ - "label": label, - "value": keyValues[label], - }, - }) + }) + + // Gather a list of rows. + rows = make([]Row, 0, len(rowsMapByID)) + for _, row := range rowsMapByID { + rows = append(rows, row) + } + + // Return the rows sorted by ID. + sort.Sort(rowsByID(rows)) + return rows +} + +// ExtractPropertyList returns the rows to build a property list from this node +func (node Node) ExtractPropertyList(template TableTemplate) (rows []Row) { + valuesMapByLabel := map[string]string{} + + // Itearate through the whole of our map to extract all the values with the key + // with the given prefix as well as the keys corresponding to the fixed table rows. + node.Latest.ForEach(func(key string, _ time.Time, value string) { + if label, ok := template.FixedRows[key]; ok { + valuesMapByLabel[label] = value + } else if label, ok := WithoutPrefix(key, template.Prefix); ok { + valuesMapByLabel[label] = value } + }) + + // Gather a label-value formatted list of rows. + rows = make([]Row, 0, len(valuesMapByLabel)) + for label, value := range valuesMapByLabel { + rows = append(rows, Row{ + ID: "label_" + label, + Entries: map[string]string{ + "label": label, + "value": value, + }, + }) + } + + // Return the rows sorted by ID. + sort.Sort(rowsByID(rows)) + return rows +} + +// ExtractTable returns the rows to build either a property list or a generic table from this node +func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCount int) { + switch template.Type { + case MulticolumnTableType: + rows = node.ExtractMulticolumnTable(template) + default: // By default assume it's a property list (for backward compatibility). + rows = node.ExtractPropertyList(template) } truncationCount = 0 @@ -117,17 +154,25 @@ func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCou return rows, truncationCount } +// Column is the type for multi-column tables in the UI. type Column struct { ID string `json:"id"` Label string `json:"label"` DataType string `json:"dataType"` } +// Row is the type that holds the table data for the UI. Entries map from column ID to cell value. type Row struct { ID string `json:"id"` Entries map[string]string `json:"entries"` } +type rowsByID []Row + +func (t rowsByID) Len() int { return len(t) } +func (t rowsByID) Swap(i, j int) { t[i], t[j] = t[j], t[i] } +func (t rowsByID) Less(i, j int) bool { return t[i].ID < t[j].ID } + // Copy returns a copy of the Row. func (r Row) Copy() Row { entriesCopy := make(map[string]string, len(r.Entries)) @@ -206,18 +251,18 @@ func (t TableTemplate) Merge(other TableTemplate) TableTemplate { return s2 } + // NOTE: Consider actually merging the columns and fixed rows. fixedRows := t.FixedRows if len(other.FixedRows) > len(fixedRows) { fixedRows = other.FixedRows } - columns := t.Columns if len(other.Columns) > len(columns) { columns = other.Columns } - // TODO: Refactor the merging logic, as mixing - // the types now might result in invalid tables. + // TODO: Refactor the merging logic, as mixing the types now might result in + // invalid tables. Maybe we should return an error if the types are different? return TableTemplate{ ID: max(t.ID, other.ID), Label: max(t.Label, other.Label), @@ -236,11 +281,17 @@ func (t TableTemplates) Tables(node Node) []Table { var result []Table for _, template := range t { rows, truncationCount := node.ExtractTable(template) + // Extract the type from the template; default to + // property list for backwards-compatibility. + tableType := template.Type + if tableType == "" { + tableType = PropertyListType + } result = append(result, Table{ ID: template.ID, Label: template.Label, - Type: template.Type, Columns: template.Columns, + Type: tableType, Rows: rows, TruncationCount: truncationCount, }) diff --git a/report/table_test.go b/report/table_test.go index 017762a7a5..fa94d5d8c5 100644 --- a/report/table_test.go +++ b/report/table_test.go @@ -9,15 +9,34 @@ import ( "github.com/weaveworks/scope/report" ) -func TestPrefixTables(t *testing.T) { - want := map[string]string{ - "foo1": "bar1", - "foo2": "bar2", +func TestMulticolumnTables(t *testing.T) { + want := []report.Row{ + { + ID: "row1", + Entries: map[string]string{ + "col1": "r1c1", + "col2": "r1c2", + "col3": "r1c3", + }, + }, + { + ID: "row2", + Entries: map[string]string{ + "col1": "r2c1", + "col3": "r2c3", + }, + }, } + nmd := report.MakeNode("foo1") + nmd = nmd.AddPrefixMulticolumnTable("foo_", want) + + template := report.TableTemplate{ + Type: report.MulticolumnTableType, + Prefix: "foo_", + } - nmd = nmd.AddPrefixLabels("foo_", want) - have, truncationCount := nmd.ExtractTable(report.TableTemplate{Prefix: "foo_"}) + have, truncationCount := nmd.ExtractTable(template) if truncationCount != 0 { t.Error("Table shouldn't had been truncated") @@ -28,49 +47,258 @@ func TestPrefixTables(t *testing.T) { } } -func TestFixedTables(t *testing.T) { - want := map[string]string{ +func TestPrefixPropertyLists(t *testing.T) { + want := []report.Row{ + { + ID: "label_foo1", + Entries: map[string]string{ + "label": "foo1", + "value": "bar1", + }, + }, + { + ID: "label_foo3", + Entries: map[string]string{ + "label": "foo3", + "value": "bar3", + }, + }, + } + + nmd := report.MakeNode("foo1") + nmd = nmd.AddPrefixPropertyList("foo_", map[string]string{ + "foo3": "bar3", "foo1": "bar1", + }) + nmd = nmd.AddPrefixPropertyList("zzz_", map[string]string{ "foo2": "bar2", + }) + + template := report.TableTemplate{ + Type: report.PropertyListType, + Prefix: "foo_", + } + + have, truncationCount := nmd.ExtractTable(template) + + if truncationCount != 0 { + t.Error("Table shouldn't had been truncated") + } + + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } +} + +func TestFixedPropertyLists(t *testing.T) { + want := []report.Row{ + { + ID: "label_foo1", + Entries: map[string]string{ + "label": "foo1", + "value": "bar1", + }, + }, + { + ID: "label_foo2", + Entries: map[string]string{ + "label": "foo2", + "value": "bar2", + }, + }, } + nmd := report.MakeNodeWith("foo1", map[string]string{ - "foo1key": "bar1", "foo2key": "bar2", + "foo1key": "bar1", }) - template := report.TableTemplate{FixedRows: map[string]string{ - "foo1key": "foo1", - "foo2key": "foo2", - }, + template := report.TableTemplate{ + Type: report.PropertyListType, + FixedRows: map[string]string{ + "foo2key": "foo2", + "foo1key": "foo1", + }, } - have, _ := nmd.ExtractTable(template) + have, truncationCount := nmd.ExtractTable(template) + + if truncationCount != 0 { + t.Error("Table shouldn't had been truncated") + } if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } } -func TestTruncation(t *testing.T) { +func TestPropertyListTruncation(t *testing.T) { wantTruncationCount := 1 - want := map[string]string{} + propertyList := 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 + propertyList[key] = value } nmd := report.MakeNode("foo1") + nmd = nmd.AddPrefixPropertyList("foo_", propertyList) + + template := report.TableTemplate{ + Type: report.PropertyListType, + Prefix: "foo_", + } - nmd = nmd.AddPrefixLabels("foo_", want) - _, truncationCount := nmd.ExtractTable(report.TableTemplate{Prefix: "foo_"}) + _, truncationCount := nmd.ExtractTable(template) if truncationCount != wantTruncationCount { t.Error( - "Table should had been truncated by", + "Property list should had been truncated by", wantTruncationCount, "and not", truncationCount, ) } } + +func TestMulticolumnTableTruncation(t *testing.T) { + wantTruncationCount := 1 + rows := []report.Row{} + for i := 0; i < report.MaxTableRows+wantTruncationCount; i++ { + rowID := fmt.Sprintf("row%d", i) + colID := fmt.Sprintf("col%d", i) + value := fmt.Sprintf("value%d", i) + rows = append(rows, report.Row{ + ID: rowID, + Entries: map[string]string{ + colID: value, + }, + }) + } + + nmd := report.MakeNode("foo1") + nmd = nmd.AddPrefixMulticolumnTable("foo_", rows) + + template := report.TableTemplate{ + Type: report.MulticolumnTableType, + Prefix: "foo_", + } + + _, truncationCount := nmd.ExtractTable(template) + + if truncationCount != wantTruncationCount { + t.Error( + "Property list should had been truncated by", + wantTruncationCount, + "and not", + truncationCount, + ) + } +} + +func TestTables(t *testing.T) { + want := []report.Table{ + { + ID: "AAA", + Label: "Aaa", + Type: report.PropertyListType, + Columns: nil, + Rows: []report.Row{ + { + ID: "label_foo1", + Entries: map[string]string{ + "label": "foo1", + "value": "bar1", + }, + }, + { + ID: "label_foo3", + Entries: map[string]string{ + "label": "foo3", + "value": "bar3", + }, + }, + }, + }, + { + ID: "BBB", + Label: "Bbb", + Type: report.MulticolumnTableType, + Columns: []report.Column{{ID: "col1", Label: "Column 1"}}, + Rows: []report.Row{ + { + ID: "row1", + Entries: map[string]string{ + "col1": "r1c1", + }, + }, + { + ID: "row2", + Entries: map[string]string{ + "col3": "r2c3", + }, + }, + }, + }, + { + ID: "CCC", + Label: "Ccc", + Type: report.PropertyListType, + Columns: nil, + Rows: []report.Row{ + { + ID: "label_foo3", + Entries: map[string]string{ + "label": "foo3", + "value": "bar3", + }, + }, + }, + }, + } + + nmd := report.MakeNodeWith("foo1", map[string]string{ + "foo3key": "bar3", + "foo1key": "bar1", + }) + nmd = nmd.AddPrefixMulticolumnTable("bbb_", []report.Row{ + {ID: "row1", Entries: map[string]string{"col1": "r1c1"}}, + {ID: "row2", Entries: map[string]string{"col3": "r2c3"}}, + }) + nmd = nmd.AddPrefixPropertyList("aaa_", map[string]string{ + "foo3": "bar3", + "foo1": "bar1", + }) + + aaaTemplate := report.TableTemplate{ + ID: "AAA", + Label: "Aaa", + Prefix: "aaa_", + Type: report.PropertyListType, + } + bbbTemplate := report.TableTemplate{ + ID: "BBB", + Label: "Bbb", + Prefix: "bbb_", + Type: report.MulticolumnTableType, + Columns: []report.Column{{ID: "col1", Label: "Column 1"}}, + } + cccTemplate := report.TableTemplate{ + ID: "CCC", + Label: "Ccc", + Prefix: "ccc_", + Type: report.PropertyListType, + FixedRows: map[string]string{"foo3key": "foo3"}, + } + templates := report.TableTemplates{ + aaaTemplate.ID: aaaTemplate, + bbbTemplate.ID: bbbTemplate, + cccTemplate.ID: cccTemplate, + } + + have := templates.Tables(nmd) + + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } +}