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.

Sign up for GitHub

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

Jump to bottom

fix(inputs/nginx_plus_api): swallow 404 error reporting #6015

Merged
merged 2 commits into from
Jun 19, 2019
Merged

fix(inputs/nginx_plus_api): swallow 404 error reporting #6015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7075e43
fix(inputs/nginx_plus_api): swallow 404 error reporting
goller Jun 19, 2019
f1313d8
chore(inputs/nginx_plus_api): go fmt of switch
goller Jun 19, 2019
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
46 changes: 36 additions & 10 deletions plugins/inputs/nginx_plus_api/nginx_plus_api_metrics.go
View file
Open in desktop
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
Open in desktop
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