From 24672ed046bbde3d9e41ca872f793fbcd5353d65 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 12 Apr 2018 17:03:27 +0100 Subject: [PATCH 1/2] do not truncate tables Limiting env vars, docker&k8s labels, and weave net connection entries to 20 is problematic because - the truncation is arbitrary - there is a good chance that if you care about a specific entry it won't be there - the truncation is not consistent - different entries get truncated at different times - some of the rendering logic depends on specific labels, for example namespace filtering of containers depends on the `io.kubernetes.pod.namespace` label. In practice, there should never be a huge number of labels, or Weave Net connection entries. So there is no need to truncate them. That leaves env vars. These are of limited use, so we now omit them by default. If they are included they are included in full, so they are actually useful. Fixes #3127 --- prog/main.go | 2 +- report/table.go | 33 ++++++---------------- report/table_test.go | 65 -------------------------------------------- 3 files changed, 9 insertions(+), 91 deletions(-) diff --git a/prog/main.go b/prog/main.go index 4bf027628..f8099bc60 100644 --- a/prog/main.go +++ b/prog/main.go @@ -281,7 +281,7 @@ func setupFlags(flags *flags) { flag.StringVar(&flags.probe.pluginsRoot, "probe.plugins.root", "/var/run/scope/plugins", "Root directory to search for plugins") flag.BoolVar(&flags.probe.noControls, "probe.no-controls", false, "Disable controls (e.g. start/stop containers, terminals, logs ...)") flag.BoolVar(&flags.probe.noCommandLineArguments, "probe.omit.cmd-args", false, "Disable collection of command-line arguments") - flag.BoolVar(&flags.probe.noEnvironmentVariables, "probe.omit.env-vars", false, "Disable collection of environment variables") + flag.BoolVar(&flags.probe.noEnvironmentVariables, "probe.omit.env-vars", true, "Disable collection of environment variables") flag.BoolVar(&flags.probe.insecure, "probe.insecure", false, "(SSL) explicitly allow \"insecure\" SSL connections and transfers") flag.StringVar(&flags.probe.resolver, "probe.resolver", "", "IP address & port of resolver to use. Default is to use system resolver.") diff --git a/report/table.go b/report/table.go index e661e0f32..a36f8e217 100644 --- a/report/table.go +++ b/report/table.go @@ -10,55 +10,33 @@ import ( "github.com/weaveworks/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 TableEntryKeySeparator = "___" TruncationCountPrefix = "table_truncation_count_" MulticolumnTableType = "multicolumn-table" PropertyListType = "property-list" ) -// withTableTruncationInformation appends table truncation info to the node, returning the new node. -func (node Node) withTableTruncationInformation(prefix string, totalRowsCount int, now time.Time) Node { - if totalRowsCount > MaxTableRows { - truncationCount := fmt.Sprintf("%d", totalRowsCount-MaxTableRows) - node = node.WithLatest(TruncationCountPrefix+prefix, now, truncationCount) - } - return node -} - // AddPrefixMulticolumnTable appends arbitrary rows to the Node, returning a new node. func (node Node) AddPrefixMulticolumnTable(prefix string, rows []Row) Node { now := mtime.Now() - addedRowsCount := 0 for _, row := range rows { - if addedRowsCount >= MaxTableRows { - break - } // 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, now, value) } - addedRowsCount++ } - return node.withTableTruncationInformation(prefix, len(rows), now) + return node } // AddPrefixPropertyList appends arbitrary key-value pairs to the Node, returning a new node. func (node Node) AddPrefixPropertyList(prefix string, propertyList map[string]string) Node { now := mtime.Now() - addedPropertiesCount := 0 for label, value := range propertyList { - if addedPropertiesCount >= MaxTableRows { - break - } node = node.WithLatest(prefix+label, now, value) - addedPropertiesCount++ } - return node.withTableTruncationInformation(prefix, len(propertyList), now) + return node } // WithoutPrefix returns the string with trimmed prefix and a @@ -137,7 +115,12 @@ func (node Node) ExtractPropertyList(template TableTemplate) (rows []Row) { return rows } -// ExtractTable returns the rows to build either a property list or a generic table from this node +// ExtractTable returns the rows to build either a property list or a +// generic table from this node. It also returns the number of rows, +// if any, that were truncated. The probes used to limit the number of +// labels, env vars and Weave Net connections they report, but this +// logic has since been removed. So the code here dealing with +// truncation is only retained in order to handle legacy reports. func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCount int) { switch template.Type { case MulticolumnTableType: diff --git a/report/table_test.go b/report/table_test.go index fa94d5d8c..0c2529d06 100644 --- a/report/table_test.go +++ b/report/table_test.go @@ -1,7 +1,6 @@ package report_test import ( - "fmt" "reflect" "testing" @@ -132,70 +131,6 @@ func TestFixedPropertyLists(t *testing.T) { } } -func TestPropertyListTruncation(t *testing.T) { - wantTruncationCount := 1 - propertyList := map[string]string{} - for i := 0; i < report.MaxTableRows+wantTruncationCount; i++ { - key := fmt.Sprintf("key%d", i) - value := fmt.Sprintf("value%d", i) - propertyList[key] = value - } - - nmd := report.MakeNode("foo1") - nmd = nmd.AddPrefixPropertyList("foo_", propertyList) - - template := report.TableTemplate{ - Type: report.PropertyListType, - Prefix: "foo_", - } - - _, truncationCount := nmd.ExtractTable(template) - - if truncationCount != wantTruncationCount { - t.Error( - "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{ { From 36ea7907e422a67399cd817c71c16cca29f129b7 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 12 Apr 2018 17:24:29 +0100 Subject: [PATCH 2/2] export less & comment more to make the linter happy --- report/table.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/report/table.go b/report/table.go index a36f8e217..9e0f90279 100644 --- a/report/table.go +++ b/report/table.go @@ -10,11 +10,15 @@ import ( "github.com/weaveworks/common/mtime" ) +// Table types const ( - TableEntryKeySeparator = "___" - TruncationCountPrefix = "table_truncation_count_" - MulticolumnTableType = "multicolumn-table" - PropertyListType = "property-list" + MulticolumnTableType = "multicolumn-table" + PropertyListType = "property-list" +) + +const ( + tableEntryKeySeparator = "___" + truncationCountPrefix = "table_truncation_count_" ) // AddPrefixMulticolumnTable appends arbitrary rows to the Node, returning a new node. @@ -23,7 +27,7 @@ func (node Node) AddPrefixMulticolumnTable(prefix string, rows []Row) Node { for _, row := range rows { // Add all the row values as separate entries for columnID, value := range row.Entries { - key := strings.Join([]string{row.ID, columnID}, TableEntryKeySeparator) + key := strings.Join([]string{row.ID, columnID}, tableEntryKeySeparator) node = node.WithLatest(prefix+key, now, value) } } @@ -59,7 +63,7 @@ func (node Node) ExtractMulticolumnTable(template TableTemplate) (rows []Row) { // 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) + 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 { @@ -130,7 +134,7 @@ func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCou } truncationCount = 0 - if str, ok := node.Latest.Lookup(TruncationCountPrefix + template.Prefix); ok { + 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) }