Skip 404 error reporting in nginx_plus_api input (#6015)

plugins/inputs/nginx_plus_api/nginx_plus_api_metrics.go

@@ -2,6 +2,7 @@ package nginx_plus_api
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -13,16 +14,33 @@ import (
 	"github.com/influxdata/telegraf"
 )
 
+var (
+	// errNotFound signals that the NGINX API routes does not exist.
+	errNotFound = errors.New("not found")
+)
+
 func (n *NginxPlusApi) gatherMetrics(addr *url.URL, acc telegraf.Accumulator) {
-	acc.AddError(n.gatherProcessesMetrics(addr, acc))
+	addError(acc, n.gatherProcessesMetrics(addr, acc))
-	acc.AddError(n.gatherConnectionsMetrics(addr, acc))
+	addError(acc, n.gatherConnectionsMetrics(addr, acc))
-	acc.AddError(n.gatherSslMetrics(addr, acc))
+	addError(acc, n.gatherSslMetrics(addr, acc))
-	acc.AddError(n.gatherHttpRequestsMetrics(addr, acc))
+	addError(acc, n.gatherHttpRequestsMetrics(addr, acc))
-	acc.AddError(n.gatherHttpServerZonesMetrics(addr, acc))
+	addError(acc, n.gatherHttpServerZonesMetrics(addr, acc))
-	acc.AddError(n.gatherHttpUpstreamsMetrics(addr, acc))
+	addError(acc, n.gatherHttpUpstreamsMetrics(addr, acc))
-	acc.AddError(n.gatherHttpCachesMetrics(addr, acc))
+	addError(acc, n.gatherHttpCachesMetrics(addr, acc))
-	acc.AddError(n.gatherStreamServerZonesMetrics(addr, acc))
+	addError(acc, n.gatherStreamServerZonesMetrics(addr, acc))
-	acc.AddError(n.gatherStreamUpstreamsMetrics(addr, acc))
+	addError(acc, n.gatherStreamUpstreamsMetrics(addr, acc))
+}
+
+func addError(acc telegraf.Accumulator, err error) {
+	// This plugin has hardcoded API resource paths it checks that may not
+	// be in the nginx.conf.  Currently, this is to prevent logging of
+	// paths that are not configured.
+	//
+	// The correct solution is to do a GET to /api to get the available paths
+	// on the server rather than simply ignore.
+	if err != errNotFound {
+		acc.AddError(err)
+	}
 }
 
 func (n *NginxPlusApi) gatherUrl(addr *url.URL, path string) ([]byte, error) {
@@ -33,9 +51,17 @@ func (n *NginxPlusApi) gatherUrl(addr *url.URL, path string) ([]byte, error) {
 		return nil, fmt.Errorf("error making HTTP request to %s: %s", url, err)
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
+
+	switch resp.StatusCode {
+	case http.StatusOK:
+	case http.StatusNotFound:
+		// format as special error to catch and ignore as some nginx API
+		// features are either optional, or only available in some versions
+		return nil, errNotFound
+	default:
 		return nil, fmt.Errorf("%s returned HTTP status %s", url, resp.Status)
 	}
+
 	contentType := strings.Split(resp.Header.Get("Content-Type"), ";")[0]
 	switch contentType {
 	case "application/json":

plugins/inputs/nginx_plus_api/nginx_plus_api_metrics_test.go

@@ -448,11 +448,11 @@ const streamServerZonesPayload = `
 `
 
 func TestGatherProcessesMetrics(t *testing.T) {
-	ts, n := prepareEndpoint(processesPath, defaultApiVersion, processesPayload)
+	ts, n := prepareEndpoint(t, processesPath, defaultApiVersion, processesPayload)
 	defer ts.Close()
 
 	var acc testutil.Accumulator
-	addr, host, port := prepareAddr(ts)
+	addr, host, port := prepareAddr(t, ts)
 
 	require.NoError(t, n.gatherProcessesMetrics(addr, &acc))
 
@@ -468,12 +468,12 @@ func TestGatherProcessesMetrics(t *testing.T) {
 		})
 }
 
-func TestGatherConnectioinsMetrics(t *testing.T) {
+func TestGatherConnectionsMetrics(t *testing.T) {
-	ts, n := prepareEndpoint(connectionsPath, defaultApiVersion, connectionsPayload)
+	ts, n := prepareEndpoint(t, connectionsPath, defaultApiVersion, connectionsPayload)
 	defer ts.Close()
 
 	var acc testutil.Accumulator
-	addr, host, port := prepareAddr(ts)
+	addr, host, port := prepareAddr(t, ts)
 
 	require.NoError(t, n.gatherConnectionsMetrics(addr, &acc))
 
@@ -493,11 +493,11 @@ func TestGatherConnectioinsMetrics(t *testing.T) {
 }
 
 func TestGatherSslMetrics(t *testing.T) {
-	ts, n := prepareEndpoint(sslPath, defaultApiVersion, sslPayload)
+	ts, n := prepareEndpoint(t, sslPath, defaultApiVersion, sslPayload)
 	defer ts.Close()
 
 	var acc testutil.Accumulator
-	addr, host, port := prepareAddr(ts)
+	addr, host, port := prepareAddr(t, ts)
 
 	require.NoError(t, n.gatherSslMetrics(addr, &acc))
 
@@ -516,11 +516,11 @@ func TestGatherSslMetrics(t *testing.T) {
 }
 
 func TestGatherHttpRequestsMetrics(t *testing.T) {
-	ts, n := prepareEndpoint(httpRequestsPath, defaultApiVersion, httpRequestsPayload)
+	ts, n := prepareEndpoint(t, httpRequestsPath, defaultApiVersion, httpRequestsPayload)
 	defer ts.Close()
 
 	var acc testutil.Accumulator
-	addr, host, port := prepareAddr(ts)
+	addr, host, port := prepareAddr(t, ts)
 
 	require.NoError(t, n.gatherHttpRequestsMetrics(addr, &acc))
 
@@ -538,11 +538,11 @@ func TestGatherHttpRequestsMetrics(t *testing.T) {
 }
 
 func TestGatherHttpServerZonesMetrics(t *testing.T) {
-	ts, n := prepareEndpoint(httpServerZonesPath, defaultApiVersion, httpServerZonesPayload)
+	ts, n := prepareEndpoint(t, httpServerZonesPath, defaultApiVersion, httpServerZonesPayload)
 	defer ts.Close()
 
 	var acc testutil.Accumulator
-	addr, host, port := prepareAddr(ts)
+	addr, host, port := prepareAddr(t, ts)
 
 	require.NoError(t, n.gatherHttpServerZonesMetrics(addr, &acc))
 
@@ -592,11 +592,11 @@ func TestGatherHttpServerZonesMetrics(t *testing.T) {
 }
 
 func TestHatherHttpUpstreamsMetrics(t *testing.T) {
-	ts, n := prepareEndpoint(httpUpstreamsPath, defaultApiVersion, httpUpstreamsPayload)
+	ts, n := prepareEndpoint(t, httpUpstreamsPath, defaultApiVersion, httpUpstreamsPayload)
 	defer ts.Close()
 
 	var acc testutil.Accumulator
-	addr, host, port := prepareAddr(ts)
+	addr, host, port := prepareAddr(t, ts)
 
 	require.NoError(t, n.gatherHttpUpstreamsMetrics(addr, &acc))
 
@@ -764,11 +764,11 @@ func TestHatherHttpUpstreamsMetrics(t *testing.T) {
 }
 
 func TestGatherHttpCachesMetrics(t *testing.T) {
-	ts, n := prepareEndpoint(httpCachesPath, defaultApiVersion, httpCachesPayload)
+	ts, n := prepareEndpoint(t, httpCachesPath, defaultApiVersion, httpCachesPayload)
 	defer ts.Close()
 
 	var acc testutil.Accumulator
-	addr, host, port := prepareAddr(ts)
+	addr, host, port := prepareAddr(t, ts)
 
 	require.NoError(t, n.gatherHttpCachesMetrics(addr, &acc))
 
@@ -842,11 +842,11 @@ func TestGatherHttpCachesMetrics(t *testing.T) {
 }
 
 func TestGatherStreamUpstreams(t *testing.T) {
-	ts, n := prepareEndpoint(streamUpstreamsPath, defaultApiVersion, streamUpstreamsPayload)
+	ts, n := prepareEndpoint(t, streamUpstreamsPath, defaultApiVersion, streamUpstreamsPayload)
 	defer ts.Close()
 
 	var acc testutil.Accumulator
-	addr, host, port := prepareAddr(ts)
+	addr, host, port := prepareAddr(t, ts)
 
 	require.NoError(t, n.gatherStreamUpstreamsMetrics(addr, &acc))
 
@@ -984,12 +984,12 @@ func TestGatherStreamUpstreams(t *testing.T) {
 
 }
 
-func TestGatherStreamServerZonesMatrics(t *testing.T) {
+func TestGatherStreamServerZonesMetrics(t *testing.T) {
-	ts, n := prepareEndpoint(streamServerZonesPath, defaultApiVersion, streamServerZonesPayload)
+	ts, n := prepareEndpoint(t, streamServerZonesPath, defaultApiVersion, streamServerZonesPayload)
 	defer ts.Close()
 
 	var acc testutil.Accumulator
-	addr, host, port := prepareAddr(ts)
+	addr, host, port := prepareAddr(t, ts)
 
 	require.NoError(t, n.gatherStreamServerZonesMetrics(addr, &acc))
 
@@ -1023,11 +1023,92 @@ func TestGatherStreamServerZonesMatrics(t *testing.T) {
 			"zone":   "dns",
 		})
 }
+func TestUnavailableEndpoints(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	}))
+	defer ts.Close()
 
-func prepareAddr(ts *httptest.Server) (*url.URL, string, string) {
+	n := &NginxPlusApi{
+		client: ts.Client(),
+	}
+
+	addr, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var acc testutil.Accumulator
+	n.gatherMetrics(addr, &acc)
+	require.NoError(t, acc.FirstError())
+}
+
+func TestServerError(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer ts.Close()
+
+	n := &NginxPlusApi{
+		client: ts.Client(),
+	}
+
+	addr, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var acc testutil.Accumulator
+	n.gatherMetrics(addr, &acc)
+	require.Error(t, acc.FirstError())
+}
+
+func TestMalformedJSON(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json; charset=utf-8")
+		fmt.Fprintln(w, "this is not JSON")
+	}))
+	defer ts.Close()
+
+	n := &NginxPlusApi{
+		client: ts.Client(),
+	}
+
+	addr, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var acc testutil.Accumulator
+	n.gatherMetrics(addr, &acc)
+	require.Error(t, acc.FirstError())
+}
+
+func TestUnknownContentType(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+	}))
+	defer ts.Close()
+
+	n := &NginxPlusApi{
+		client: ts.Client(),
+	}
+
+	addr, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var acc testutil.Accumulator
+	n.gatherMetrics(addr, &acc)
+	require.Error(t, acc.FirstError())
+}
+
+func prepareAddr(t *testing.T, ts *httptest.Server) (*url.URL, string, string) {
+	t.Helper()
 	addr, err := url.Parse(fmt.Sprintf("%s/api", ts.URL))
 	if err != nil {
-		panic(err)
+		t.Fatal(err)
 	}
 
 	host, port, err := net.SplitHostPort(addr.Host)
@@ -1046,7 +1127,7 @@ func prepareAddr(ts *httptest.Server) (*url.URL, string, string) {
 	return addr, host, port
 }
 
-func prepareEndpoint(path string, apiVersion int64, payload string) (*httptest.Server, *NginxPlusApi) {
+func prepareEndpoint(t *testing.T, path string, apiVersion int64, payload string) (*httptest.Server, *NginxPlusApi) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		var rsp string
 
@@ -1054,7 +1135,7 @@ func prepareEndpoint(path string, apiVersion int64, payload string) (*httptest.S
 			rsp = payload
 			w.Header()["Content-Type"] = []string{"application/json"}
 		} else {
-			panic("Cannot handle request")
+			t.Errorf("unknown request path")
 		}
 
 		fmt.Fprintln(w, rsp)
@@ -1067,7 +1148,7 @@ func prepareEndpoint(path string, apiVersion int64, payload string) (*httptest.S
 
 	client, err := n.createHttpClient()
 	if err != nil {
-		panic(err)
+		t.Fatal(err)
 	}
 	n.client = client