Fix snmp overriding of auto-configured table fields (#4208)

Whenever the snmp plugin was configured with a table with automatic field
discovery, if one of those fields was explicitly overridden in the config and
the value of is_tag was changed, the field would be duplicated, once as a tag
& once as a field.

This change fixes the behavior so that if a field is explicitly configured,
automatic table field discovery doesn't touch it.

(cherry picked from commit 5ae2b02f5d)

CHANGELOG.md

@@ -1,3 +1,75 @@
+## v1.7 [unreleased]
+
+### Release Notes
+
+- The `cassandra` input plugin has been deprecated in favor of the `jolokia2`
+  input plugin which is much more configurable and more performant.  There is
+  an [example configuration](./plugins/inputs/jolokia2/examples) to help you
+  get started.
+
+- For plugins supporting TLS, you can now specify the certificate and keys
+  using `tls_ca`, `tls_cert`, `tls_key`.  These options behave the same as
+  the, now deprecated, `ssl` forms.
+
+### New Inputs
+
+- [aurora](./plugins/inputs/aurora/README.md) - Contributed by @influxdata
+- [burrow](./plugins/inputs/burrow/README.md) - Contributed by @arkady-emelyanov
+- [fibaro](./plugins/inputs/fibaro/README.md) - Contributed by @dynek
+- [jti_openconfig_telemetry](./plugins/inputs/jti_openconfig_telemetry/README.md) - Contributed by @ajhai
+- [mcrouter](./plugins/inputs/mcrouter/README.md) - Contributed by @cthayer
+- [nvidia_smi](./plugins/inputs/nvidia_smi/README.md) - Contributed by @jackzampolin
+- [syslog](./plugins/inputs/syslog/README.md) - Contributed by @influxdata
+
+### New Processors
+
+- [converter](./plugins/processors/converter/README.md) - Contributed by @influxdata
+- [regex](./plugins/processors/regex/README.md) - Contributed by @44px
+- [topk](./plugins/processors/topk/README.md) - Contributed by @mirath
+
+### New Outputs
+
+- [http](./plugins/outputs/http/README.md) - Contributed by @Dark0096
+- [application_insights](./plugins/outputs/application_insights/README.md): Contribute by @karolz-ms
+
+### Features
+
+- [#3964](https://github.com/influxdata/telegraf/pull/3964): Add repl_oplog_window_sec metric to mongodb input.
+- [#3819](https://github.com/influxdata/telegraf/pull/3819): Add per-host shard metrics in mongodb input.
+- [#3999](https://github.com/influxdata/telegraf/pull/3999): Skip files with leading `..` in config directory.
+- [#4021](https://github.com/influxdata/telegraf/pull/4021): Add TLS support to socket_writer and socket_listener plugins.
+- [#4025](https://github.com/influxdata/telegraf/pull/4025): Add snmp input option to strip non fixed length index suffixes.
+- [#4035](https://github.com/influxdata/telegraf/pull/4035): Add server version tag to docker input.
+- [#4044](https://github.com/influxdata/telegraf/pull/4044): Add support for LeoFS 1.4 to leofs input.
+- [#4068](https://github.com/influxdata/telegraf/pull/4068): Add parameter to force the interval of gather for sysstat.
+- [#3877](https://github.com/influxdata/telegraf/pull/3877): Support busybox ping in the ping input.
+- [#4077](https://github.com/influxdata/telegraf/pull/4077): Add input plugin for McRouter.
+- [#4096](https://github.com/influxdata/telegraf/pull/4096): Add topk processor plugin.
+- [#4114](https://github.com/influxdata/telegraf/pull/4114): Add cursor metrics to mongodb input.
+- [#3455](https://github.com/influxdata/telegraf/pull/3455): Add tag/integer pair for result to net_response.
+- [#4010](https://github.com/influxdata/telegraf/pull/3455): Add application_insights output plugin.
+- [#4167](https://github.com/influxdata/telegraf/pull/4167): Added several important elasticsearch cluster health metrics.
+- [#4094](https://github.com/influxdata/telegraf/pull/4094): Add batch mode to mqtt output.
+- [#4158](https://github.com/influxdata/telegraf/pull/4158): Add aurora input plugin.
+- [#3839](https://github.com/influxdata/telegraf/pull/3839): Add regex processor plugin.
+- [#4165](https://github.com/influxdata/telegraf/pull/4165): Add support for Graphite 1.1 tags.
+- [#4162](https://github.com/influxdata/telegraf/pull/4162): Add timeout option to sensors input.
+- [#3489](https://github.com/influxdata/telegraf/pull/3489): Add burrow input plugin.
+- [#3969](https://github.com/influxdata/telegraf/pull/3969): Add option to unbound module to use threads as tags.
+- [#4183](https://github.com/influxdata/telegraf/pull/4183): Add support for TLS and username/password auth to aerospike input.
+- [#4190](https://github.com/influxdata/telegraf/pull/4190): Add special syslog timestamp parser to grok parser that uses current year.
+- [#4181](https://github.com/influxdata/telegraf/pull/4181): Add syslog input plugin.
+
+### Bugfixes
+
+- [#4018](https://github.com/influxdata/telegraf/pull/4018): Write to working file outputs if any files are not writeable.
+- [#4036](https://github.com/influxdata/telegraf/pull/4036): Add all win_perf_counters fields for a series in a single metric.
+- [#4118](https://github.com/influxdata/telegraf/pull/4118): Report results of dns_query instead of 0ms on timeout.
+- [#4155](https://github.com/influxdata/telegraf/pull/4155): Add consul service tags to metric.
+- [#2879](https://github.com/influxdata/telegraf/issues/2879): Fix wildcards and multi instance processes in win_perf_counters.
+- [#2468](https://github.com/influxdata/telegraf/issues/2468): Fix crash on 32-bit Windows in win_perf_counters.
+- [#4203](https://github.com/influxdata/telegraf/issues/4203): Fix snmp overriding of auto-configured table fields.
+
 ## v1.6.3 [2018-05-21]
 
 ### Bugfixes

plugins/inputs/snmp/snmp.go

@@ -218,10 +218,20 @@ func (t *Table) initBuild() error {
 	if err != nil {
 		return err
 	}
+
 	if t.Name == "" {
 		t.Name = oidText
 	}
-	t.Fields = append(t.Fields, fields...)
+
+	knownOIDs := map[string]bool{}
+	for _, f := range t.Fields {
+		knownOIDs[f.Oid] = true
+	}
+	for _, f := range fields {
+		if !knownOIDs[f.Oid] {
+			t.Fields = append(t.Fields, f)
+		}
+	}
 
 	return nil
 }

plugins/inputs/snmp/snmp_mocks_generate.go

@@ -21,7 +21,7 @@ var mockedCommands = [][]string{
 	{"snmptranslate", "-Td", "-Ob", "-m", "all", "1.0.0.1.1"},
 	{"snmptranslate", "-Td", "-Ob", "-m", "all", ".1.0.0.0.1.1"},
 	{"snmptranslate", "-Td", "-Ob", "-m", "all", ".1.0.0.0.1.1.0"},
-	{"snmptranslate", "-Td", "-Ob", "-m", "all", ".1.0.0.0.1.4"},
+	{"snmptranslate", "-Td", "-Ob", "-m", "all", ".1.0.0.0.1.5"},
 	{"snmptranslate", "-Td", "-Ob", "-m", "all", ".1.2.3"},
 	{"snmptranslate", "-Td", "-Ob", ".iso.2.3"},
 	{"snmptranslate", "-Td", "-Ob", "-m", "all", ".999"},
@@ -30,6 +30,7 @@ var mockedCommands = [][]string{
 	{"snmptranslate", "-Td", "-Ob", "TEST::testTable"},
 	{"snmptranslate", "-Td", "-Ob", "TEST::connections"},
 	{"snmptranslate", "-Td", "-Ob", "TEST::latency"},
+	{"snmptranslate", "-Td", "-Ob", "TEST::description"},
 	{"snmptranslate", "-Td", "-Ob", "TEST::hostname"},
 	{"snmptranslate", "-Td", "-Ob", "IF-MIB::ifPhysAddress.1"},
 	{"snmptranslate", "-Td", "-Ob", "BRIDGE-MIB::dot1dTpFdbAddress.1"},

plugins/inputs/snmp/snmp_mocks_test.go

@@ -67,7 +67,7 @@ var mockedCommandResults = map[string]mockedCommandResult{
 	"snmptranslate\x00-Td\x00-Ob\x00-m\x00all\x001.0.0.1.1":                   mockedCommandResult{stdout: "TEST::hostname\nhostname OBJECT-TYPE\n  -- FROM\tTEST\n  SYNTAX\tOCTET STRING\n  MAX-ACCESS\tread-only\n  STATUS\tcurrent\n::= { iso(1) 0 testOID(0) 1 1 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00-m\x00all\x00.1.0.0.0.1.1":                mockedCommandResult{stdout: "TEST::server\nserver OBJECT-TYPE\n  -- FROM\tTEST\n  SYNTAX\tOCTET STRING\n  MAX-ACCESS\tread-only\n  STATUS\tcurrent\n::= { iso(1) 0 testOID(0) testTable(0) testTableEntry(1) 1 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00-m\x00all\x00.1.0.0.0.1.1.0":              mockedCommandResult{stdout: "TEST::server.0\nserver OBJECT-TYPE\n  -- FROM\tTEST\n  SYNTAX\tOCTET STRING\n  MAX-ACCESS\tread-only\n  STATUS\tcurrent\n::= { iso(1) 0 testOID(0) testTable(0) testTableEntry(1) server(1) 0 }\n", stderr: "", exitError: false},
-	"snmptranslate\x00-Td\x00-Ob\x00-m\x00all\x00.1.0.0.0.1.4":                mockedCommandResult{stdout: "TEST::testTableEntry.4\ntestTableEntry OBJECT-TYPE\n  -- FROM\tTEST\n  MAX-ACCESS\tnot-accessible\n  STATUS\tcurrent\n  INDEX\t\t{ server }\n::= { iso(1) 0 testOID(0) testTable(0) testTableEntry(1) 4 }\n", stderr: "", exitError: false},
+	"snmptranslate\x00-Td\x00-Ob\x00-m\x00all\x00.1.0.0.0.1.5":                mockedCommandResult{stdout: "TEST::testTableEntry.5\ntestTableEntry OBJECT-TYPE\n  -- FROM\tTEST\n  MAX-ACCESS\tnot-accessible\n  STATUS\tcurrent\n  INDEX\t\t{ server }\n::= { iso(1) 0 testOID(0) testTable(0) testTableEntry(1) 5 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00-m\x00all\x00.1.2.3":                      mockedCommandResult{stdout: "iso.2.3\niso OBJECT-TYPE\n  -- FROM\t#-1\n::= { iso(1) 2 3 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00.iso.2.3":                                 mockedCommandResult{stdout: "iso.2.3\niso OBJECT-TYPE\n  -- FROM\t#-1\n::= { iso(1) 2 3 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00-m\x00all\x00.999":                        mockedCommandResult{stdout: ".999\n [TRUNCATED]\n", stderr: "", exitError: false},
@@ -76,10 +76,11 @@ var mockedCommandResults = map[string]mockedCommandResult{
 	"snmptranslate\x00-Td\x00-Ob\x00TEST::testTable":                          mockedCommandResult{stdout: "TEST::testTable\ntestTable OBJECT-TYPE\n  -- FROM\tTEST\n  MAX-ACCESS\tnot-accessible\n  STATUS\tcurrent\n::= { iso(1) 0 testOID(0) 0 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00TEST::connections":                        mockedCommandResult{stdout: "TEST::connections\nconnections OBJECT-TYPE\n  -- FROM\tTEST\n  SYNTAX\tINTEGER\n  MAX-ACCESS\tread-only\n  STATUS\tcurrent\n::= { iso(1) 0 testOID(0) testTable(0) testTableEntry(1) 2 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00TEST::latency":                            mockedCommandResult{stdout: "TEST::latency\nlatency OBJECT-TYPE\n  -- FROM\tTEST\n  SYNTAX\tOCTET STRING\n  MAX-ACCESS\tread-only\n  STATUS\tcurrent\n::= { iso(1) 0 testOID(0) testTable(0) testTableEntry(1) 3 }\n", stderr: "", exitError: false},
+	"snmptranslate\x00-Td\x00-Ob\x00TEST::description":                        mockedCommandResult{stdout: "TEST::description\ndescription OBJECT-TYPE\n  -- FROM\tTEST\n  SYNTAX\tOCTET STRING\n  MAX-ACCESS\tread-only\n  STATUS\tcurrent\n::= { iso(1) 0 testOID(0) testTable(0) testTableEntry(1) 4 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00TEST::hostname":                           mockedCommandResult{stdout: "TEST::hostname\nhostname OBJECT-TYPE\n  -- FROM\tTEST\n  SYNTAX\tOCTET STRING\n  MAX-ACCESS\tread-only\n  STATUS\tcurrent\n::= { iso(1) 0 testOID(0) 1 1 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00IF-MIB::ifPhysAddress.1":                  mockedCommandResult{stdout: "IF-MIB::ifPhysAddress.1\nifPhysAddress OBJECT-TYPE\n  -- FROM\tIF-MIB\n  -- TEXTUAL CONVENTION PhysAddress\n  SYNTAX\tOCTET STRING\n  DISPLAY-HINT\t\"1x:\"\n  MAX-ACCESS\tread-only\n  STATUS\tcurrent\n  DESCRIPTION\t\"The interface's address at its protocol sub-layer.  For\n            example, for an 802.x interface, this object normally\n            contains a MAC address.  The interface's media-specific MIB\n            must define the bit and byte ordering and the format of the\n            value of this object.  For interfaces which do not have such\n            an address (e.g., a serial line), this object should contain\n            an octet string of zero length.\"\n::= { iso(1) org(3) dod(6) internet(1) mgmt(2) mib-2(1) interfaces(2) ifTable(2) ifEntry(1) ifPhysAddress(6) 1 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00BRIDGE-MIB::dot1dTpFdbAddress.1":          mockedCommandResult{stdout: "BRIDGE-MIB::dot1dTpFdbAddress.1\ndot1dTpFdbAddress OBJECT-TYPE\n  -- FROM\tBRIDGE-MIB\n  -- TEXTUAL CONVENTION MacAddress\n  SYNTAX\tOCTET STRING (6) \n  DISPLAY-HINT\t\"1x:\"\n  MAX-ACCESS\tread-only\n  STATUS\tcurrent\n  DESCRIPTION\t\"A unicast MAC address for which the bridge has\n        forwarding and/or filtering information.\"\n::= { iso(1) org(3) dod(6) internet(1) mgmt(2) mib-2(1) dot1dBridge(17) dot1dTp(4) dot1dTpFdbTable(3) dot1dTpFdbEntry(1) dot1dTpFdbAddress(1) 1 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00-Ob\x00TCP-MIB::tcpConnectionLocalAddress.1":     mockedCommandResult{stdout: "TCP-MIB::tcpConnectionLocalAddress.1\ntcpConnectionLocalAddress OBJECT-TYPE\n  -- FROM\tTCP-MIB\n  -- TEXTUAL CONVENTION InetAddress\n  SYNTAX\tOCTET STRING (0..255) \n  MAX-ACCESS\tnot-accessible\n  STATUS\tcurrent\n  DESCRIPTION\t\"The local IP address for this TCP connection.  The type\n            of this address is determined by the value of\n            tcpConnectionLocalAddressType.\n\n            As this object is used in the index for the\n            tcpConnectionTable, implementors should be\n            careful not to create entries that would result in OIDs\n            with more than 128 subidentifiers; otherwise the information\n            cannot be accessed by using SNMPv1, SNMPv2c, or SNMPv3.\"\n::= { iso(1) org(3) dod(6) internet(1) mgmt(2) mib-2(1) tcp(6) tcpConnectionTable(19) tcpConnectionEntry(1) tcpConnectionLocalAddress(2) 1 }\n", stderr: "", exitError: false},
 	"snmptranslate\x00-Td\x00TEST::testTable.1":                               mockedCommandResult{stdout: "TEST::testTableEntry\ntestTableEntry OBJECT-TYPE\n  -- FROM\tTEST\n  MAX-ACCESS\tnot-accessible\n  STATUS\tcurrent\n  INDEX\t\t{ server }\n::= { iso(1) 0 testOID(0) testTable(0) 1 }\n", stderr: "", exitError: false},
-	"snmptable\x00-Ch\x00-Cl\x00-c\x00public\x00127.0.0.1\x00TEST::testTable": mockedCommandResult{stdout: "server connections latency \nTEST::testTable: No entries\n", stderr: "", exitError: false},
+	"snmptable\x00-Ch\x00-Cl\x00-c\x00public\x00127.0.0.1\x00TEST::testTable": mockedCommandResult{stdout: "server connections latency description \nTEST::testTable: No entries\n", stderr: "", exitError: false},
 }

plugins/inputs/snmp/snmp_test.go

@@ -72,7 +72,7 @@ var tsc = &testSNMPConnection{
 		".1.0.0.0.1.3.1":     "0.456",
 		".1.0.0.0.1.3.2":     "0.000",
 		".1.0.0.0.1.3.3":     "9.999",
-		".1.0.0.0.1.4.0":     123456,
+		".1.0.0.0.1.5.0":     123456,
 		".1.0.0.1.1":         "baz",
 		".1.0.0.1.2":         234,
 		".1.0.0.1.3":         []byte("byte slice"),
@@ -160,18 +160,22 @@ func TestFieldInit(t *testing.T) {
 func TestTableInit(t *testing.T) {
 	tbl := Table{
 		Oid: ".1.0.0.0",
-		Fields: []Field{{Oid: ".999", Name: "foo"}},
+		Fields: []Field{
+			{Oid: ".999", Name: "foo"},
+			{Oid: "TEST::description", Name: "description", IsTag: true},
+		},
 	}
 	err := tbl.init()
 	require.NoError(t, err)
 
 	assert.Equal(t, "testTable", tbl.Name)
 
-	assert.Len(t, tbl.Fields, 4)
+	assert.Len(t, tbl.Fields, 5)
 	assert.Contains(t, tbl.Fields, Field{Oid: ".999", Name: "foo", initialized: true})
 	assert.Contains(t, tbl.Fields, Field{Oid: ".1.0.0.0.1.1", Name: "server", IsTag: true, initialized: true})
 	assert.Contains(t, tbl.Fields, Field{Oid: ".1.0.0.0.1.2", Name: "connections", initialized: true})
 	assert.Contains(t, tbl.Fields, Field{Oid: ".1.0.0.0.1.3", Name: "latency", initialized: true})
+	assert.Contains(t, tbl.Fields, Field{Oid: ".1.0.0.0.1.4", Name: "description", IsTag: true, initialized: true})
 }
 
 func TestSnmpInit(t *testing.T) {
@@ -187,10 +191,11 @@ func TestSnmpInit(t *testing.T) {
 	err := s.init()
 	require.NoError(t, err)
 
-	assert.Len(t, s.Tables[0].Fields, 3)
+	assert.Len(t, s.Tables[0].Fields, 4)
 	assert.Contains(t, s.Tables[0].Fields, Field{Oid: ".1.0.0.0.1.1", Name: "server", IsTag: true, initialized: true})
 	assert.Contains(t, s.Tables[0].Fields, Field{Oid: ".1.0.0.0.1.2", Name: "connections", initialized: true})
 	assert.Contains(t, s.Tables[0].Fields, Field{Oid: ".1.0.0.0.1.3", Name: "latency", initialized: true})
+	assert.Contains(t, s.Tables[0].Fields, Field{Oid: ".1.0.0.0.1.4", Name: "description", initialized: true})
 
 	assert.Equal(t, Field{
 		Oid:         ".1.0.0.1.1",
@@ -572,7 +577,7 @@ func TestGather(t *testing.T) {
 				Fields: []Field{
 					{
 						Name: "myOtherField",
-						Oid:  ".1.0.0.0.1.4",
+						Oid:  ".1.0.0.0.1.5",
 					},
 				},
 			},

plugins/inputs/snmp/testdata/test.mib

@@ -22,6 +22,7 @@ TestTableEntry ::=
 		server OCTET STRING,
 		connections  INTEGER,
 		latency  OCTET STRING,
+		description OCTET STRING,
 	}
 
 server OBJECT-TYPE
@@ -42,6 +43,12 @@ latency OBJECT-TYPE
 	STATUS current
 	::= { testTableEntry 3 }
 
+description OBJECT-TYPE
+	SYNTAX OCTET STRING
+	MAX-ACCESS read-only
+	STATUS current
+	::= { testTableEntry 4 }
+
 hostname OBJECT-TYPE
 	SYNTAX OCTET STRING
 	MAX-ACCESS read-only