chore(inputs.modbus): Add device or controller information to error m…

…essages (influxdata#16114)

plugins/inputs/modbus/configuration_request_test.go

@@ -1722,7 +1722,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: empty field name in request for slave 1",
+			errormsg: "empty field name in request for slave 1",
 		},
 		{
 			name: "invalid byte-order (coil)",
@@ -1733,7 +1733,7 @@ func TestRequestFail(t *testing.T) {
 					RegisterType: "coil",
 				},
 			},
-			errormsg: "configuration invalid: unknown byte-order \"AB\"",
+			errormsg: "unknown byte-order \"AB\"",
 		},
 		{
 			name: "duplicate fields (coil)",
@@ -1754,7 +1754,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: field \"coil-0\" duplicated in measurement \"modbus\" (slave 1/\"coil\")",
+			errormsg: "field \"coil-0\" duplicated in measurement \"modbus\" (slave 1/\"coil\")",
 		},
 		{
 			name: "duplicate fields multiple requests (coil)",
@@ -1784,7 +1784,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: field \"coil-0\" duplicated in measurement \"foo\" (slave 1/\"coil\")",
+			errormsg: "field \"coil-0\" duplicated in measurement \"foo\" (slave 1/\"coil\")",
 		},
 		{
 			name: "invalid byte-order (discrete)",
@@ -1795,7 +1795,7 @@ func TestRequestFail(t *testing.T) {
 					RegisterType: "discrete",
 				},
 			},
-			errormsg: "configuration invalid: unknown byte-order \"AB\"",
+			errormsg: "unknown byte-order \"AB\"",
 		},
 		{
 			name: "duplicate fields (discrete)",
@@ -1816,7 +1816,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: field \"discrete-0\" duplicated in measurement \"modbus\" (slave 1/\"discrete\")",
+			errormsg: "field \"discrete-0\" duplicated in measurement \"modbus\" (slave 1/\"discrete\")",
 		},
 		{
 			name: "duplicate fields multiple requests (discrete)",
@@ -1846,7 +1846,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: field \"discrete-0\" duplicated in measurement \"foo\" (slave 1/\"discrete\")",
+			errormsg: "field \"discrete-0\" duplicated in measurement \"foo\" (slave 1/\"discrete\")",
 		},
 		{
 			name: "invalid byte-order (holding)",
@@ -1857,7 +1857,7 @@ func TestRequestFail(t *testing.T) {
 					RegisterType: "holding",
 				},
 			},
-			errormsg: "configuration invalid: unknown byte-order \"AB\"",
+			errormsg: "unknown byte-order \"AB\"",
 		},
 		{
 			name: "invalid field name (holding)",
@@ -1872,7 +1872,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: empty field name in request for slave 1",
+			errormsg: "empty field name in request for slave 1",
 		},
 		{
 			name: "invalid field input type (holding)",
@@ -1888,7 +1888,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "cannot process configuration: initializing field \"holding-0\" failed: invalid input datatype \"\" for determining field length",
+			errormsg: "initializing field \"holding-0\" failed: invalid input datatype \"\" for determining field length",
 		},
 		{
 			name: "invalid field output type (holding)",
@@ -1906,7 +1906,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: `configuration invalid: unknown output data-type "UINT8" for field "holding-0"`,
+			errormsg: `unknown output data-type "UINT8" for field "holding-0"`,
 		},
 		{
 			name: "duplicate fields (holding)",
@@ -1927,7 +1927,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: field \"holding-0\" duplicated in measurement \"modbus\" (slave 1/\"holding\")",
+			errormsg: "field \"holding-0\" duplicated in measurement \"modbus\" (slave 1/\"holding\")",
 		},
 		{
 			name: "duplicate fields multiple requests (holding)",
@@ -1957,7 +1957,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: field \"holding-0\" duplicated in measurement \"foo\" (slave 1/\"holding\")",
+			errormsg: "field \"holding-0\" duplicated in measurement \"foo\" (slave 1/\"holding\")",
 		},
 		{
 			name: "invalid byte-order (input)",
@@ -1968,7 +1968,7 @@ func TestRequestFail(t *testing.T) {
 					RegisterType: "input",
 				},
 			},
-			errormsg: "configuration invalid: unknown byte-order \"AB\"",
+			errormsg: "unknown byte-order \"AB\"",
 		},
 		{
 			name: "invalid field name (input)",
@@ -1983,7 +1983,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: empty field name in request for slave 1",
+			errormsg: "empty field name in request for slave 1",
 		},
 		{
 			name: "invalid field input type (input)",
@@ -1999,7 +1999,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "cannot process configuration: initializing field \"input-0\" failed: invalid input datatype \"\" for determining field length",
+			errormsg: "initializing field \"input-0\" failed: invalid input datatype \"\" for determining field length",
 		},
 		{
 			name: "invalid field output type (input)",
@@ -2017,7 +2017,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: `configuration invalid: unknown output data-type "UINT8" for field "input-0"`,
+			errormsg: `unknown output data-type "UINT8" for field "input-0"`,
 		},
 		{
 			name: "duplicate fields (input)",
@@ -2038,7 +2038,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: field \"input-0\" duplicated in measurement \"modbus\" (slave 1/\"input\")",
+			errormsg: "field \"input-0\" duplicated in measurement \"modbus\" (slave 1/\"input\")",
 		},
 		{
 			name: "duplicate fields multiple requests (input)",
@@ -2068,7 +2068,7 @@ func TestRequestFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: field \"input-0\" duplicated in measurement \"foo\" (slave 1/\"input\")",
+			errormsg: "field \"input-0\" duplicated in measurement \"foo\" (slave 1/\"input\")",
 		},
 	}
 
@@ -2082,9 +2082,7 @@ func TestRequestFail(t *testing.T) {
 			}
 			plugin.Requests = tt.requests
 
-			err := plugin.Init()
-			require.Error(t, err)
-			require.Equal(t, tt.errormsg, err.Error())
+			require.ErrorContains(t, plugin.Init(), tt.errormsg)
 			require.Empty(t, plugin.requests)
 		})
 	}
@@ -2258,7 +2256,7 @@ func TestRequestEmptyFields(t *testing.T) {
 		},
 	}
 	err := modbus.Init()
-	require.EqualError(t, err, `configuration invalid: found request section without fields`)
+	require.ErrorContains(t, err, `found request section without fields`)
 }
 
 func TestRequestMultipleSlavesOneFail(t *testing.T) {
@@ -2357,7 +2355,7 @@ func TestRequestMultipleSlavesOneFail(t *testing.T) {
 	actual := acc.GetTelegrafMetrics()
 	testutil.RequireMetricsEqual(t, expected, actual, testutil.IgnoreTime(), testutil.SortMetrics())
 	require.Len(t, acc.Errors, 1)
-	require.ErrorContains(t, acc.FirstError(), "slave 2: modbus: exception '11' (gateway target device failed to respond)")
+	require.ErrorContains(t, acc.FirstError(), `slave 2 on controller "tcp://localhost:1502": modbus: exception '11' (gateway target device failed to respond)`)
 }
 
 func TestRequestOptimizationShrink(t *testing.T) {
@@ -3002,7 +3000,7 @@ func TestRequestOptimizationMaxExtraRegisterFail(t *testing.T) {
 				},
 			},
 		},
-		errormsg: "configuration invalid: optimization_max_register_fill has to be between 1 and 125",
+		errormsg: "optimization_max_register_fill has to be between 1 and 125",
 	},
 		{
 			name: "MaxExtraRegister too small",
@@ -3022,7 +3020,7 @@ func TestRequestOptimizationMaxExtraRegisterFail(t *testing.T) {
 					},
 				},
 			},
-			errormsg: "configuration invalid: optimization_max_register_fill has to be between 1 and 125",
+			errormsg: "optimization_max_register_fill has to be between 1 and 125",
 		}}
 
 	for _, tt := range tests {
@@ -3035,9 +3033,7 @@ func TestRequestOptimizationMaxExtraRegisterFail(t *testing.T) {
 			}
 			plugin.Requests = tt.requests
 
-			err := plugin.Init()
-			require.Error(t, err)
-			require.Equal(t, tt.errormsg, err.Error())
+			require.ErrorContains(t, plugin.Init(), tt.errormsg)
 			require.Empty(t, plugin.requests)
 		})
 	}

plugins/inputs/modbus/modbus.go

@@ -136,7 +136,7 @@ func (m *Modbus) Init() error {
 	}
 
 	if m.Retries < 0 {
-		return errors.New("retries cannot be negative")
+		return fmt.Errorf("retries cannot be negative in device %q", m.Name)
 	}
 
 	// Determine the configuration style
@@ -157,23 +157,23 @@ func (m *Modbus) Init() error {
 		m.ConfigurationPerMetric.logger = m.Log
 		cfg = &m.ConfigurationPerMetric
 	default:
-		return fmt.Errorf("unknown configuration type %q", m.ConfigurationType)
+		return fmt.Errorf("unknown configuration type %q in device %q", m.ConfigurationType, m.Name)
 	}
 
 	// Check and process the configuration
 	if err := cfg.Check(); err != nil {
-		return fmt.Errorf("configuration invalid: %w", err)
+		return fmt.Errorf("configuration invalid for device %q: %w", m.Name, err)
 	}
 
 	r, err := cfg.Process()
 	if err != nil {
-		return fmt.Errorf("cannot process configuration: %w", err)
+		return fmt.Errorf("cannot process configuration for device %q: %w", m.Name, err)
 	}
 	m.requests = r
 
 	// Setup client
 	if err := m.initClient(); err != nil {
-		return fmt.Errorf("initializing client failed: %w", err)
+		return fmt.Errorf("initializing client failed for controller %q: %w", m.Controller, err)
 	}
 	for slaveID, rqs := range m.requests {
 		var nHoldingRegs, nInputsRegs, nDiscreteRegs, nCoilRegs uint16
@@ -195,23 +195,23 @@ func (m *Modbus) Init() error {
 			nCoilRegs += r.length
 			nCoilFields += len(r.fields)
 		}
-		m.Log.Infof("Got %d request(s) touching %d holding registers for %d fields (slave %d)",
-			len(rqs.holding), nHoldingRegs, nHoldingFields, slaveID)
+		m.Log.Infof("Got %d request(s) touching %d holding registers for %d fields (slave %d) on device %q",
+			len(rqs.holding), nHoldingRegs, nHoldingFields, slaveID, m.Name)
 		for i, r := range rqs.holding {
 			m.Log.Debugf("    #%d: @%d with length %d", i+1, r.address, r.length)
 		}
-		m.Log.Infof("Got %d request(s) touching %d inputs registers for %d fields (slave %d)",
-			len(rqs.input), nInputsRegs, nInputsFields, slaveID)
+		m.Log.Infof("Got %d request(s) touching %d inputs registers for %d fields (slave %d) on device %q",
+			len(rqs.input), nInputsRegs, nInputsFields, slaveID, m.Name)
 		for i, r := range rqs.input {
 			m.Log.Debugf("    #%d: @%d with length %d", i+1, r.address, r.length)
 		}
-		m.Log.Infof("Got %d request(s) touching %d discrete registers for %d fields (slave %d)",
-			len(rqs.discrete), nDiscreteRegs, nDiscreteFields, slaveID)
+		m.Log.Infof("Got %d request(s) touching %d discrete registers for %d fields (slave %d) on device %q",
+			len(rqs.discrete), nDiscreteRegs, nDiscreteFields, slaveID, m.Name)
 		for i, r := range rqs.discrete {
 			m.Log.Debugf("    #%d: @%d with length %d", i+1, r.address, r.length)
 		}
-		m.Log.Infof("Got %d request(s) touching %d coil registers for %d fields (slave %d)",
-			len(rqs.coil), nCoilRegs, nCoilFields, slaveID)
+		m.Log.Infof("Got %d request(s) touching %d coil registers for %d fields (slave %d) on device %q",
+			len(rqs.coil), nCoilRegs, nCoilFields, slaveID, m.Name)
 		for i, r := range rqs.coil {
 			m.Log.Debugf("    #%d: @%d with length %d", i+1, r.address, r.length)
 		}
@@ -230,15 +230,15 @@ func (m *Modbus) Gather(acc telegraf.Accumulator) error {
 	for slaveID, requests := range m.requests {
 		m.Log.Debugf("Reading slave %d for %s...", slaveID, m.Controller)
 		if err := m.readSlaveData(slaveID, requests); err != nil {
-			acc.AddError(fmt.Errorf("slave %d: %w", slaveID, err))
+			acc.AddError(fmt.Errorf("slave %d on controller %q: %w", slaveID, m.Controller, err))
 			var mbErr *mb.Error
 			if !errors.As(err, &mbErr) || mbErr.ExceptionCode != mb.ExceptionCodeServerDeviceBusy {
 				m.Log.Debugf("Reconnecting to %s...", m.Controller)
 				if err := m.disconnect(); err != nil {
-					return fmt.Errorf("disconnecting failed: %w", err)
+					return fmt.Errorf("disconnecting failed for controller %q: %w", m.Controller, err)
 				}
 				if err := m.connect(); err != nil {
-					return fmt.Errorf("slave %d: connecting failed: %w", slaveID, err)
+					return fmt.Errorf("slave %d on controller %q: connecting failed: %w", slaveID, m.Controller, err)
 				}
 			}
 			continue
@@ -319,7 +319,7 @@ func (m *Modbus) initClient() error {
 			handler.Logger = tracelog
 			m.handler = handler
 		default:
-			return fmt.Errorf("invalid transmission mode %q for %q", m.TransmissionMode, u.Scheme)
+			return fmt.Errorf("invalid transmission mode %q for %q on device %q", m.TransmissionMode, u.Scheme, m.Name)
 		}
 	case "", "file":
 		path := filepath.Join(u.Host, u.Path)
@@ -362,7 +362,7 @@ func (m *Modbus) initClient() error {
 			}
 			m.handler = handler
 		default:
-			return fmt.Errorf("invalid transmission mode %q for %q", m.TransmissionMode, u.Scheme)
+			return fmt.Errorf("invalid transmission mode %q for %q on device %q", m.TransmissionMode, u.Scheme, m.Name)
 		}
 	default:
 		return fmt.Errorf("invalid controller %q", m.Controller)
@@ -408,7 +408,7 @@ func (m *Modbus) readSlaveData(slaveID byte, requests requestSet) error {
 		}
 
 		// Wait some time and try again reading the slave.
-		m.Log.Infof("Device busy! Retrying %d more time(s)...", m.Retries-retry)
+		m.Log.Infof("Device busy! Retrying %d more time(s) on controller %q...", m.Retries-retry, m.Controller)
 		time.Sleep(time.Duration(m.RetriesWaitTime))
 	}
 	return m.gatherFields(requests)

plugins/inputs/modbus/modbus_test.go

@@ -244,7 +244,7 @@ func TestRetryFailExhausted(t *testing.T) {
 
 	require.NoError(t, modbus.Gather(&acc))
 	require.Len(t, acc.Errors, 1)
-	require.ErrorContains(t, acc.FirstError(), "slave 1: modbus: exception '6' (server device busy)")
+	require.ErrorContains(t, acc.FirstError(), `slave 1 on controller "tcp://localhost:1502": modbus: exception '6' (server device busy)`)
 }
 
 func TestRetryFailIllegal(t *testing.T) {
@@ -287,7 +287,7 @@ func TestRetryFailIllegal(t *testing.T) {
 
 	require.NoError(t, modbus.Gather(&acc))
 	require.Len(t, acc.Errors, 1)
-	require.ErrorContains(t, acc.FirstError(), "slave 1: modbus: exception '1' (illegal function)")
+	require.ErrorContains(t, acc.FirstError(), `slave 1 on controller "tcp://localhost:1502": modbus: exception '1' (illegal function)`)
 	require.Equal(t, 1, counter)
 }
 

plugins/inputs/modbus/testcases/duplicate_fields_same_slave_and_request/init.err

@@ -1 +1 @@
-configuration invalid: field "Voltage" duplicated in measurement "V"
+field "Voltage" duplicated in measurement "V"