CL-Softwares
/
telegraf
8
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

chore(inputs.modbus): Add device or controller information to error messages (#16114)

This commit is contained in:
Sven Rebhan 2024-11-04 22:55:54 +01:00 committed by GitHub
parent 5793ff6366
commit 85f025ad22
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 47 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
plugins/inputs/modbus/configuration_request_test.go
View File

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

38
plugins/inputs/modbus/modbus.go
View File

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

4
plugins/inputs/modbus/modbus_test.go
View File

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

2
plugins/inputs/modbus/testcases/duplicate_fields_same_slave_and_request/init.err
View File

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