Update health_checker plugin and add test

Author: Ben Keith

collectd-plugins.yaml

@@ -26,7 +26,7 @@
   repo: signalfx/collectd-haproxy
 
 - name: health_checker
-  version: multi-instance  #v1.0.0
+  version: v0.0.3
   repo: signalfx/collectd-health_checker
 
 - name: jenkins

docs/monitors/collectd-health-checker.md

@@ -22,9 +22,12 @@ Monitor Type: `collectd/health-checker`
 | `host` | **yes** | `string` |  |
 | `port` | **yes** | `integer` |  |
 | `name` | no | `string` |  |
-| `url` | no | `string` |  |
+| `path` | no | `string` | The HTTP path that contains a JSON document to verify (**default:** `/`) |
+| `jsonKey` | no | `string` | If `jsonKey` and `jsonVal` are given, the given endpoint will be interpreted as a JSON document and will be expected to contain the given key and value for the service to be considered healthy. |
 | `jsonVal` | no | `any` | This can be either a string or numeric type |
-| `jsonKey` | no | `string` |  |
+| `useHTTPS` | no | `bool` | If true, the endpoint will be connected to on HTTPS instead of plain HTTP.  It is invalid to specify this if `tcpCheck` is true. (**default:** `false`) |
+| `skipSecurity` | no | `bool` | If true, and `useHTTPS` is true, the server's SSL/TLS cert will not be verified. (**default:** `false`) |
+| `tcpCheck` | no | `bool` | If true, the plugin will verify that it can connect to the given host/port value. JSON checking is not supported. (**default:** `false`) |
 
 
 

internal/monitors/collectd/healthchecker/healthchecker.go

@@ -5,6 +5,8 @@ package healthchecker
 //go:generate collectd-template-to-go healthchecker.tmpl
 
 import (
+	"errors"
+
 	"github.com/signalfx/signalfx-agent/internal/core/config"
 	"github.com/signalfx/signalfx-agent/internal/core/services"
 	"github.com/signalfx/signalfx-agent/internal/monitors"
@@ -37,10 +39,34 @@ type Config struct {
 	Port uint16 `yaml:"port" validate:"required"`
 	Name string `yaml:"name"`
 
-	URL string `yaml:"url"`
+	// The HTTP path that contains a JSON document to verify
+	Path string `yaml:"path" default:"/"`
+	// If `jsonKey` and `jsonVal` are given, the given endpoint will be
+	// interpreted as a JSON document and will be expected to contain the given
+	// key and value for the service to be considered healthy.
+	JSONKey string `yaml:"jsonKey"`
 	// This can be either a string or numeric type
 	JSONVal interface{} `yaml:"jsonVal"`
-	JSONKey string      `yaml:"jsonKey"`
+	// If true, the endpoint will be connected to on HTTPS instead of plain
+	// HTTP.  It is invalid to specify this if `tcpCheck` is true.
+	UseHTTPS bool `yaml:"useHTTPS"`
+	// If true, and `useHTTPS` is true, the server's SSL/TLS cert will not be
+	// verified.
+	SkipSecurity bool `yaml:"skipSecurity"`
+	// If true, the plugin will verify that it can connect to the given
+	// host/port value. JSON checking is not supported.
+	TCPCheck bool `yaml:"tcpCheck"`
+}
+
+// Validate the given config
+func (c *Config) Validate() error {
+	if c.TCPCheck && (c.SkipSecurity || c.UseHTTPS) {
+		return errors.New("neither skipSecurity nor useHTTPS should be set when tcpCheck is true")
+	}
+	if c.TCPCheck && (c.JSONKey != "" || c.JSONVal != nil) {
+		return errors.New("cannot do JSON value check with tcpCheck set to true")
+	}
+	return nil
 }
 
 // Monitor is the main type that represents the monitor

internal/monitors/collectd/healthchecker/healthchecker.tmpl

@@ -1,12 +1,19 @@
 LoadPlugin "python"
 <Plugin python>
   ModulePath "{{ pluginRoot }}/health_checker"
+  LogTraces true
 
   Import "health_checker"
   <Module health_checker>
-    URL "{{renderValue .URL .}}"
-    Instance "{{.Name}}[monitorID={{.MonitorID}}]"
+  {{ if not .TCPCheck }}
+    URL "http{{if .UseHTTPS}}s{{end}}://{{.Host}}:{{.Port}}{{.Path}}"
     JSONKey "{{.JSONKey}}"
     JSONVal "{{.JSONVal}}"
+  {{ else }}
+    URL "{{.Host}}"
+    TCP {{.Port}}
+  {{ end }}
+    SkipSecurity {{toBool .SkipSecurity}}
+    Instance "{{.Name}}[monitorID={{.MonitorID}}]"
   </Module>
 </Plugin>

tests/helpers/util.py

@@ -15,7 +15,7 @@
 AGENT_BIN = os.environ.get("AGENT_BIN", "/bundle/bin/signalfx-agent")
 PROJECT_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
 TEST_SERVICES_DIR = os.environ.get("TEST_SERVICES_DIR", "/test-services")
-DEFAULT_TIMEOUT = os.environ.get("DEFAULT_TIMEOUT", 20)
+DEFAULT_TIMEOUT = os.environ.get("DEFAULT_TIMEOUT", 30)
 DOCKER_API_VERSION = "1.34"
 
 

tests/kubernetes/data.py

@@ -89,7 +89,7 @@
      ["haproxy-configmap.yaml", "haproxy.yaml"]),
     ({"type": "collectd/health-checker",
       "discoveryRule": 'container_image =~ "json-server" && private_port == 80',
-      "url": 'http://{{.Host}}:{{.Port}}/health',
+      "path": '/health',
       "jsonKey": "status",
       "jsonVal": "ok"},
      ["health-checker-configmap.yaml", "health-checker.yaml"]),

tests/monitors/health_checker_test.py

@@ -0,0 +1,38 @@
+from functools import partial as p
+from textwrap import dedent
+import string
+
+from tests.helpers.util import wait_for, run_agent, run_service, container_ip
+from tests.helpers.assertions import has_datapoint_with_dim, tcp_socket_open
+
+config = string.Template("""
+monitors:
+  - type: collectd/health-checker
+    host: $host
+    port: 80
+    tcpCheck: true
+""")
+
+def test_health_checker_tcp():
+    with run_service("nginx") as nginx_container:
+        host = container_ip(nginx_container)
+        assert wait_for(p(tcp_socket_open, host, 80), 60), "service didn't start"
+
+        with run_agent(config.substitute(host=host)) as [backend, _, _]:
+            assert wait_for(p(has_datapoint_with_dim, backend, "plugin", "health_checker")), \
+                "Didn't get health_checker datapoints"
+
+def test_health_checker_http():
+    with run_service("nginx") as nginx_container:
+        host = container_ip(nginx_container)
+        assert wait_for(p(tcp_socket_open, host, 80), 60), "service didn't start"
+
+        with run_agent(string.Template(dedent("""
+        monitors:
+          - type: collectd/health-checker
+            host: $host
+            port: 80
+            path: /nonexistent
+        """)).substitute(host=host)) as [backend, _, _]:
+            assert wait_for(p(has_datapoint_with_dim, backend, "plugin", "health_checker")), \
+                "Didn't get health_checker datapoints"