Skip to content

Commit

Permalink
Skip 404 error reporting in nginx_plus_api input (influxdata#6015)
Browse files Browse the repository at this point in the history
  • Loading branch information
@goller
goller authored and Mathieu Lecarme committed Apr 17, 2020
1 parent 81ee324 commit f6ca0bd
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 35 deletions.
46 changes: 36 additions & 10 deletions plugins/inputs/nginx_plus_api/nginx_plus_api_metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package nginx_plus_api

import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net"
Expand All @@ -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))
acc.AddError(n.gatherConnectionsMetrics(addr, acc))
acc.AddError(n.gatherSslMetrics(addr, acc))
acc.AddError(n.gatherHttpRequestsMetrics(addr, acc))
acc.AddError(n.gatherHttpServerZonesMetrics(addr, acc))
acc.AddError(n.gatherHttpUpstreamsMetrics(addr, acc))
acc.AddError(n.gatherHttpCachesMetrics(addr, acc))
acc.AddError(n.gatherStreamServerZonesMetrics(addr, acc))
acc.AddError(n.gatherStreamUpstreamsMetrics(addr, acc))
addError(acc, n.gatherProcessesMetrics(addr, acc))
addError(acc, n.gatherConnectionsMetrics(addr, acc))
addError(acc, n.gatherSslMetrics(addr, acc))
addError(acc, n.gatherHttpRequestsMetrics(addr, acc))
addError(acc, n.gatherHttpServerZonesMetrics(addr, acc))
addError(acc, n.gatherHttpUpstreamsMetrics(addr, acc))
addError(acc, n.gatherHttpCachesMetrics(addr, acc))
addError(acc, n.gatherStreamServerZonesMetrics(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) {
Expand All @@ -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":
Expand Down
131 changes: 106 additions & 25 deletions plugins/inputs/nginx_plus_api/nginx_plus_api_metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand All @@ -468,12 +468,12 @@ func TestGatherProcessesMetrics(t *testing.T) {
})
}

func TestGatherConnectioinsMetrics(t *testing.T) {
ts, n := prepareEndpoint(connectionsPath, defaultApiVersion, connectionsPayload)
func TestGatherConnectionsMetrics(t *testing.T) {
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))

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

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

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

Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -984,12 +984,12 @@ func TestGatherStreamUpstreams(t *testing.T) {

}

func TestGatherStreamServerZonesMatrics(t *testing.T) {
ts, n := prepareEndpoint(streamServerZonesPath, defaultApiVersion, streamServerZonesPayload)
func TestGatherStreamServerZonesMetrics(t *testing.T) {
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))

Expand Down Expand Up @@ -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()

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(ts *httptest.Server) (*url.URL, string, string) {
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)
Expand All @@ -1046,15 +1127,15 @@ 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

if r.URL.Path == fmt.Sprintf("/api/%d/%s", apiVersion, path) {
rsp = payload
w.Header()["Content-Type"] = []string{"application/json"}
} else {
panic("Cannot handle request")
t.Errorf("unknown request path")
}

fmt.Fprintln(w, rsp)
Expand All @@ -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

Expand Down

0 comments on commit f6ca0bd

Please sign in to comment.