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.
This commit is contained in:
Patrick Hemmer 2018-05-29 22:03:37 -04:00 committed by Daniel Nelson
parent 59f0a5354f
commit 5ae2b02f5d
6 changed files with 35 additions and 10 deletions

View File

@ -68,6 +68,7 @@
- [#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]

View File

@ -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
}

View File

@ -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"},

View File

@ -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},
}

View File

@ -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"),
@ -159,19 +159,23 @@ func TestFieldInit(t *testing.T) {
func TestTableInit(t *testing.T) {
tbl := Table{
Oid: ".1.0.0.0",
Fields: []Field{{Oid: ".999", Name: "foo"}},
Oid: ".1.0.0.0",
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",
@ -579,7 +584,7 @@ func TestGather(t *testing.T) {
Fields: []Field{
{
Name: "myOtherField",
Oid: ".1.0.0.0.1.4",
Oid: ".1.0.0.0.1.5",
},
},
},

View File

@ -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