From bdda6a10ed855b887ae09ce552e8f14990d9b106 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Wed, 30 Oct 2019 14:22:18 -0600
Subject: [PATCH 01/26] Add snmp trap receiving using gosnmp library

---
 plugins/inputs/all/all.go                  |   1 +
 plugins/inputs/snmp_trap/snmp_trap.go      | 127 +++++++++++++++++++
 plugins/inputs/snmp_trap/snmp_trap_test.go | 135 +++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 plugins/inputs/snmp_trap/snmp_trap.go
 create mode 100644 plugins/inputs/snmp_trap/snmp_trap_test.go

diff --git a/plugins/inputs/all/all.go b/plugins/inputs/all/all.go
index 6934266429b2d..e1a528810fcac 100644
--- a/plugins/inputs/all/all.go
+++ b/plugins/inputs/all/all.go
@@ -135,6 +135,7 @@ import (
 	_ "github.com/influxdata/telegraf/plugins/inputs/smart"
 	_ "github.com/influxdata/telegraf/plugins/inputs/snmp"
 	_ "github.com/influxdata/telegraf/plugins/inputs/snmp_legacy"
+	_ "github.com/influxdata/telegraf/plugins/inputs/snmp_trap"
 	_ "github.com/influxdata/telegraf/plugins/inputs/socket_listener"
 	_ "github.com/influxdata/telegraf/plugins/inputs/solr"
 	_ "github.com/influxdata/telegraf/plugins/inputs/sqlserver"
diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
new file mode 100644
index 0000000000000..f3458d7a61b1f
--- /dev/null
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -0,0 +1,127 @@
+package snmp_trap
+
+import (
+	"fmt"
+	"log"
+	"net"
+	"strconv"
+	"sync"
+	"time"
+
+	"github.com/influxdata/telegraf"
+	"github.com/influxdata/telegraf/plugins/inputs"
+
+	"github.com/soniah/gosnmp"
+)
+
+type SnmpTrap struct {
+	Port uint16 `toml:"port"`
+	//todo add mib settings
+
+	acc      telegraf.Accumulator
+	listener *gosnmp.TrapListener
+	wg       sync.WaitGroup
+	timeFunc func() time.Time
+
+	makeHandlerWrapper func(func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)) func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)
+	Errch              chan error
+}
+
+var sampleConfig = `
+  ## Port to listen on.  Default 162
+  #port = 162
+`
+
+func (s *SnmpTrap) SampleConfig() string {
+	return sampleConfig
+}
+
+func (s *SnmpTrap) Description() string {
+	return "Receive SNMP traps"
+}
+
+func (s *SnmpTrap) Gather(_ telegraf.Accumulator) error {
+	return nil
+}
+
+func init() {
+	inputs.Add("snmp_trap", func() telegraf.Input {
+		return &SnmpTrap{
+			timeFunc: time.Now,
+			Port:     162,
+		}
+	})
+}
+
+func (s *SnmpTrap) Init() error {
+	return nil
+}
+
+func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
+	s.acc = acc
+	s.listener = gosnmp.NewTrapListener()
+	s.listener.OnNewTrap = makeTrapHandler(s)
+	s.listener.Params = gosnmp.Default
+	//s.listener.Params.Logger = log.New(os.Stdout, "", 0)
+
+	//wrap the handler, used in unit tests
+	if nil != s.makeHandlerWrapper {
+		s.listener.OnNewTrap = s.makeHandlerWrapper(s.listener.OnNewTrap)
+	}
+
+	s.wg.Add(1)
+	go func() {
+		defer s.wg.Done()
+
+		//no ip means listen on all interfaces, ipv4 and ipv6
+		err := s.listener.Listen(":" + strconv.FormatUint(uint64(s.Port), 10))
+		if err != nil {
+			s.Errch <- err
+			log.Panicf("error in listen: %s", err)
+		}
+	}()
+
+	return nil
+}
+
+func (s *SnmpTrap) Stop() {
+	s.listener.Close()
+	s.wg.Wait()
+}
+
+func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr) {
+	return func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr) {
+		tm := s.timeFunc()
+		fields := map[string]interface{}{}
+		tags := map[string]string{}
+
+		for _, v := range packet.Variables {
+			//todo: determine which snmp variables are tags and which
+			//are fields.  for now everything's a tag
+
+			//todo: look up v.Name smi
+			var name string
+			name = v.Name
+
+			//todo: format value based on its snmp type
+			var value string
+			switch v.Type {
+			//case gosnmp.OctetString:
+			//b := v.Value.([]byte)
+			//case gosnmp.ObjectIdentifier:
+			//todo: look up v.Value smi
+			default:
+				value = fmt.Sprintf("%v", v.Value)
+			}
+
+			tags[name] = value //fmt.Sprintf("%v", v.Value)
+			tags[name+"_type"] = fmt.Sprintf("%v", v.Type)
+		}
+		fields["foo"] = "bar"
+		s.acc.AddFields("snmp_trap", fields, tags, tm)
+	}
+}
+
+func (s *SnmpTrap) Listening() <-chan bool {
+	return s.listener.Listening()
+}
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
new file mode 100644
index 0000000000000..5a7cbb4afbf59
--- /dev/null
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -0,0 +1,135 @@
+package snmp_trap
+
+//todo: look up smi
+
+import (
+	//"log"
+	//"os"
+	"fmt"
+	"net"
+	"testing"
+	"time"
+
+	"github.com/soniah/gosnmp"
+
+	"github.com/influxdata/telegraf"
+	"github.com/influxdata/telegraf/testutil"
+)
+
+func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
+	s := &gosnmp.GoSNMP{
+		Port:      port,
+		Community: "public",
+		Version:   gosnmp.Version2c,
+		Timeout:   time.Duration(2) * time.Second,
+		Retries:   3,
+		MaxOids:   gosnmp.MaxOids,
+		Target:    "127.0.0.1",
+		//Logger:    log.New(os.Stdout, "", 0),
+	}
+
+	err := s.Connect()
+	if err != nil {
+		t.Errorf("Connect() err: %v", err)
+	}
+	defer s.Conn.Close()
+
+	//if the first pdu isn't type TimeTicks, gosnmp.SendTrap() will
+	//prepend one with time.Now().  We need to check the time later on
+	//so we have to do add it here.
+	now := uint32(time.Now().Unix())
+	timePdu := gosnmp.SnmpPDU{
+		Name:  ".1.3.6.1.2.1.1.3.0",
+		Type:  gosnmp.TimeTicks,
+		Value: now,
+	}
+
+	pdu := gosnmp.SnmpPDU{
+		Name:  ".1.3.6.1.6.3.1.1.4.1.0",
+		Type:  gosnmp.ObjectIdentifier,
+		Value: ".1.3.6.1.6.3.1.1.5.1",
+	}
+
+	trap := gosnmp.SnmpTrap{
+		Variables: []gosnmp.SnmpPDU{
+			timePdu,
+			pdu,
+		},
+	}
+
+	_, err = s.SendTrap(trap)
+	if err != nil {
+		t.Errorf("SendTrap() err: %v", err)
+	}
+
+	return now
+}
+
+// TestReceiveTrap
+func TestReceiveTrap(t *testing.T) {
+	const port = 12399 //todo: find unused port
+	var fakeTime = time.Now()
+
+	//hook into the trap handler so the test knows when the trap has been received
+	received := make(chan int)
+	wrap := func(f func(*gosnmp.SnmpPacket, *net.UDPAddr)) func(*gosnmp.SnmpPacket, *net.UDPAddr) {
+		return func(p *gosnmp.SnmpPacket, a *net.UDPAddr) {
+			f(p, a)
+			received <- 0
+		}
+	}
+
+	//set up the service input plugin
+	n := &SnmpTrap{
+		Port:               port,
+		makeHandlerWrapper: wrap,
+		timeFunc: func() time.Time {
+			return fakeTime
+		},
+	}
+	n.Init()
+	var acc testutil.Accumulator
+	n.Start(&acc)
+	defer n.Stop()
+
+	//wait until input plugin is listening
+	select {
+	case <-n.Listening():
+	case err := <-n.Errch:
+		t.Fatalf("error in listen: %v", err)
+	case <-time.After(2 * time.Second):
+		t.Fatal("timed out waiting to listen")
+	}
+
+	//send the trap
+	sentTimestamp := sendTrap(t, port)
+
+	//wait for trap to be received
+	select {
+	case <-received:
+	case <-time.After(2 * time.Second):
+		t.Fatal("timed out waiting for trap to be received")
+	}
+
+	//validate plugin output
+	expected := []telegraf.Metric{
+		testutil.MustMetric(
+			"snmp_trap", //name
+			map[string]string{ //tags
+				".1.3.6.1.2.1.1.3.0":          fmt.Sprintf("%v", sentTimestamp),
+				".1.3.6.1.2.1.1.3.0_type":     "67",
+				".1.3.6.1.6.3.1.1.4.1.0":      ".1.3.6.1.6.3.1.1.5.1",
+				".1.3.6.1.6.3.1.1.4.1.0_type": "6",
+			},
+			map[string]interface{}{ //fields
+				"foo": "bar",
+			},
+			fakeTime,
+		),
+	}
+
+	testutil.RequireMetricsEqual(t,
+		expected, acc.GetTelegrafMetrics(),
+		testutil.SortMetrics())
+
+}

From de1d648517cb6547c360f76ff6b7a83eec593bce Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Wed, 6 Nov 2019 16:06:31 -0700
Subject: [PATCH 02/26] Improve translation of traps to metrics.

Look up OIDs in SNMP traps using SnmpTranslate() from the SNMP plugin.

Update dep for github.com/soniah/gosnmp to get Asn1BER stringer function.

Add README.md.
---
 Gopkg.lock                                 | 12 +++++-
 plugins/inputs/snmp/snmp.go                |  6 +--
 plugins/inputs/snmp/snmp_test.go           |  4 +-
 plugins/inputs/snmp_trap/README.md         | 33 +++++++++++++++
 plugins/inputs/snmp_trap/snmp_trap.go      | 49 +++++++++++++++-------
 plugins/inputs/snmp_trap/snmp_trap_test.go | 40 ++++++++++++------
 6 files changed, 108 insertions(+), 36 deletions(-)
 create mode 100644 plugins/inputs/snmp_trap/README.md

diff --git a/Gopkg.lock b/Gopkg.lock
index 410b9b284a2e3..0062ecc0a7b9c 100644
--- a/Gopkg.lock
+++ b/Gopkg.lock
@@ -513,6 +513,14 @@
   revision = "636bf0302bc95575d69441b25a2603156ffdddf1"
   version = "v1.1.1"
 
+[[projects]]
+  digest = "1:68c64bb61d55dcd17c82ca0b871ddddb5ae18b30cfe26f6bfd4b6df6287dc2e0"
+  name = "github.com/golang/mock"
+  packages = ["gomock"]
+  pruneopts = ""
+  revision = "9fa652df1129bef0e734c9cf9bf6dbae9ef3b9fa"
+  version = "1.3.1"
+
 [[projects]]
   digest = "1:f958a1c137db276e52f0b50efee41a1a389dcdded59a69711f3e872757dab34b"
   name = "github.com/golang/protobuf"
@@ -1105,11 +1113,11 @@
 
 [[projects]]
   branch = "master"
-  digest = "1:4b0cabe65ca903a7b2a3e6272c5304eb788ce196d35ecb901c6563e5e7582443"
+  digest = "1:c74e7ad448ffae5fd76a71079615467e229c5c7ab4bfa309b80f796fe9c0da82"
   name = "github.com/soniah/gosnmp"
   packages = ["."]
   pruneopts = ""
-  revision = "96b86229e9b3ffb4b954144cdc7f98fe3ee1003f"
+  revision = "b3532de686f6834255deeb98a64768a591f42195"
 
 [[projects]]
   branch = "master"
diff --git a/plugins/inputs/snmp/snmp.go b/plugins/inputs/snmp/snmp.go
index 18eed4e47196d..bff48fb6eb4ee 100644
--- a/plugins/inputs/snmp/snmp.go
+++ b/plugins/inputs/snmp/snmp.go
@@ -277,7 +277,7 @@ func (f *Field) init() error {
 		return nil
 	}
 
-	_, oidNum, oidText, conversion, err := snmpTranslate(f.Oid)
+	_, oidNum, oidText, conversion, err := SnmpTranslate(f.Oid)
 	if err != nil {
 		return Errorf(err, "translating")
 	}
@@ -878,7 +878,7 @@ func snmpTable(oid string) (mibName string, oidNum string, oidText string, field
 }
 
 func snmpTableCall(oid string) (mibName string, oidNum string, oidText string, fields []Field, err error) {
-	mibName, oidNum, oidText, _, err = snmpTranslate(oid)
+	mibName, oidNum, oidText, _, err = SnmpTranslate(oid)
 	if err != nil {
 		return "", "", "", nil, Errorf(err, "translating")
 	}
@@ -948,7 +948,7 @@ var snmpTranslateCachesLock sync.Mutex
 var snmpTranslateCaches map[string]snmpTranslateCache
 
 // snmpTranslate resolves the given OID.
-func snmpTranslate(oid string) (mibName string, oidNum string, oidText string, conversion string, err error) {
+func SnmpTranslate(oid string) (mibName string, oidNum string, oidText string, conversion string, err error) {
 	snmpTranslateCachesLock.Lock()
 	if snmpTranslateCaches == nil {
 		snmpTranslateCaches = map[string]snmpTranslateCache{}
diff --git a/plugins/inputs/snmp/snmp_test.go b/plugins/inputs/snmp/snmp_test.go
index db1a49605df05..32d3745ab99d3 100644
--- a/plugins/inputs/snmp/snmp_test.go
+++ b/plugins/inputs/snmp/snmp_test.go
@@ -708,7 +708,7 @@ func TestFieldConvert(t *testing.T) {
 func TestSnmpTranslateCache_miss(t *testing.T) {
 	snmpTranslateCaches = nil
 	oid := "IF-MIB::ifPhysAddress.1"
-	mibName, oidNum, oidText, conversion, err := snmpTranslate(oid)
+	mibName, oidNum, oidText, conversion, err := SnmpTranslate(oid)
 	assert.Len(t, snmpTranslateCaches, 1)
 	stc := snmpTranslateCaches[oid]
 	require.NotNil(t, stc)
@@ -729,7 +729,7 @@ func TestSnmpTranslateCache_hit(t *testing.T) {
 			err:        fmt.Errorf("e"),
 		},
 	}
-	mibName, oidNum, oidText, conversion, err := snmpTranslate("foo")
+	mibName, oidNum, oidText, conversion, err := SnmpTranslate("foo")
 	assert.Equal(t, "a", mibName)
 	assert.Equal(t, "b", oidNum)
 	assert.Equal(t, "c", oidText)
diff --git a/plugins/inputs/snmp_trap/README.md b/plugins/inputs/snmp_trap/README.md
new file mode 100644
index 0000000000000..40f28a1c5e809
--- /dev/null
+++ b/plugins/inputs/snmp_trap/README.md
@@ -0,0 +1,33 @@
+# SNMP Trap Input Plugin
+
+The SNMP Trap plugin is a service input plugin that receives SNMP
+notifications (traps and inform requests).
+
+Notifications are received on plain UDP. The port to listen is
+configurable.
+
+OIDs can be resolved to strings using system MIB files. This is done
+in same way as the SNMP input plugin. See the section "MIB Lookups" in
+the SNMP [README.md](../snmp/README.md) for details.
+
+### Configuration
+```toml
+  ## Port to listen on.  Default 162
+  #port = 162
+```
+
+### Metrics
+
+- snmp_trap
+  - tags:
+	- trap_name (string, value from SNMPv2-MIB::snmpTrapOID.0 PDU)
+	- trap_version (string, "1" or "2c" or "3")
+  - fields:
+	- $NAME (the type is variable and depends on the PDU)
+	- $NAME_type (string, description of the Asn1BER type of the PDU.  Examples: "Integer", "TimeTicks", "IPAddress")
+
+### Example Output
+```
+> snmp_trap,host=debian,trap_name=coldStart,trap_version=2c sysUpTimeInstance_type="TimeTicks",snmpTrapEnterprise.0="linux",snmpTrapEnterprise.0_type="ObjectIdentifier",sysUpTimeInstance=1i 1573078928344595213
+> snmp_trap,host=debian,trap_name=nsNotifyShutdown,trap_version=2c sysUpTimeInstance=1224i,sysUpTimeInstance_type="TimeTicks",snmpTrapEnterprise.0="netSnmpNotificationPrefix",snmpTrapEnterprise.0_type="ObjectIdentifier" 1573078928299642679
+```
diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index f3458d7a61b1f..566ac5f0010e9 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -10,13 +10,13 @@ import (
 
 	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/plugins/inputs"
+	"github.com/influxdata/telegraf/plugins/inputs/snmp"
 
 	"github.com/soniah/gosnmp"
 )
 
 type SnmpTrap struct {
 	Port uint16 `toml:"port"`
-	//todo add mib settings
 
 	acc      telegraf.Accumulator
 	listener *gosnmp.TrapListener
@@ -95,29 +95,46 @@ func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPA
 		fields := map[string]interface{}{}
 		tags := map[string]string{}
 
+		tags["trap_version"] = packet.Version.String()
+
 		for _, v := range packet.Variables {
-			//todo: determine which snmp variables are tags and which
-			//are fields.  for now everything's a tag
+			//build a name and value for each variable to use as tags
+			//and fields.  defaults are the uninterpreted values
+			name := v.Name
+			value := v.Value
+
+			//use system mibs to resolve the name if possible
+			_, _, oidText, _, err := snmp.SnmpTranslate(v.Name)
+			if nil == err {
+				name = oidText //would mib name be useful here?
+			}
 
-			//todo: look up v.Name smi
-			var name string
-			name = v.Name
+			//todo: format the pdu value based on its snmp type and
+			//the mib's textual convention.  The snmp input plugin
+			//only handles textual convention for ip and mac addresses
 
-			//todo: format value based on its snmp type
-			var value string
 			switch v.Type {
 			//case gosnmp.OctetString:
-			//b := v.Value.([]byte)
-			//case gosnmp.ObjectIdentifier:
-			//todo: look up v.Value smi
-			default:
-				value = fmt.Sprintf("%v", v.Value)
+				//b := v.Value.([]byte)
+			case gosnmp.ObjectIdentifier:
+				s, ok := v.Value.(string)
+				if (ok) {
+					if _, _, oidText, _, err := snmp.SnmpTranslate(s); ok && nil == err {
+						value = oidText //would mib name be useful here?
+					}
+				}
+				//1.3.6.1.6.3.1.1.4.1.0 is SNMPv2-MIB::snmpTrapOID.0.  If
+				//v.Name is this oid, set a tag of the trap name.
+				if (v.Name == ".1.3.6.1.6.3.1.1.4.1.0") {
+					tags["trap_name"] = fmt.Sprintf("%v", value)
+					continue
+				}
 			}
 
-			tags[name] = value //fmt.Sprintf("%v", v.Value)
-			tags[name+"_type"] = fmt.Sprintf("%v", v.Type)
+			fields[name] = value
+			fields[name+"_type"] = v.Type.String()
 		}
-		fields["foo"] = "bar"
+
 		s.acc.AddFields("snmp_trap", fields, tags, tm)
 	}
 }
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 5a7cbb4afbf59..333a14752e357 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -1,11 +1,13 @@
 package snmp_trap
 
-//todo: look up smi
+//todo: tests that look up oids will pass only if snmptranslate (part
+//of net-snmp) is installed and working.  We need to mock name lookup
+//or add a way to disable it so tests will pass when snmptranslate
+//isn't available.
 
 import (
 	//"log"
 	//"os"
-	"fmt"
 	"net"
 	"testing"
 	"time"
@@ -14,8 +16,20 @@ import (
 
 	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/testutil"
+
+	"github.com/influxdata/telegraf/plugins/inputs/snmp"
+	"github.com/stretchr/testify/require"
 )
 
+func TestTranslate(t *testing.T) {
+	mibName, oidNum, oidText, conversion, err := snmp.SnmpTranslate(".1.3.6.1.6.3.1.1.5.1")
+	require.NoError(t, err)
+	require.Equal(t, "SNMPv2-MIB", mibName)
+	require.Equal(t, ".1.3.6.1.6.3.1.1.5.1", oidNum)
+	require.Equal(t, "coldStart", oidText)
+	require.Equal(t, "", conversion)
+}
+
 func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 	s := &gosnmp.GoSNMP{
 		Port:      port,
@@ -34,9 +48,10 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 	}
 	defer s.Conn.Close()
 
-	//if the first pdu isn't type TimeTicks, gosnmp.SendTrap() will
-	//prepend one with time.Now().  We need to check the time later on
-	//so we have to do add it here.
+	//If the first pdu isn't type TimeTicks, gosnmp.SendTrap() will
+	//prepend one with time.Now().  The time value is part of the
+	//plugin output so we need to keep track of it and verify it
+	//later.
 	now := uint32(time.Now().Unix())
 	timePdu := gosnmp.SnmpPDU{
 		Name:  ".1.3.6.1.2.1.1.3.0",
@@ -45,9 +60,9 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 	}
 
 	pdu := gosnmp.SnmpPDU{
-		Name:  ".1.3.6.1.6.3.1.1.4.1.0",
+		Name:  ".1.3.6.1.6.3.1.1.4.1.0", //SNMPv2-MIB::snmpTrapOID.0
 		Type:  gosnmp.ObjectIdentifier,
-		Value: ".1.3.6.1.6.3.1.1.5.1",
+		Value: ".1.3.6.1.6.3.1.1.5.1", //coldStart
 	}
 
 	trap := gosnmp.SnmpTrap{
@@ -111,18 +126,17 @@ func TestReceiveTrap(t *testing.T) {
 		t.Fatal("timed out waiting for trap to be received")
 	}
 
-	//validate plugin output
+	//verify plugin output
 	expected := []telegraf.Metric{
 		testutil.MustMetric(
 			"snmp_trap", //name
 			map[string]string{ //tags
-				".1.3.6.1.2.1.1.3.0":          fmt.Sprintf("%v", sentTimestamp),
-				".1.3.6.1.2.1.1.3.0_type":     "67",
-				".1.3.6.1.6.3.1.1.4.1.0":      ".1.3.6.1.6.3.1.1.5.1",
-				".1.3.6.1.6.3.1.1.4.1.0_type": "6",
+				"trap_name": "coldStart",
+				"trap_version": "2c",
 			},
 			map[string]interface{}{ //fields
-				"foo": "bar",
+				"sysUpTimeInstance": sentTimestamp,
+				"sysUpTimeInstance_type": "TimeTicks",
 			},
 			fakeTime,
 		),

From 0f16d1bf4a1ed7759f1e7f6d14bae372bbf03845 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Wed, 6 Nov 2019 16:15:03 -0700
Subject: [PATCH 03/26] gofmt

---
 plugins/inputs/snmp_trap/snmp_trap.go      | 6 +++---
 plugins/inputs/snmp_trap/snmp_trap_test.go | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 566ac5f0010e9..ee59b05fee1a9 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -115,17 +115,17 @@ func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPA
 
 			switch v.Type {
 			//case gosnmp.OctetString:
-				//b := v.Value.([]byte)
+			//b := v.Value.([]byte)
 			case gosnmp.ObjectIdentifier:
 				s, ok := v.Value.(string)
-				if (ok) {
+				if ok {
 					if _, _, oidText, _, err := snmp.SnmpTranslate(s); ok && nil == err {
 						value = oidText //would mib name be useful here?
 					}
 				}
 				//1.3.6.1.6.3.1.1.4.1.0 is SNMPv2-MIB::snmpTrapOID.0.  If
 				//v.Name is this oid, set a tag of the trap name.
-				if (v.Name == ".1.3.6.1.6.3.1.1.4.1.0") {
+				if v.Name == ".1.3.6.1.6.3.1.1.4.1.0" {
 					tags["trap_name"] = fmt.Sprintf("%v", value)
 					continue
 				}
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 333a14752e357..6196e55664e36 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -131,11 +131,11 @@ func TestReceiveTrap(t *testing.T) {
 		testutil.MustMetric(
 			"snmp_trap", //name
 			map[string]string{ //tags
-				"trap_name": "coldStart",
+				"trap_name":    "coldStart",
 				"trap_version": "2c",
 			},
 			map[string]interface{}{ //fields
-				"sysUpTimeInstance": sentTimestamp,
+				"sysUpTimeInstance":      sentTimestamp,
 				"sysUpTimeInstance_type": "TimeTicks",
 			},
 			fakeTime,

From 0f9566985d4f1ac7ad84fa4a569f6bca2623a5e9 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Mon, 11 Nov 2019 10:52:21 -0700
Subject: [PATCH 04/26] add space after //

---
 plugins/inputs/snmp_trap/snmp_trap.go      | 31 +++++++-------
 plugins/inputs/snmp_trap/snmp_trap_test.go | 48 +++++++++++-----------
 2 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index ee59b05fee1a9..6a0dff69800e3 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -62,9 +62,9 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 	s.listener = gosnmp.NewTrapListener()
 	s.listener.OnNewTrap = makeTrapHandler(s)
 	s.listener.Params = gosnmp.Default
-	//s.listener.Params.Logger = log.New(os.Stdout, "", 0)
+	// s.listener.Params.Logger = log.New(os.Stdout, "", 0)
 
-	//wrap the handler, used in unit tests
+	// wrap the handler, used in unit tests
 	if nil != s.makeHandlerWrapper {
 		s.listener.OnNewTrap = s.makeHandlerWrapper(s.listener.OnNewTrap)
 	}
@@ -73,7 +73,7 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 	go func() {
 		defer s.wg.Done()
 
-		//no ip means listen on all interfaces, ipv4 and ipv6
+		// no ip means listen on all interfaces, ipv4 and ipv6
 		err := s.listener.Listen(":" + strconv.FormatUint(uint64(s.Port), 10))
 		if err != nil {
 			s.Errch <- err
@@ -98,33 +98,34 @@ func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPA
 		tags["trap_version"] = packet.Version.String()
 
 		for _, v := range packet.Variables {
-			//build a name and value for each variable to use as tags
-			//and fields.  defaults are the uninterpreted values
+			// build a name and value for each variable to use as tags
+			// and fields.  defaults are the uninterpreted values
 			name := v.Name
 			value := v.Value
 
-			//use system mibs to resolve the name if possible
+			// use system mibs to resolve the name if possible
 			_, _, oidText, _, err := snmp.SnmpTranslate(v.Name)
 			if nil == err {
-				name = oidText //would mib name be useful here?
+				name = oidText // would mib name be useful here?
 			}
 
-			//todo: format the pdu value based on its snmp type and
-			//the mib's textual convention.  The snmp input plugin
-			//only handles textual convention for ip and mac addresses
+			// todo: format the pdu value based on its snmp type and
+			// the mib's textual convention.  The snmp input plugin
+			// only handles textual convention for ip and mac
+			// addresses
 
 			switch v.Type {
-			//case gosnmp.OctetString:
-			//b := v.Value.([]byte)
+			// case gosnmp.OctetString:
+			// b := v.Value.([]byte)
 			case gosnmp.ObjectIdentifier:
 				s, ok := v.Value.(string)
 				if ok {
 					if _, _, oidText, _, err := snmp.SnmpTranslate(s); ok && nil == err {
-						value = oidText //would mib name be useful here?
+						value = oidText // would mib name be useful here?
 					}
 				}
-				//1.3.6.1.6.3.1.1.4.1.0 is SNMPv2-MIB::snmpTrapOID.0.  If
-				//v.Name is this oid, set a tag of the trap name.
+				// 1.3.6.1.6.3.1.1.4.1.0 is SNMPv2-MIB::snmpTrapOID.0.
+				// If v.Name is this oid, set a tag of the trap name.
 				if v.Name == ".1.3.6.1.6.3.1.1.4.1.0" {
 					tags["trap_name"] = fmt.Sprintf("%v", value)
 					continue
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 6196e55664e36..68b5843101c04 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -1,13 +1,13 @@
 package snmp_trap
 
-//todo: tests that look up oids will pass only if snmptranslate (part
-//of net-snmp) is installed and working.  We need to mock name lookup
-//or add a way to disable it so tests will pass when snmptranslate
-//isn't available.
+// todo: tests that look up oids will pass only if snmptranslate (part
+// of net-snmp) is installed and working.  We need to mock name lookup
+// or add a way to disable it so tests will pass when snmptranslate
+// isn't available.
 
 import (
-	//"log"
-	//"os"
+	// "log"
+	// "os"
 	"net"
 	"testing"
 	"time"
@@ -39,7 +39,7 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 		Retries:   3,
 		MaxOids:   gosnmp.MaxOids,
 		Target:    "127.0.0.1",
-		//Logger:    log.New(os.Stdout, "", 0),
+		// Logger:    log.New(os.Stdout, "", 0),
 	}
 
 	err := s.Connect()
@@ -48,10 +48,10 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 	}
 	defer s.Conn.Close()
 
-	//If the first pdu isn't type TimeTicks, gosnmp.SendTrap() will
-	//prepend one with time.Now().  The time value is part of the
-	//plugin output so we need to keep track of it and verify it
-	//later.
+	// If the first pdu isn't type TimeTicks, gosnmp.SendTrap() will
+	// prepend one with time.Now().  The time value is part of the
+	// plugin output so we need to keep track of it and verify it
+	// later.
 	now := uint32(time.Now().Unix())
 	timePdu := gosnmp.SnmpPDU{
 		Name:  ".1.3.6.1.2.1.1.3.0",
@@ -60,9 +60,9 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 	}
 
 	pdu := gosnmp.SnmpPDU{
-		Name:  ".1.3.6.1.6.3.1.1.4.1.0", //SNMPv2-MIB::snmpTrapOID.0
+		Name:  ".1.3.6.1.6.3.1.1.4.1.0", // SNMPv2-MIB::snmpTrapOID.0
 		Type:  gosnmp.ObjectIdentifier,
-		Value: ".1.3.6.1.6.3.1.1.5.1", //coldStart
+		Value: ".1.3.6.1.6.3.1.1.5.1", // coldStart
 	}
 
 	trap := gosnmp.SnmpTrap{
@@ -80,12 +80,12 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 	return now
 }
 
-// TestReceiveTrap
+//  TestReceiveTrap
 func TestReceiveTrap(t *testing.T) {
-	const port = 12399 //todo: find unused port
+	const port = 12399 // todo: find unused port
 	var fakeTime = time.Now()
 
-	//hook into the trap handler so the test knows when the trap has been received
+	// hook into the trap handler so the test knows when the trap has been received
 	received := make(chan int)
 	wrap := func(f func(*gosnmp.SnmpPacket, *net.UDPAddr)) func(*gosnmp.SnmpPacket, *net.UDPAddr) {
 		return func(p *gosnmp.SnmpPacket, a *net.UDPAddr) {
@@ -94,7 +94,7 @@ func TestReceiveTrap(t *testing.T) {
 		}
 	}
 
-	//set up the service input plugin
+	// set up the service input plugin
 	n := &SnmpTrap{
 		Port:               port,
 		makeHandlerWrapper: wrap,
@@ -107,7 +107,7 @@ func TestReceiveTrap(t *testing.T) {
 	n.Start(&acc)
 	defer n.Stop()
 
-	//wait until input plugin is listening
+	// wait until input plugin is listening
 	select {
 	case <-n.Listening():
 	case err := <-n.Errch:
@@ -116,25 +116,25 @@ func TestReceiveTrap(t *testing.T) {
 		t.Fatal("timed out waiting to listen")
 	}
 
-	//send the trap
+	// send the trap
 	sentTimestamp := sendTrap(t, port)
 
-	//wait for trap to be received
+	// wait for trap to be received
 	select {
 	case <-received:
 	case <-time.After(2 * time.Second):
 		t.Fatal("timed out waiting for trap to be received")
 	}
 
-	//verify plugin output
+	// verify plugin output
 	expected := []telegraf.Metric{
 		testutil.MustMetric(
-			"snmp_trap", //name
-			map[string]string{ //tags
+			"snmp_trap", // name
+			map[string]string{ // tags
 				"trap_name":    "coldStart",
 				"trap_version": "2c",
 			},
-			map[string]interface{}{ //fields
+			map[string]interface{}{ // fields
 				"sysUpTimeInstance":      sentTimestamp,
 				"sysUpTimeInstance_type": "TimeTicks",
 			},

From 0faad8095eb84da64c4f9995bb4f212e3386d9ee Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Mon, 11 Nov 2019 11:04:14 -0700
Subject: [PATCH 05/26] log error instead of panic

---
 plugins/inputs/snmp_trap/snmp_trap.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 6a0dff69800e3..3f527fb1da565 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -2,7 +2,6 @@ package snmp_trap
 
 import (
 	"fmt"
-	"log"
 	"net"
 	"strconv"
 	"sync"
@@ -25,6 +24,7 @@ type SnmpTrap struct {
 
 	makeHandlerWrapper func(func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)) func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)
 	Errch              chan error
+	Log telegraf.Logger
 }
 
 var sampleConfig = `
@@ -77,7 +77,7 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 		err := s.listener.Listen(":" + strconv.FormatUint(uint64(s.Port), 10))
 		if err != nil {
 			s.Errch <- err
-			log.Panicf("error in listen: %s", err)
+			s.Log.Errorf("error in listen: %s", err)
 		}
 	}()
 

From b0581794af8f67e53f35b556f129be74f58b51a6 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Mon, 11 Nov 2019 11:25:10 -0700
Subject: [PATCH 06/26] change port setting to service_address to allow user to
 specify which interfaces to listen on

---
 plugins/inputs/snmp_trap/README.md         |  5 +++--
 plugins/inputs/snmp_trap/snmp_trap.go      | 16 ++++++++--------
 plugins/inputs/snmp_trap/snmp_trap_test.go |  3 ++-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/plugins/inputs/snmp_trap/README.md b/plugins/inputs/snmp_trap/README.md
index 40f28a1c5e809..397d3f40675bc 100644
--- a/plugins/inputs/snmp_trap/README.md
+++ b/plugins/inputs/snmp_trap/README.md
@@ -12,8 +12,9 @@ the SNMP [README.md](../snmp/README.md) for details.
 
 ### Configuration
 ```toml
-  ## Port to listen on.  Default 162
-  #port = 162
+  ## Local address and port to listen on.  Omit address to listen on
+  ## all interfaces.  Example "127.0.0.1:1234", default ":162"
+  #service_address = :162
 ```
 
 ### Metrics
diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 3f527fb1da565..47b86006035c2 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -3,7 +3,6 @@ package snmp_trap
 import (
 	"fmt"
 	"net"
-	"strconv"
 	"sync"
 	"time"
 
@@ -15,7 +14,7 @@ import (
 )
 
 type SnmpTrap struct {
-	Port uint16 `toml:"port"`
+	ServiceAddress string `toml:"service_address"`
 
 	acc      telegraf.Accumulator
 	listener *gosnmp.TrapListener
@@ -24,12 +23,13 @@ type SnmpTrap struct {
 
 	makeHandlerWrapper func(func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)) func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)
 	Errch              chan error
-	Log telegraf.Logger
+	Log                telegraf.Logger
 }
 
 var sampleConfig = `
-  ## Port to listen on.  Default 162
-  #port = 162
+  ## Local address and port to listen on.  Omit address to listen on
+  ## all interfaces.  Example "127.0.0.1:1234", default ":162"
+  #service_address = :162
 `
 
 func (s *SnmpTrap) SampleConfig() string {
@@ -47,8 +47,8 @@ func (s *SnmpTrap) Gather(_ telegraf.Accumulator) error {
 func init() {
 	inputs.Add("snmp_trap", func() telegraf.Input {
 		return &SnmpTrap{
-			timeFunc: time.Now,
-			Port:     162,
+			timeFunc:       time.Now,
+			ServiceAddress: ":162",
 		}
 	})
 }
@@ -74,7 +74,7 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 		defer s.wg.Done()
 
 		// no ip means listen on all interfaces, ipv4 and ipv6
-		err := s.listener.Listen(":" + strconv.FormatUint(uint64(s.Port), 10))
+		err := s.listener.Listen(s.ServiceAddress)
 		if err != nil {
 			s.Errch <- err
 			s.Log.Errorf("error in listen: %s", err)
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 68b5843101c04..5fca232fbcec5 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -9,6 +9,7 @@ import (
 	// "log"
 	// "os"
 	"net"
+	"strconv"
 	"testing"
 	"time"
 
@@ -96,7 +97,7 @@ func TestReceiveTrap(t *testing.T) {
 
 	// set up the service input plugin
 	n := &SnmpTrap{
-		Port:               port,
+		ServiceAddress: "localhost:" + strconv.Itoa(port),
 		makeHandlerWrapper: wrap,
 		timeFunc: func() time.Time {
 			return fakeTime

From 9b8e72d00b58bb069afd2a1047c5d1c9302ebd9d Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Tue, 12 Nov 2019 10:25:02 -0700
Subject: [PATCH 07/26] add trap_oid, trap_mib, source tags

---
 plugins/inputs/snmp_trap/README.md         |  3 +++
 plugins/inputs/snmp_trap/snmp_trap.go      | 16 ++++++++++++----
 plugins/inputs/snmp_trap/snmp_trap_test.go |  5 ++++-
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/plugins/inputs/snmp_trap/README.md b/plugins/inputs/snmp_trap/README.md
index 397d3f40675bc..fec59e7d4b13f 100644
--- a/plugins/inputs/snmp_trap/README.md
+++ b/plugins/inputs/snmp_trap/README.md
@@ -21,7 +21,10 @@ the SNMP [README.md](../snmp/README.md) for details.
 
 - snmp_trap
   - tags:
+	- source (string, IP address of trap source)
 	- trap_name (string, value from SNMPv2-MIB::snmpTrapOID.0 PDU)
+	- trap_mib (string, mib from SNMPv2-MIB::snmpTrapOID.0 PDU)
+	- trap_oid (string, oid string from SNMPv2-MIB::snmpTrapOID.0 PDU)
 	- trap_version (string, "1" or "2c" or "3")
   - fields:
 	- $NAME (the type is variable and depends on the PDU)
diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 47b86006035c2..4e2418537bfe6 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -1,7 +1,6 @@
 package snmp_trap
 
 import (
-	"fmt"
 	"net"
 	"sync"
 	"time"
@@ -96,6 +95,7 @@ func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPA
 		tags := map[string]string{}
 
 		tags["trap_version"] = packet.Version.String()
+		tags["source"] = addr.IP.String()
 
 		for _, v := range packet.Variables {
 			// build a name and value for each variable to use as tags
@@ -119,15 +119,23 @@ func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPA
 			// b := v.Value.([]byte)
 			case gosnmp.ObjectIdentifier:
 				s, ok := v.Value.(string)
+				var mibName string
+				var oidText string
+				var err error
 				if ok {
-					if _, _, oidText, _, err := snmp.SnmpTranslate(s); ok && nil == err {
-						value = oidText // would mib name be useful here?
+					mibName, _, oidText, _, err = snmp.SnmpTranslate(s)
+					if nil == err {
+						value = oidText
 					}
 				}
 				// 1.3.6.1.6.3.1.1.4.1.0 is SNMPv2-MIB::snmpTrapOID.0.
 				// If v.Name is this oid, set a tag of the trap name.
 				if v.Name == ".1.3.6.1.6.3.1.1.4.1.0" {
-					tags["trap_name"] = fmt.Sprintf("%v", value)
+					tags["trap_oid"] = s
+					if err == nil {
+						tags["trap_name"] = oidText
+						tags["trap_mib"] = mibName
+					}
 					continue
 				}
 			}
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 5fca232fbcec5..89e65be2b8bc3 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -97,7 +97,7 @@ func TestReceiveTrap(t *testing.T) {
 
 	// set up the service input plugin
 	n := &SnmpTrap{
-		ServiceAddress: "localhost:" + strconv.Itoa(port),
+		ServiceAddress:     "localhost:" + strconv.Itoa(port),
 		makeHandlerWrapper: wrap,
 		timeFunc: func() time.Time {
 			return fakeTime
@@ -132,8 +132,11 @@ func TestReceiveTrap(t *testing.T) {
 		testutil.MustMetric(
 			"snmp_trap", // name
 			map[string]string{ // tags
+				"trap_oid":     ".1.3.6.1.6.3.1.1.5.1",
 				"trap_name":    "coldStart",
+				"trap_mib":     "SNMPv2-MIB",
 				"trap_version": "2c",
+				"source":       "127.0.0.1",
 			},
 			map[string]interface{}{ // fields
 				"sysUpTimeInstance":      sentTimestamp,

From c7f22e3f9df500c973486d3d74bbbd902c0a00a6 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Tue, 12 Nov 2019 10:42:34 -0700
Subject: [PATCH 08/26] don't return from Start() until gosnmp TrapListener is
 ready

---
 plugins/inputs/snmp_trap/snmp_trap.go      | 9 +++------
 plugins/inputs/snmp_trap/snmp_trap_test.go | 9 ---------
 2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 4e2418537bfe6..e871182428718 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -21,7 +21,7 @@ type SnmpTrap struct {
 	timeFunc func() time.Time
 
 	makeHandlerWrapper func(func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)) func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)
-	Errch              chan error
+
 	Log                telegraf.Logger
 }
 
@@ -75,11 +75,12 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 		// no ip means listen on all interfaces, ipv4 and ipv6
 		err := s.listener.Listen(s.ServiceAddress)
 		if err != nil {
-			s.Errch <- err
 			s.Log.Errorf("error in listen: %s", err)
 		}
 	}()
 
+	<-s.listener.Listening()
+
 	return nil
 }
 
@@ -147,7 +148,3 @@ func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPA
 		s.acc.AddFields("snmp_trap", fields, tags, tm)
 	}
 }
-
-func (s *SnmpTrap) Listening() <-chan bool {
-	return s.listener.Listening()
-}
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 89e65be2b8bc3..688ff560a513e 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -108,15 +108,6 @@ func TestReceiveTrap(t *testing.T) {
 	n.Start(&acc)
 	defer n.Stop()
 
-	// wait until input plugin is listening
-	select {
-	case <-n.Listening():
-	case err := <-n.Errch:
-		t.Fatalf("error in listen: %v", err)
-	case <-time.After(2 * time.Second):
-		t.Fatal("timed out waiting to listen")
-	}
-
 	// send the trap
 	sentTimestamp := sendTrap(t, port)
 

From d7fa50d32856b5cb6cfbb9aef07c13cf62b81f02 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Tue, 12 Nov 2019 12:01:00 -0700
Subject: [PATCH 09/26] clean up comments

---
 plugins/inputs/snmp_trap/snmp_trap.go      |  3 ---
 plugins/inputs/snmp_trap/snmp_trap_test.go | 13 ++++++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index e871182428718..555fd8a4962a4 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -61,7 +61,6 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 	s.listener = gosnmp.NewTrapListener()
 	s.listener.OnNewTrap = makeTrapHandler(s)
 	s.listener.Params = gosnmp.Default
-	// s.listener.Params.Logger = log.New(os.Stdout, "", 0)
 
 	// wrap the handler, used in unit tests
 	if nil != s.makeHandlerWrapper {
@@ -116,8 +115,6 @@ func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPA
 			// addresses
 
 			switch v.Type {
-			// case gosnmp.OctetString:
-			// b := v.Value.([]byte)
 			case gosnmp.ObjectIdentifier:
 				s, ok := v.Value.(string)
 				var mibName string
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 688ff560a513e..f2f2dfe503560 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -6,8 +6,6 @@ package snmp_trap
 // isn't available.
 
 import (
-	// "log"
-	// "os"
 	"net"
 	"strconv"
 	"testing"
@@ -40,7 +38,6 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 		Retries:   3,
 		MaxOids:   gosnmp.MaxOids,
 		Target:    "127.0.0.1",
-		// Logger:    log.New(os.Stdout, "", 0),
 	}
 
 	err := s.Connect()
@@ -83,10 +80,16 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 
 //  TestReceiveTrap
 func TestReceiveTrap(t *testing.T) {
-	const port = 12399 // todo: find unused port
+
+	// We would prefer to specify port 0 and let the network stack
+	// choose an unused port for us but TrapListener doesn't have a
+	// way to return the autoselected port.  Instead, we'll use an
+	// unusual port and hope it's unused.
+	const port = 12399
 	var fakeTime = time.Now()
 
-	// hook into the trap handler so the test knows when the trap has been received
+	// hook into the trap handler so the test knows when the trap has
+	// been received
 	received := make(chan int)
 	wrap := func(f func(*gosnmp.SnmpPacket, *net.UDPAddr)) func(*gosnmp.SnmpPacket, *net.UDPAddr) {
 		return func(p *gosnmp.SnmpPacket, a *net.UDPAddr) {

From 35cc6c314813f01667b1f9eceb4a69d46e10a8d4 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Tue, 12 Nov 2019 12:15:45 -0700
Subject: [PATCH 10/26] update readme example output

---
 plugins/inputs/snmp_trap/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/inputs/snmp_trap/README.md b/plugins/inputs/snmp_trap/README.md
index fec59e7d4b13f..c10dfda81d0af 100644
--- a/plugins/inputs/snmp_trap/README.md
+++ b/plugins/inputs/snmp_trap/README.md
@@ -32,6 +32,6 @@ the SNMP [README.md](../snmp/README.md) for details.
 
 ### Example Output
 ```
-> snmp_trap,host=debian,trap_name=coldStart,trap_version=2c sysUpTimeInstance_type="TimeTicks",snmpTrapEnterprise.0="linux",snmpTrapEnterprise.0_type="ObjectIdentifier",sysUpTimeInstance=1i 1573078928344595213
-> snmp_trap,host=debian,trap_name=nsNotifyShutdown,trap_version=2c sysUpTimeInstance=1224i,sysUpTimeInstance_type="TimeTicks",snmpTrapEnterprise.0="netSnmpNotificationPrefix",snmpTrapEnterprise.0_type="ObjectIdentifier" 1573078928299642679
+snmp_trap,source=192.168.122.102,trap_mib=SNMPv2-MIB,trap_name=coldStart,trap_oid=.1.3.6.1.6.3.1.1.5.1,trap_version=2c sysUpTimeInstance=0i,sysUpTimeInstance_type="TimeTicks",snmpTrapEnterprise.0="linux",snmpTrapEnterprise.0_type="ObjectIdentifier" 1573586012665359513
+snmp_trap,source=192.168.122.102,trap_mib=NET-SNMP-AGENT-MIB,trap_name=nsNotifyShutdown,trap_oid=.1.3.6.1.4.1.8072.4.0.2,trap_version=2c sysUpTimeInstance=2196i,sysUpTimeInstance_type="TimeTicks",snmpTrapEnterprise.0="netSnmpNotificationPrefix",snmpTrapEnterprise.0_type="ObjectIdentifier" 1573586012076284951
 ```

From 38a1ffd7e02486cab505651d11dc7f7e54387f13 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Tue, 12 Nov 2019 16:21:45 -0700
Subject: [PATCH 11/26] preload oid cache in unit test so snmptranslate doesn't
 need to be installed

---
 plugins/inputs/snmp/snmp.go                | 22 ++++++++++++
 plugins/inputs/snmp_trap/snmp_trap.go      |  2 +-
 plugins/inputs/snmp_trap/snmp_trap_test.go | 39 ++++++++++++++++++----
 3 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/plugins/inputs/snmp/snmp.go b/plugins/inputs/snmp/snmp.go
index bff48fb6eb4ee..275314053f26b 100644
--- a/plugins/inputs/snmp/snmp.go
+++ b/plugins/inputs/snmp/snmp.go
@@ -974,6 +974,28 @@ func SnmpTranslate(oid string) (mibName string, oidNum string, oidText string, c
 	return stc.mibName, stc.oidNum, stc.oidText, stc.conversion, stc.err
 }
 
+func SnmpTranslateForce(oid string, mibName string, oidNum string, oidText string, conversion string) {
+	snmpTranslateCachesLock.Lock()
+	defer snmpTranslateCachesLock.Unlock()
+	if snmpTranslateCaches == nil {
+		snmpTranslateCaches = map[string]snmpTranslateCache{}
+	}
+
+	var stc snmpTranslateCache
+	stc.mibName = mibName
+	stc.oidNum = oidNum
+	stc.oidText = oidText
+	stc.conversion = conversion
+	stc.err = nil
+	snmpTranslateCaches[oid] = stc
+}
+
+func SnmpTranslateClear() {
+	snmpTranslateCachesLock.Lock()
+	defer snmpTranslateCachesLock.Unlock()
+	snmpTranslateCaches = map[string]snmpTranslateCache{}
+}
+
 func snmpTranslateCall(oid string) (mibName string, oidNum string, oidText string, conversion string, err error) {
 	var out []byte
 	if strings.ContainsAny(oid, ":abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") {
diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 555fd8a4962a4..f008b832140ae 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -106,7 +106,7 @@ func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPA
 			// use system mibs to resolve the name if possible
 			_, _, oidText, _, err := snmp.SnmpTranslate(v.Name)
 			if nil == err {
-				name = oidText // would mib name be useful here?
+				name = oidText
 			}
 
 			// todo: format the pdu value based on its snmp type and
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index f2f2dfe503560..213f0822c66cd 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -1,10 +1,5 @@
 package snmp_trap
 
-// todo: tests that look up oids will pass only if snmptranslate (part
-// of net-snmp) is installed and working.  We need to mock name lookup
-// or add a way to disable it so tests will pass when snmptranslate
-// isn't available.
-
 import (
 	"net"
 	"strconv"
@@ -21,6 +16,15 @@ import (
 )
 
 func TestTranslate(t *testing.T) {
+	defer snmp.SnmpTranslateClear()
+	snmp.SnmpTranslateForce(
+		".1.3.6.1.6.3.1.1.5.1",
+		"SNMPv2-MIB",
+		".1.3.6.1.6.3.1.1.5.1",
+		"coldStart",
+		"",
+	)
+
 	mibName, oidNum, oidText, conversion, err := snmp.SnmpTranslate(".1.3.6.1.6.3.1.1.5.1")
 	require.NoError(t, err)
 	require.Equal(t, "SNMPv2-MIB", mibName)
@@ -78,8 +82,31 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 	return now
 }
 
-//  TestReceiveTrap
 func TestReceiveTrap(t *testing.T) {
+	// Preload the cache with the oids we'll use in this test so
+	// snmptranslate and mibs don't need to be installed.
+	defer snmp.SnmpTranslateClear()
+	snmp.SnmpTranslateForce(
+		".1.3.6.1.6.3.1.1.4.1.0",
+		"SNMPv2-MIB",
+		".1.3.6.1.6.3.1.1.4.1.0",
+		"snmpTrapOID.0",
+		"",
+	)
+	snmp.SnmpTranslateForce(
+		".1.3.6.1.6.3.1.1.5.1",
+		"SNMPv2-MIB",
+		".1.3.6.1.6.3.1.1.5.1",
+		"coldStart",
+		"",
+	)
+	snmp.SnmpTranslateForce(
+		".1.3.6.1.2.1.1.3.0",
+		"UNUSED_MIB_NAME",
+		".1.3.6.1.2.1.1.3.0",
+		"sysUpTimeInstance",
+		"",
+	)
 
 	// We would prefer to specify port 0 and let the network stack
 	// choose an unused port for us but TrapListener doesn't have a

From 4353fb37c5199b7413e53d3550fb24840267ee20 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Tue, 12 Nov 2019 16:22:59 -0700
Subject: [PATCH 12/26] gofmt

---
 plugins/inputs/snmp_trap/snmp_trap.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index f008b832140ae..29fa994441bb8 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -22,7 +22,7 @@ type SnmpTrap struct {
 
 	makeHandlerWrapper func(func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)) func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)
 
-	Log                telegraf.Logger
+	Log telegraf.Logger
 }
 
 var sampleConfig = `

From 789c7b1d55e6f68c0732a195cd09b9894a60d937 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Thu, 14 Nov 2019 09:42:18 -0700
Subject: [PATCH 13/26] make handler function type to simplify definition of
 makeWrapperHandler

---
 plugins/inputs/snmp_trap/snmp_trap.go | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 29fa994441bb8..5ca288eaa74e0 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -12,6 +12,8 @@ import (
 	"github.com/soniah/gosnmp"
 )
 
+type handler func(*gosnmp.SnmpPacket, *net.UDPAddr)
+
 type SnmpTrap struct {
 	ServiceAddress string `toml:"service_address"`
 
@@ -20,7 +22,7 @@ type SnmpTrap struct {
 	wg       sync.WaitGroup
 	timeFunc func() time.Time
 
-	makeHandlerWrapper func(func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)) func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr)
+	makeHandlerWrapper func(handler) handler
 
 	Log telegraf.Logger
 }
@@ -88,7 +90,7 @@ func (s *SnmpTrap) Stop() {
 	s.wg.Wait()
 }
 
-func makeTrapHandler(s *SnmpTrap) func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr) {
+func makeTrapHandler(s *SnmpTrap) handler {
 	return func(packet *gosnmp.SnmpPacket, addr *net.UDPAddr) {
 		tm := s.timeFunc()
 		fields := map[string]interface{}{}

From 1dd91579996b4b9a479258ff5396a2ba8e4356e3 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Thu, 14 Nov 2019 09:45:58 -0700
Subject: [PATCH 14/26] make Log unsettable from toml

---
 plugins/inputs/snmp_trap/snmp_trap.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 5ca288eaa74e0..9f8ad6a001e0c 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -24,7 +24,7 @@ type SnmpTrap struct {
 
 	makeHandlerWrapper func(handler) handler
 
-	Log telegraf.Logger
+	Log telegraf.Logger `toml:"-"`
 }
 
 var sampleConfig = `

From a7188729de2c47bd105b5098d742af6090cce0b0 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Thu, 14 Nov 2019 09:47:32 -0700
Subject: [PATCH 15/26] make handler function type to simplify definition of
 makeWrapperHandler

---
 plugins/inputs/snmp_trap/snmp_trap_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 213f0822c66cd..3ddcf74ebe015 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -118,7 +118,7 @@ func TestReceiveTrap(t *testing.T) {
 	// hook into the trap handler so the test knows when the trap has
 	// been received
 	received := make(chan int)
-	wrap := func(f func(*gosnmp.SnmpPacket, *net.UDPAddr)) func(*gosnmp.SnmpPacket, *net.UDPAddr) {
+	wrap := func(f handler) handler {
 		return func(p *gosnmp.SnmpPacket, a *net.UDPAddr) {
 			f(p, a)
 			received <- 0

From 5ba2f5542c30a8224268b72704d8a3746431ed6f Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Fri, 15 Nov 2019 14:16:00 -0700
Subject: [PATCH 16/26] require udp:// on service address.  Log when listening

---
 plugins/inputs/snmp_trap/README.md         |  9 +++++---
 plugins/inputs/snmp_trap/snmp_trap.go      | 26 ++++++++++++++++++----
 plugins/inputs/snmp_trap/snmp_trap_test.go |  7 +++---
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/plugins/inputs/snmp_trap/README.md b/plugins/inputs/snmp_trap/README.md
index c10dfda81d0af..e2c7f93f1f6de 100644
--- a/plugins/inputs/snmp_trap/README.md
+++ b/plugins/inputs/snmp_trap/README.md
@@ -12,9 +12,12 @@ the SNMP [README.md](../snmp/README.md) for details.
 
 ### Configuration
 ```toml
-  ## Local address and port to listen on.  Omit address to listen on
-  ## all interfaces.  Example "127.0.0.1:1234", default ":162"
-  #service_address = :162
+# Snmp trap listener
+[[inputs.snmp_trap]]
+  ## Transport, local address, and port to listen on.  Transport must
+  ## be "udp://".  Omit local address to listen on all interfaces.
+  ## Example "udp://127.0.0.1:1234", default "udp://:162"
+  #service_address = udp://:162
 ```
 
 ### Metrics
diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 9f8ad6a001e0c..0548f58bceefd 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -1,7 +1,9 @@
 package snmp_trap
 
 import (
+	"fmt"
 	"net"
+	"strings"
 	"sync"
 	"time"
 
@@ -28,9 +30,10 @@ type SnmpTrap struct {
 }
 
 var sampleConfig = `
-  ## Local address and port to listen on.  Omit address to listen on
-  ## all interfaces.  Example "127.0.0.1:1234", default ":162"
-  #service_address = :162
+  ## Transport, local address, and port to listen on.  Transport must
+  ## be "udp://".  Omit local address to listen on all interfaces.
+  ## Example "udp://127.0.0.1:1234", default "udp://:162"
+  #service_address = udp://:162
 `
 
 func (s *SnmpTrap) SampleConfig() string {
@@ -69,18 +72,33 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 		s.listener.OnNewTrap = s.makeHandlerWrapper(s.listener.OnNewTrap)
 	}
 
+	split := strings.SplitN(s.ServiceAddress, "://", 2)
+	if len(split) != 2 {
+		return fmt.Errorf("invalid service address: %s", s.ServiceAddress)
+	}
+
+	protocol := split[0]
+	addr := split[1]
+
+	// gosnmp.TrapListener currentl supports udp only.  For forward
+	// compatibility, require udp in the service address
+	if protocol != "udp" {
+		return fmt.Errorf("unknown protocol '%s' in '%s'", protocol, s.ServiceAddress)
+	}
+
 	s.wg.Add(1)
 	go func() {
 		defer s.wg.Done()
 
 		// no ip means listen on all interfaces, ipv4 and ipv6
-		err := s.listener.Listen(s.ServiceAddress)
+		err := s.listener.Listen(addr)
 		if err != nil {
 			s.Log.Errorf("error in listen: %s", err)
 		}
 	}()
 
 	<-s.listener.Listening()
+	s.Log.Infof("Listening on %s", s.ServiceAddress)
 
 	return nil
 }
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 3ddcf74ebe015..82fe5905ff873 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -127,15 +127,16 @@ func TestReceiveTrap(t *testing.T) {
 
 	// set up the service input plugin
 	n := &SnmpTrap{
-		ServiceAddress:     "localhost:" + strconv.Itoa(port),
+		ServiceAddress:     "udp://:" + strconv.Itoa(port),
 		makeHandlerWrapper: wrap,
 		timeFunc: func() time.Time {
 			return fakeTime
 		},
+		Log: testutil.Logger{},
 	}
-	n.Init()
+	require.Nil(t, n.Init())
 	var acc testutil.Accumulator
-	n.Start(&acc)
+	require.Nil(t, n.Start(&acc))
 	defer n.Stop()
 
 	// send the trap

From dadcaa000c6bc024a3ecaf16ceb4c6470b3b08d4 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Fri, 15 Nov 2019 14:25:41 -0700
Subject: [PATCH 17/26] remove trap_ prefix from tags.  Don't add field for
 snmp type.

---
 plugins/inputs/snmp_trap/snmp_trap.go      |  9 ++++-----
 plugins/inputs/snmp_trap/snmp_trap_test.go | 13 ++++++-------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 0548f58bceefd..d570edca2e083 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -114,7 +114,7 @@ func makeTrapHandler(s *SnmpTrap) handler {
 		fields := map[string]interface{}{}
 		tags := map[string]string{}
 
-		tags["trap_version"] = packet.Version.String()
+		tags["version"] = packet.Version.String()
 		tags["source"] = addr.IP.String()
 
 		for _, v := range packet.Variables {
@@ -149,17 +149,16 @@ func makeTrapHandler(s *SnmpTrap) handler {
 				// 1.3.6.1.6.3.1.1.4.1.0 is SNMPv2-MIB::snmpTrapOID.0.
 				// If v.Name is this oid, set a tag of the trap name.
 				if v.Name == ".1.3.6.1.6.3.1.1.4.1.0" {
-					tags["trap_oid"] = s
+					tags["oid"] = s
 					if err == nil {
-						tags["trap_name"] = oidText
-						tags["trap_mib"] = mibName
+						tags["name"] = oidText
+						tags["mib"] = mibName
 					}
 					continue
 				}
 			}
 
 			fields[name] = value
-			fields[name+"_type"] = v.Type.String()
 		}
 
 		s.acc.AddFields("snmp_trap", fields, tags, tm)
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 82fe5905ff873..ed2480c0cafb9 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -154,15 +154,14 @@ func TestReceiveTrap(t *testing.T) {
 		testutil.MustMetric(
 			"snmp_trap", // name
 			map[string]string{ // tags
-				"trap_oid":     ".1.3.6.1.6.3.1.1.5.1",
-				"trap_name":    "coldStart",
-				"trap_mib":     "SNMPv2-MIB",
-				"trap_version": "2c",
-				"source":       "127.0.0.1",
+				"oid":     ".1.3.6.1.6.3.1.1.5.1",
+				"name":    "coldStart",
+				"mib":     "SNMPv2-MIB",
+				"version": "2c",
+				"source":  "127.0.0.1",
 			},
 			map[string]interface{}{ // fields
-				"sysUpTimeInstance":      sentTimestamp,
-				"sysUpTimeInstance_type": "TimeTicks",
+				"sysUpTimeInstance": sentTimestamp,
 			},
 			fakeTime,
 		),

From cfdeaad5a370b8e85349d8a72b9572ed1490d1e5 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Mon, 18 Nov 2019 11:26:02 -0700
Subject: [PATCH 18/26] If trap listening returns an error, return it from
 Start

---
 plugins/inputs/snmp_trap/snmp_trap.go | 29 ++++++++++++++-------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index d570edca2e083..6512c5d160558 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"net"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/influxdata/telegraf"
@@ -21,8 +20,8 @@ type SnmpTrap struct {
 
 	acc      telegraf.Accumulator
 	listener *gosnmp.TrapListener
-	wg       sync.WaitGroup
 	timeFunc func() time.Time
+	errCh    chan error
 
 	makeHandlerWrapper func(handler) handler
 
@@ -52,7 +51,7 @@ func init() {
 	inputs.Add("snmp_trap", func() telegraf.Input {
 		return &SnmpTrap{
 			timeFunc:       time.Now,
-			ServiceAddress: ":162",
+			ServiceAddress: "udp://:162",
 		}
 	})
 }
@@ -86,26 +85,28 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 		return fmt.Errorf("unknown protocol '%s' in '%s'", protocol, s.ServiceAddress)
 	}
 
-	s.wg.Add(1)
+	// If (*TrapListener).Listen immediately returns an error we need
+	// to return it from this function.  Use a channel to get it here
+	// from the goroutine.  Buffer one in case Listen returns after
+	// Listening but before our Close is called.
+	s.errCh = make(chan error, 1)
 	go func() {
-		defer s.wg.Done()
-
-		// no ip means listen on all interfaces, ipv4 and ipv6
-		err := s.listener.Listen(addr)
-		if err != nil {
-			s.Log.Errorf("error in listen: %s", err)
-		}
+		s.errCh <- s.listener.Listen(addr)
 	}()
 
-	<-s.listener.Listening()
-	s.Log.Infof("Listening on %s", s.ServiceAddress)
+	select {
+	case <-s.listener.Listening():
+		s.Log.Infof("Listening on %s", s.ServiceAddress)
+	case err := <-s.errCh:
+		return err
+	}
 
 	return nil
 }
 
 func (s *SnmpTrap) Stop() {
 	s.listener.Close()
-	s.wg.Wait()
+	_ = <-s.errCh
 }
 
 func makeTrapHandler(s *SnmpTrap) handler {

From 3f12b3212e0d59982f41bc768e3f2f21dc5887b3 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Mon, 18 Nov 2019 13:39:53 -0700
Subject: [PATCH 19/26] update example output

---
 plugins/inputs/snmp_trap/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/inputs/snmp_trap/README.md b/plugins/inputs/snmp_trap/README.md
index e2c7f93f1f6de..52a349eceffea 100644
--- a/plugins/inputs/snmp_trap/README.md
+++ b/plugins/inputs/snmp_trap/README.md
@@ -35,6 +35,6 @@ the SNMP [README.md](../snmp/README.md) for details.
 
 ### Example Output
 ```
-snmp_trap,source=192.168.122.102,trap_mib=SNMPv2-MIB,trap_name=coldStart,trap_oid=.1.3.6.1.6.3.1.1.5.1,trap_version=2c sysUpTimeInstance=0i,sysUpTimeInstance_type="TimeTicks",snmpTrapEnterprise.0="linux",snmpTrapEnterprise.0_type="ObjectIdentifier" 1573586012665359513
-snmp_trap,source=192.168.122.102,trap_mib=NET-SNMP-AGENT-MIB,trap_name=nsNotifyShutdown,trap_oid=.1.3.6.1.4.1.8072.4.0.2,trap_version=2c sysUpTimeInstance=2196i,sysUpTimeInstance_type="TimeTicks",snmpTrapEnterprise.0="netSnmpNotificationPrefix",snmpTrapEnterprise.0_type="ObjectIdentifier" 1573586012076284951
+snmp_trap,mib=SNMPv2-MIB,name=coldStart,oid=.1.3.6.1.6.3.1.1.5.1,source=192.168.122.102,version=2c snmpTrapEnterprise.0="linux",sysUpTimeInstance=1i 1574109187723429814
+snmp_trap,mib=NET-SNMP-AGENT-MIB,name=nsNotifyShutdown,oid=.1.3.6.1.4.1.8072.4.0.2,source=192.168.122.102,version=2c sysUpTimeInstance=5803i,snmpTrapEnterprise.0="netSnmpNotificationPrefix" 1574109186555115459
 ```

From eff649379d5bc65d9c983801c460c58a48bd2334 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Wed, 20 Nov 2019 15:26:19 -0700
Subject: [PATCH 20/26] log an error and skip the trap if we can't resolve an
 oid name

---
 plugins/inputs/snmp_trap/snmp_trap.go | 51 ++++++++++++++++-----------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 6512c5d160558..2485ae3fd1786 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -119,16 +119,12 @@ func makeTrapHandler(s *SnmpTrap) handler {
 		tags["source"] = addr.IP.String()
 
 		for _, v := range packet.Variables {
-			// build a name and value for each variable to use as tags
-			// and fields.  defaults are the uninterpreted values
-			name := v.Name
-			value := v.Value
+			// Use system mibs to resolve oids.  Don't fall back to
+			// numeric oid because it's not useful enough to the end
+			// user and can be difficult to translate or remove from
+			// the database later.
 
-			// use system mibs to resolve the name if possible
-			_, _, oidText, _, err := snmp.SnmpTranslate(v.Name)
-			if nil == err {
-				name = oidText
-			}
+			var value interface{}
 
 			// todo: format the pdu value based on its snmp type and
 			// the mib's textual convention.  The snmp input plugin
@@ -137,28 +133,43 @@ func makeTrapHandler(s *SnmpTrap) handler {
 
 			switch v.Type {
 			case gosnmp.ObjectIdentifier:
-				s, ok := v.Value.(string)
+				val, ok := v.Value.(string)
 				var mibName string
 				var oidText string
 				var err error
-				if ok {
-					mibName, _, oidText, _, err = snmp.SnmpTranslate(s)
-					if nil == err {
-						value = oidText
-					}
+				if ! ok {
+					s.Log.Errorf("Error getting value OID: %v", err)
+					return
 				}
+
+				mibName, _, oidText, _, err = snmp.SnmpTranslate(val)
+				if nil != err {
+					s.Log.Errorf("Error resolving value OID: %v", err)
+					return
+				}
+
+				value = oidText
+
 				// 1.3.6.1.6.3.1.1.4.1.0 is SNMPv2-MIB::snmpTrapOID.0.
 				// If v.Name is this oid, set a tag of the trap name.
 				if v.Name == ".1.3.6.1.6.3.1.1.4.1.0" {
-					tags["oid"] = s
-					if err == nil {
-						tags["name"] = oidText
-						tags["mib"] = mibName
-					}
+					tags["oid"] = val
+					tags["name"] = oidText
+					tags["mib"] = mibName
 					continue
 				}
+			default:
+				value = v.Value
 			}
 
+			_, _, oidText, _, err := snmp.SnmpTranslate(v.Name)
+			if nil != err {
+				s.Log.Errorf("Error resolving OID: %v", err)
+				return
+			}
+
+			name := oidText
+
 			fields[name] = value
 		}
 

From 3760a7813e9eaf53b54e84afa8a9fa1ec6c4545d Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Wed, 20 Nov 2019 17:23:53 -0700
Subject: [PATCH 21/26] don't use snmp plugin's oid lookup because it doesn't
 return errors when lookup fails

---
 plugins/inputs/snmp_trap/snmp_trap.go      |  93 +++++++++++++--
 plugins/inputs/snmp_trap/snmp_trap_test.go | 131 ++++++++++++++-------
 2 files changed, 172 insertions(+), 52 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 2485ae3fd1786..9a6fc7a236421 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -1,19 +1,28 @@
 package snmp_trap
 
 import (
+	"bufio"
+	"bytes"
 	"fmt"
 	"net"
+	"os/exec"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/plugins/inputs"
-	"github.com/influxdata/telegraf/plugins/inputs/snmp"
 
 	"github.com/soniah/gosnmp"
 )
 
 type handler func(*gosnmp.SnmpPacket, *net.UDPAddr)
+type execer func(string, ...string) ([]byte, error)
+
+type mibEntry struct {
+	mibName string
+	oidText string
+}
 
 type SnmpTrap struct {
 	ServiceAddress string `toml:"service_address"`
@@ -26,6 +35,11 @@ type SnmpTrap struct {
 	makeHandlerWrapper func(handler) handler
 
 	Log telegraf.Logger `toml:"-"`
+
+	cacheLock sync.Mutex
+	cache     map[string]mibEntry
+
+	execCmd execer
 }
 
 var sampleConfig = `
@@ -56,7 +70,13 @@ func init() {
 	})
 }
 
+func realExecCmd(arg0 string, args ...string) ([]byte, error) {
+	return exec.Command(arg0, args...).Output()
+}
+
 func (s *SnmpTrap) Init() error {
+	s.cache = map[string]mibEntry{}
+	s.execCmd = realExecCmd
 	return nil
 }
 
@@ -134,41 +154,40 @@ func makeTrapHandler(s *SnmpTrap) handler {
 			switch v.Type {
 			case gosnmp.ObjectIdentifier:
 				val, ok := v.Value.(string)
-				var mibName string
-				var oidText string
+				var e mibEntry
 				var err error
-				if ! ok {
+				if !ok {
 					s.Log.Errorf("Error getting value OID: %v", err)
 					return
 				}
 
-				mibName, _, oidText, _, err = snmp.SnmpTranslate(val)
+				e, err = s.lookup(val)
 				if nil != err {
 					s.Log.Errorf("Error resolving value OID: %v", err)
 					return
 				}
 
-				value = oidText
+				value = e.oidText
 
 				// 1.3.6.1.6.3.1.1.4.1.0 is SNMPv2-MIB::snmpTrapOID.0.
 				// If v.Name is this oid, set a tag of the trap name.
 				if v.Name == ".1.3.6.1.6.3.1.1.4.1.0" {
 					tags["oid"] = val
-					tags["name"] = oidText
-					tags["mib"] = mibName
+					tags["name"] = e.oidText
+					tags["mib"] = e.mibName
 					continue
 				}
 			default:
 				value = v.Value
 			}
 
-			_, _, oidText, _, err := snmp.SnmpTranslate(v.Name)
+			e, err := s.lookup(v.Name)
 			if nil != err {
 				s.Log.Errorf("Error resolving OID: %v", err)
 				return
 			}
 
-			name := oidText
+			name := e.oidText
 
 			fields[name] = value
 		}
@@ -176,3 +195,57 @@ func makeTrapHandler(s *SnmpTrap) handler {
 		s.acc.AddFields("snmp_trap", fields, tags, tm)
 	}
 }
+
+func (s *SnmpTrap) lookup(oid string) (e mibEntry, err error) {
+	s.cacheLock.Lock()
+	defer s.cacheLock.Unlock()
+	var ok bool
+	if e, ok = s.cache[oid]; !ok {
+		// cache miss.  exec snmptranlate
+		e, err = s.snmptranslate(oid)
+		if err == nil {
+			s.cache[oid] = e
+		}
+		return e, err
+	}
+	return e, nil
+}
+
+func (s *SnmpTrap) clear() {
+	s.cacheLock.Lock()
+	defer s.cacheLock.Unlock()
+	s.cache = map[string]mibEntry{}
+}
+
+func (s *SnmpTrap) load(oid string, e mibEntry) {
+	s.cacheLock.Lock()
+	defer s.cacheLock.Unlock()
+	s.cache[oid] = e
+}
+
+func (s *SnmpTrap) snmptranslate(oid string) (e mibEntry, err error) {
+	var out []byte
+	out, err = s.execCmd("snmptranslate", "-Td", "-Ob", "-m", "all", oid)
+
+	//var e mibEntry
+
+	if err != nil {
+		return e, err
+	}
+
+	scanner := bufio.NewScanner(bytes.NewBuffer(out))
+	ok := scanner.Scan()
+	if err = scanner.Err(); !ok && err != nil {
+		return e, err //Errorf(scanner.Err(), "getting OID text")
+	}
+
+	e.oidText = scanner.Text()
+
+	i := strings.Index(e.oidText, "::")
+	if i == -1 {
+		return e, fmt.Errorf("not found")
+	}
+	e.mibName = e.oidText[:i]
+	e.oidText = e.oidText[i+2:]
+	return e, nil
+}
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index ed2480c0cafb9..0e9a0daa4f25a 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -1,6 +1,7 @@
 package snmp_trap
 
 import (
+	"fmt"
 	"net"
 	"strconv"
 	"testing"
@@ -11,26 +12,26 @@ import (
 	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/testutil"
 
-	"github.com/influxdata/telegraf/plugins/inputs/snmp"
 	"github.com/stretchr/testify/require"
 )
 
-func TestTranslate(t *testing.T) {
-	defer snmp.SnmpTranslateClear()
-	snmp.SnmpTranslateForce(
-		".1.3.6.1.6.3.1.1.5.1",
-		"SNMPv2-MIB",
+func TestLoad(t *testing.T) {
+	s := &SnmpTrap{}
+	require.Nil(t, s.Init())
+
+	defer s.clear()
+	s.load(
 		".1.3.6.1.6.3.1.1.5.1",
-		"coldStart",
-		"",
+		mibEntry{
+			"SNMPv2-MIB",
+			"coldStart",
+		},
 	)
 
-	mibName, oidNum, oidText, conversion, err := snmp.SnmpTranslate(".1.3.6.1.6.3.1.1.5.1")
+	e, err := s.lookup(".1.3.6.1.6.3.1.1.5.1")
 	require.NoError(t, err)
-	require.Equal(t, "SNMPv2-MIB", mibName)
-	require.Equal(t, ".1.3.6.1.6.3.1.1.5.1", oidNum)
-	require.Equal(t, "coldStart", oidText)
-	require.Equal(t, "", conversion)
+	require.Equal(t, "SNMPv2-MIB", e.mibName)
+	require.Equal(t, "coldStart", e.oidText)
 }
 
 func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
@@ -83,31 +84,6 @@ func sendTrap(t *testing.T, port uint16) (sentTimestamp uint32) {
 }
 
 func TestReceiveTrap(t *testing.T) {
-	// Preload the cache with the oids we'll use in this test so
-	// snmptranslate and mibs don't need to be installed.
-	defer snmp.SnmpTranslateClear()
-	snmp.SnmpTranslateForce(
-		".1.3.6.1.6.3.1.1.4.1.0",
-		"SNMPv2-MIB",
-		".1.3.6.1.6.3.1.1.4.1.0",
-		"snmpTrapOID.0",
-		"",
-	)
-	snmp.SnmpTranslateForce(
-		".1.3.6.1.6.3.1.1.5.1",
-		"SNMPv2-MIB",
-		".1.3.6.1.6.3.1.1.5.1",
-		"coldStart",
-		"",
-	)
-	snmp.SnmpTranslateForce(
-		".1.3.6.1.2.1.1.3.0",
-		"UNUSED_MIB_NAME",
-		".1.3.6.1.2.1.1.3.0",
-		"sysUpTimeInstance",
-		"",
-	)
-
 	// We would prefer to specify port 0 and let the network stack
 	// choose an unused port for us but TrapListener doesn't have a
 	// way to return the autoselected port.  Instead, we'll use an
@@ -126,7 +102,7 @@ func TestReceiveTrap(t *testing.T) {
 	}
 
 	// set up the service input plugin
-	n := &SnmpTrap{
+	s := &SnmpTrap{
 		ServiceAddress:     "udp://:" + strconv.Itoa(port),
 		makeHandlerWrapper: wrap,
 		timeFunc: func() time.Time {
@@ -134,10 +110,29 @@ func TestReceiveTrap(t *testing.T) {
 		},
 		Log: testutil.Logger{},
 	}
-	require.Nil(t, n.Init())
+	require.Nil(t, s.Init())
 	var acc testutil.Accumulator
-	require.Nil(t, n.Start(&acc))
-	defer n.Stop()
+	require.Nil(t, s.Start(&acc))
+	defer s.Stop()
+
+	// Preload the cache with the oids we'll use in this test so
+	// snmptranslate and mibs don't need to be installed.
+	defer s.clear()
+	s.load(".1.3.6.1.6.3.1.1.4.1.0",
+		mibEntry{
+			"SNMPv2-MIB",
+			"snmpTrapOID.0",
+		})
+	s.load(".1.3.6.1.6.3.1.1.5.1",
+		mibEntry{
+			"SNMPv2-MIB",
+			"coldStart",
+		})
+	s.load(".1.3.6.1.2.1.1.3.0",
+		mibEntry{
+			"UNUSED_MIB_NAME",
+			"sysUpTimeInstance",
+		})
 
 	// send the trap
 	sentTimestamp := sendTrap(t, port)
@@ -172,3 +167,55 @@ func TestReceiveTrap(t *testing.T) {
 		testutil.SortMetrics())
 
 }
+
+func fakeExecCmd(_ string, _ ...string) ([]byte, error) {
+	return nil, fmt.Errorf("intentional failure")
+}
+
+func TestMissingOid(t *testing.T) {
+	// should fail even if snmptranslate is installed
+	const port = 12399
+	var fakeTime = time.Now()
+
+	received := make(chan int)
+	wrap := func(f handler) handler {
+		return func(p *gosnmp.SnmpPacket, a *net.UDPAddr) {
+			f(p, a)
+			received <- 0
+		}
+	}
+
+	s := &SnmpTrap{
+		ServiceAddress:     "udp://:" + strconv.Itoa(port),
+		makeHandlerWrapper: wrap,
+		timeFunc: func() time.Time {
+			return fakeTime
+		},
+		Log: testutil.Logger{},
+	}
+	require.Nil(t, s.Init())
+	var acc testutil.Accumulator
+	require.Nil(t, s.Start(&acc))
+	defer s.Stop()
+
+	// make sure the cache is empty
+	s.clear()
+
+	// don't call the real snmptranslate
+	s.execCmd = fakeExecCmd
+
+	_ = sendTrap(t, port)
+
+	select {
+	case <-received:
+	case <-time.After(2 * time.Second):
+		t.Fatal("timed out waiting for trap to be received")
+	}
+
+	// oid lookup should fail so we shouldn't get a metric
+	expected := []telegraf.Metric{}
+
+	testutil.RequireMetricsEqual(t,
+		expected, acc.GetTelegrafMetrics(),
+		testutil.SortMetrics())
+}

From 8fe0fe347d669f7decf9e086849369cf78a9350f Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Thu, 21 Nov 2019 09:14:28 -0700
Subject: [PATCH 22/26] update sample config

---
 plugins/inputs/snmp_trap/README.md    | 4 ++--
 plugins/inputs/snmp_trap/snmp_trap.go | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/plugins/inputs/snmp_trap/README.md b/plugins/inputs/snmp_trap/README.md
index 52a349eceffea..b99e3d707b28f 100644
--- a/plugins/inputs/snmp_trap/README.md
+++ b/plugins/inputs/snmp_trap/README.md
@@ -16,8 +16,8 @@ the SNMP [README.md](../snmp/README.md) for details.
 [[inputs.snmp_trap]]
   ## Transport, local address, and port to listen on.  Transport must
   ## be "udp://".  Omit local address to listen on all interfaces.
-  ## Example "udp://127.0.0.1:1234", default "udp://:162"
-  #service_address = udp://:162
+  ##   example: "udp://127.0.0.1:1234"
+  # service_address = udp://:162
 ```
 
 ### Metrics
diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 9a6fc7a236421..4225207567ca2 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -45,8 +45,8 @@ type SnmpTrap struct {
 var sampleConfig = `
   ## Transport, local address, and port to listen on.  Transport must
   ## be "udp://".  Omit local address to listen on all interfaces.
-  ## Example "udp://127.0.0.1:1234", default "udp://:162"
-  #service_address = udp://:162
+  ##   example: "udp://127.0.0.1:1234"
+  # service_address = udp://:162
 `
 
 func (s *SnmpTrap) SampleConfig() string {

From b8a25b54d594fa0a1c756c70269c66c82967a0f9 Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Fri, 22 Nov 2019 12:00:09 -0700
Subject: [PATCH 23/26] update readme's trap and field names and descriptions

---
 plugins/inputs/snmp_trap/README.md | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/plugins/inputs/snmp_trap/README.md b/plugins/inputs/snmp_trap/README.md
index b99e3d707b28f..763733dde654c 100644
--- a/plugins/inputs/snmp_trap/README.md
+++ b/plugins/inputs/snmp_trap/README.md
@@ -25,13 +25,14 @@ the SNMP [README.md](../snmp/README.md) for details.
 - snmp_trap
   - tags:
 	- source (string, IP address of trap source)
-	- trap_name (string, value from SNMPv2-MIB::snmpTrapOID.0 PDU)
-	- trap_mib (string, mib from SNMPv2-MIB::snmpTrapOID.0 PDU)
-	- trap_oid (string, oid string from SNMPv2-MIB::snmpTrapOID.0 PDU)
-	- trap_version (string, "1" or "2c" or "3")
+	- name (string, value from SNMPv2-MIB::snmpTrapOID.0 PDU)
+	- mib (string, MIB from SNMPv2-MIB::snmpTrapOID.0 PDU)
+	- oid (string, OID string from SNMPv2-MIB::snmpTrapOID.0 PDU)
+	- version (string, "1" or "2c" or "3")
   - fields:
-	- $NAME (the type is variable and depends on the PDU)
-	- $NAME_type (string, description of the Asn1BER type of the PDU.  Examples: "Integer", "TimeTicks", "IPAddress")
+	- Fields are mapped from variables in the trap. Field names are
+      the trap variable names after MIB lookup. Field values are trap
+      variable values.
 
 ### Example Output
 ```

From e9516275e7a8cb385ca279689d38265690a3e16f Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Mon, 25 Nov 2019 11:07:41 -0700
Subject: [PATCH 24/26] add timeout for running snmptranslate, remove stray
 comment

---
 plugins/inputs/snmp_trap/README.md         |  2 ++
 plugins/inputs/snmp_trap/snmp_trap.go      | 32 +++++++++++++++-------
 plugins/inputs/snmp_trap/snmp_trap_test.go |  3 +-
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/plugins/inputs/snmp_trap/README.md b/plugins/inputs/snmp_trap/README.md
index 763733dde654c..ec3c7ba4c6efa 100644
--- a/plugins/inputs/snmp_trap/README.md
+++ b/plugins/inputs/snmp_trap/README.md
@@ -18,6 +18,8 @@ the SNMP [README.md](../snmp/README.md) for details.
   ## be "udp://".  Omit local address to listen on all interfaces.
   ##   example: "udp://127.0.0.1:1234"
   # service_address = udp://:162
+  ## Timeout running snmptranslate command
+  # timeout = "5s"
 ```
 
 ### Metrics
diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index 4225207567ca2..b1f48377525cb 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -11,13 +11,16 @@ import (
 	"time"
 
 	"github.com/influxdata/telegraf"
+	"github.com/influxdata/telegraf/internal"
 	"github.com/influxdata/telegraf/plugins/inputs"
 
 	"github.com/soniah/gosnmp"
 )
 
+var defaultTimeout = internal.Duration{Duration: time.Second * 5}
+
 type handler func(*gosnmp.SnmpPacket, *net.UDPAddr)
-type execer func(string, ...string) ([]byte, error)
+type execer func(internal.Duration, string, ...string) ([]byte, error)
 
 type mibEntry struct {
 	mibName string
@@ -25,7 +28,8 @@ type mibEntry struct {
 }
 
 type SnmpTrap struct {
-	ServiceAddress string `toml:"service_address"`
+	ServiceAddress string            `toml:"service_address"`
+	Timeout        internal.Duration `toml:"timeout"`
 
 	acc      telegraf.Accumulator
 	listener *gosnmp.TrapListener
@@ -47,6 +51,8 @@ var sampleConfig = `
   ## be "udp://".  Omit local address to listen on all interfaces.
   ##   example: "udp://127.0.0.1:1234"
   # service_address = udp://:162
+  ## Timeout running snmptranslate command
+  # timeout = "5s"
 `
 
 func (s *SnmpTrap) SampleConfig() string {
@@ -66,12 +72,20 @@ func init() {
 		return &SnmpTrap{
 			timeFunc:       time.Now,
 			ServiceAddress: "udp://:162",
+			Timeout:        defaultTimeout,
 		}
 	})
 }
 
-func realExecCmd(arg0 string, args ...string) ([]byte, error) {
-	return exec.Command(arg0, args...).Output()
+func realExecCmd(Timeout internal.Duration, arg0 string, args ...string) ([]byte, error) {
+	cmd := exec.Command(arg0, args...)
+	var out bytes.Buffer
+	cmd.Stdout = &out
+	err := internal.RunTimeout(cmd, Timeout.Duration)
+	if err != nil {
+		return nil, err
+	}
+	return out.Bytes(), nil
 }
 
 func (s *SnmpTrap) Init() error {
@@ -154,13 +168,13 @@ func makeTrapHandler(s *SnmpTrap) handler {
 			switch v.Type {
 			case gosnmp.ObjectIdentifier:
 				val, ok := v.Value.(string)
-				var e mibEntry
-				var err error
 				if !ok {
-					s.Log.Errorf("Error getting value OID: %v", err)
+					s.Log.Errorf("Error getting value OID")
 					return
 				}
 
+				var e mibEntry
+				var err error
 				e, err = s.lookup(val)
 				if nil != err {
 					s.Log.Errorf("Error resolving value OID: %v", err)
@@ -225,9 +239,7 @@ func (s *SnmpTrap) load(oid string, e mibEntry) {
 
 func (s *SnmpTrap) snmptranslate(oid string) (e mibEntry, err error) {
 	var out []byte
-	out, err = s.execCmd("snmptranslate", "-Td", "-Ob", "-m", "all", oid)
-
-	//var e mibEntry
+	out, err = s.execCmd(s.Timeout, "snmptranslate", "-Td", "-Ob", "-m", "all", oid)
 
 	if err != nil {
 		return e, err
diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go
index 0e9a0daa4f25a..ed31786d81119 100644
--- a/plugins/inputs/snmp_trap/snmp_trap_test.go
+++ b/plugins/inputs/snmp_trap/snmp_trap_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/soniah/gosnmp"
 
 	"github.com/influxdata/telegraf"
+	"github.com/influxdata/telegraf/internal"
 	"github.com/influxdata/telegraf/testutil"
 
 	"github.com/stretchr/testify/require"
@@ -168,7 +169,7 @@ func TestReceiveTrap(t *testing.T) {
 
 }
 
-func fakeExecCmd(_ string, _ ...string) ([]byte, error) {
+func fakeExecCmd(_ internal.Duration, _ string, _ ...string) ([]byte, error) {
 	return nil, fmt.Errorf("intentional failure")
 }
 

From b3b31327bdf0261b3131f200a4a6a2659cd0debb Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Mon, 25 Nov 2019 11:13:08 -0700
Subject: [PATCH 25/26] log if there's an error in Stop()

---
 plugins/inputs/snmp_trap/snmp_trap.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index b1f48377525cb..cbd3e44163d36 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -140,7 +140,10 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 
 func (s *SnmpTrap) Stop() {
 	s.listener.Close()
-	_ = <-s.errCh
+	err := <-s.errCh
+	if nil != err {
+		s.Log.Errorf("Error stopping trap listener %v", err)
+	}
 }
 
 func makeTrapHandler(s *SnmpTrap) handler {

From 8cec5a02a4f1dbdfd34991997f5d39f811dbf9ee Mon Sep 17 00:00:00 2001
From: David Reimschussel <dreimschussel@influxdata.com>
Date: Mon, 25 Nov 2019 11:28:44 -0700
Subject: [PATCH 26/26] more cleanup

---
 plugins/inputs/snmp_trap/snmp_trap.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index cbd3e44163d36..4b9ce4a563844 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -113,7 +113,7 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 	protocol := split[0]
 	addr := split[1]
 
-	// gosnmp.TrapListener currentl supports udp only.  For forward
+	// gosnmp.TrapListener currently supports udp only.  For forward
 	// compatibility, require udp in the service address
 	if protocol != "udp" {
 		return fmt.Errorf("unknown protocol '%s' in '%s'", protocol, s.ServiceAddress)
@@ -251,7 +251,7 @@ func (s *SnmpTrap) snmptranslate(oid string) (e mibEntry, err error) {
 	scanner := bufio.NewScanner(bytes.NewBuffer(out))
 	ok := scanner.Scan()
 	if err = scanner.Err(); !ok && err != nil {
-		return e, err //Errorf(scanner.Err(), "getting OID text")
+		return e, err
 	}
 
 	e.oidText = scanner.Text()