Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add result_type field to net_response input plugin #2990

Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions plugins/inputs/net_response/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Example Input Plugin
# Network Response Input Plugin

The input plugin test UDP/TCP connections response time.
It can also check response text.
Expand Down Expand Up @@ -59,7 +59,8 @@ It can also check response text.

- net_response
- response_time (float, seconds)
- string_found (bool) # Only if "expected: option is set
- result_type (string) # success, timeout, connection_failed, read_failed, string_mismatch
- [**DEPRECATED**] string_mismatch (boolean)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is wrong, should be string_found


### Tags:

Expand All @@ -72,7 +73,11 @@ It can also check response text.

```
$ ./telegraf --config telegraf.conf --input-filter net_response --test
net_response,server=192.168.2.2,port=22,protocol=tcp response_time=0.18070360500000002,string_found=true 1454785464182527094
net_response,server=192.168.2.2,port=2222,protocol=tcp response_time=1.090124776,string_found=false 1454784433658942325

net_response,server=influxdata.com,port=8080,protocol=tcp,host=localhost result_type="timeout" 1499310361000000000
net_response,server=influxdata.com,port=443,protocol=tcp,host=localhost result_type="success",response_time=0.088703864 1499310361000000000
net_response,protocol=tcp,host=localhost,server=this.domain.does.not.exist,port=443 result_type="connection_failed" 1499310361000000000
net_response,protocol=udp,host=localhost,server=influxdata.com,port=8080 result_type="read_failed" 1499310362000000000
net_response,port=31338,protocol=udp,host=localhost,server=localhost result_type="string_mismatch",string_found=false,response_time=0.00242682 1499310362000000000
net_response,protocol=udp,host=localhost,server=localhost,port=31338 response_time=0.001128598,result_type="success",string_found=true 1499310362000000000
net_response,server=this.domain.does.not.exist,port=443,protocol=udp,host=localhost result_type="connection_failed" 1499310362000000000
```
36 changes: 26 additions & 10 deletions plugins/inputs/net_response/net_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ func (n *NetResponse) TcpGather() (map[string]interface{}, error) {
responseTime := time.Since(start).Seconds()
// Handle error
if err != nil {
return nil, err
if e, ok := err.(net.Error); ok && e.Timeout() {
fields["result_type"] = "timeout"
} else {
fields["result_type"] = "connection_failed"
}
return fields, nil
}
defer conn.Close()
// Send string if needed
Expand All @@ -87,18 +92,21 @@ func (n *NetResponse) TcpGather() (map[string]interface{}, error) {
responseTime = time.Since(start).Seconds()
// Handle error
if err != nil {
fields["string_found"] = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still need this line so we don't change the existing item yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops -- yeah definitely. I'll take care of that.

fields["result_type"] = "read_failed"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just report string_mismatch here.

} else {
// Looking for string in answer
RegEx := regexp.MustCompile(`.*` + n.Expect + `.*`)
find := RegEx.FindString(string(data))
if find != "" {
fields["string_found"] = true
fields["result_type"] = "success"
fields["string_found"] = true // WARNING: This field will be deprecated in a future release.
} else {
fields["string_found"] = false
fields["result_type"] = "string_mismatch"
fields["string_found"] = false // WARNING: This field will be deprecated in a future release.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add these comments, having it up in the README is enough.

}
}

} else {
fields["result_type"] = "success"
}
fields["response_time"] = responseTime
return fields, nil
Expand All @@ -114,11 +122,16 @@ func (n *NetResponse) UdpGather() (map[string]interface{}, error) {
LocalAddr, err := net.ResolveUDPAddr("udp", "127.0.0.1:0")
// Connecting
conn, err := net.DialUDP("udp", LocalAddr, udpAddr)
defer conn.Close()
// Handle error
if err != nil {
return nil, err
if e, ok := err.(net.Error); ok && e.Timeout() {
fields["result_type"] = "timeout"
} else {
fields["result_type"] = "connection_failed"
}
return fields, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it even possible to get in here with UDP?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. The more generic "connection_failed" probably makes more sense here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or if the fact that UDP is connectionless is not consistent with such a name, perhaps add a new failure state. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a good starting point to just have connection_failed here, we can always add more specific states later if needed.

}
defer conn.Close()
// Send string
msg := []byte(n.Send)
conn.Write(msg)
Expand All @@ -132,15 +145,18 @@ func (n *NetResponse) UdpGather() (map[string]interface{}, error) {
responseTime := time.Since(start).Seconds()
// Handle error
if err != nil {
return nil, err
fields["result_type"] = "read_failed"
return fields, nil
} else {
// Looking for string in answer
RegEx := regexp.MustCompile(`.*` + n.Expect + `.*`)
find := RegEx.FindString(string(buf))
if find != "" {
fields["string_found"] = true
fields["result_type"] = "success"
fields["string_found"] = true // WARNING: This field will be deprecated in a future release.
} else {
fields["string_found"] = false
fields["result_type"] = "string_mismatch"
fields["string_found"] = false // WARNING: This field will be deprecated in a future release.
}
}
fields["response_time"] = responseTime
Expand Down
40 changes: 34 additions & 6 deletions plugins/inputs/net_response/net_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package net_response

import (
"net"
"regexp"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -36,8 +35,18 @@ func TestTCPError(t *testing.T) {
}
// Error
err1 := c.Gather(&acc)
require.Error(t, err1)
assert.Contains(t, err1.Error(), "getsockopt: connection refused")
require.NoError(t, err1)
acc.AssertContainsTaggedFields(t,
"net_response",
map[string]interface{}{
"result_type": "connection_failed",
},
map[string]string{
"server": "",
"port": "9999",
"protocol": "tcp",
},
)
}

func TestTCPOK1(t *testing.T) {
Expand Down Expand Up @@ -68,6 +77,7 @@ func TestTCPOK1(t *testing.T) {
acc.AssertContainsTaggedFields(t,
"net_response",
map[string]interface{}{
"result_type": "success",
"string_found": true,
"response_time": 1.0,
},
Expand Down Expand Up @@ -108,6 +118,7 @@ func TestTCPOK2(t *testing.T) {
acc.AssertContainsTaggedFields(t,
"net_response",
map[string]interface{}{
"result_type": "string_mismatch",
"string_found": false,
"response_time": 1.0,
},
Expand All @@ -129,10 +140,26 @@ func TestUDPrror(t *testing.T) {
Expect: "test",
Protocol: "udp",
}
// Error
// Gather
err1 := c.Gather(&acc)
require.Error(t, err1)
assert.Regexp(t, regexp.MustCompile(`read udp 127.0.0.1:[0-9]*->127.0.0.1:9999: recvfrom: connection refused`), err1.Error())
// Override response time
for _, p := range acc.Metrics {
p.Fields["response_time"] = 1.0
}
// Error
require.NoError(t, err1)
acc.AssertContainsTaggedFields(t,
"net_response",
map[string]interface{}{
"result_type": "read_failed",
"response_time": 1.0,
},
map[string]string{
"server": "",
"port": "9999",
"protocol": "udp",
},
)
}

func TestUDPOK1(t *testing.T) {
Expand Down Expand Up @@ -163,6 +190,7 @@ func TestUDPOK1(t *testing.T) {
acc.AssertContainsTaggedFields(t,
"net_response",
map[string]interface{}{
"result_type": "success",
"string_found": true,
"response_time": 1.0,
},
Expand Down