metrics: allow disabling OpenMetrics negotiation (#3944)

* metrics: allow disabling OpenMetrics negotiation

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* fixup! metrics: allow disabling OpenMetrics negotiation

caddytest/integration/caddyfile_adapt/metrics_disable_om.txt

@@ -0,0 +1,36 @@
+:80 {
+	metrics /metrics {
+		disable_openmetrics
+	}
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":80"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"path": [
+										"/metrics"
+									]
+								}
+							],
+							"handle": [
+								{
+									"disable_openmetrics": true,
+									"handler": "metrics"
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}

caddytest/integration/caddyfile_adapt/metrics_syntax.txt

@@ -0,0 +1,33 @@
+:80 {
+	metrics /metrics
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":80"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"path": [
+										"/metrics"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "metrics"
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}

go.mod

@@ -17,7 +17,7 @@ require (
 	github.com/mholt/acmez v0.1.1
 	github.com/naoina/go-stringutil v0.1.0 // indirect
 	github.com/naoina/toml v0.1.1
-	github.com/prometheus/client_golang v1.7.1
+	github.com/prometheus/client_golang v1.9.0
 	github.com/smallstep/certificates v0.15.4
 	github.com/smallstep/cli v0.15.2
 	github.com/smallstep/nosql v0.3.0 // cannot upgrade from v0.3.0 until protobuf warning is fixed

modules/metrics/adminmetrics.go

@@ -41,7 +41,7 @@ func (AdminMetrics) CaddyModule() caddy.ModuleInfo {
 
 // Routes returns a route for the /metrics endpoint.
 func (m *AdminMetrics) Routes() []caddy.AdminRoute {
-	metricsHandler := createMetricsHandler(nil)
+	metricsHandler := createMetricsHandler(nil, false)
 	h := caddy.AdminHandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
 		metricsHandler.ServeHTTP(w, r)
 		return nil

modules/metrics/metrics.go

@@ -38,6 +38,10 @@ func init() {
 // unlike AdminMetrics.
 type Metrics struct {
 	metricsHandler http.Handler
+
+	// Disable OpenMetrics negotiation, enabled by default. May be necessary if
+	// the produced metrics cannot be parsed by the service scraping metrics.
+	DisableOpenMetrics bool `json:"disable_openmetrics,omitempty"`
 }
 
 // CaddyModule returns the Caddy module information.
@@ -59,7 +63,7 @@ func (l *zapLogger) Println(v ...interface{}) {
 // Provision sets up m.
 func (m *Metrics) Provision(ctx caddy.Context) error {
 	log := ctx.Logger(m)
-	m.metricsHandler = createMetricsHandler(&zapLogger{log})
+	m.metricsHandler = createMetricsHandler(&zapLogger{log}, !m.DisableOpenMetrics)
 	return nil
 }
 
@@ -71,14 +75,25 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 
 // UnmarshalCaddyfile sets up the handler from Caddyfile tokens. Syntax:
 //
-//     metrics <matcher>
+//     metrics [<matcher>] {
+//         disable_openmetrics
+//     }
 //
 func (m *Metrics) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	for d.Next() {
 		args := d.RemainingArgs()
 		if len(args) > 0 {
 			return d.ArgErr()
 		}
+
+		for d.NextBlock(0) {
+			switch d.Val() {
+			case "disable_openmetrics":
+				m.DisableOpenMetrics = true
+			default:
+				return d.Errf("unrecognized subdirective %q", d.Val())
+			}
+		}
 	}
 	return nil
 }
@@ -95,15 +110,15 @@ var (
 	_ caddyfile.Unmarshaler       = (*Metrics)(nil)
 )
 
-func createMetricsHandler(logger promhttp.Logger) http.Handler {
+func createMetricsHandler(logger promhttp.Logger, enableOpenMetrics bool) http.Handler {
 	return promhttp.InstrumentMetricHandler(prometheus.DefaultRegisterer,
 		promhttp.HandlerFor(prometheus.DefaultGatherer, promhttp.HandlerOpts{
 			// will only log errors if logger is non-nil
 			ErrorLog: logger,
 
 			// Allow OpenMetrics format to be negotiated - largely compatible,
 			// except quantile/le label values always have a decimal.
-			EnableOpenMetrics: true,
+			EnableOpenMetrics: enableOpenMetrics,
 		}),
 	)
 }

modules/metrics/metrics_test.go

@@ -0,0 +1,45 @@
+package metrics
+
+import (
+	"testing"
+
+	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
+)
+
+func TestMetricsUnmarshalCaddyfile(t *testing.T) {
+	m := &Metrics{}
+	d := caddyfile.NewTestDispenser(`metrics bogus`)
+	err := m.UnmarshalCaddyfile(d)
+	if err == nil {
+		t.Errorf("expected error")
+	}
+
+	m = &Metrics{}
+	d = caddyfile.NewTestDispenser(`metrics`)
+	err = m.UnmarshalCaddyfile(d)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	if m.DisableOpenMetrics != false {
+		t.Errorf("DisableOpenMetrics should've been false: %v", m.DisableOpenMetrics)
+	}
+
+	m = &Metrics{}
+	d = caddyfile.NewTestDispenser(`metrics { disable_openmetrics }`)
+	err = m.UnmarshalCaddyfile(d)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	if m.DisableOpenMetrics != true {
+		t.Errorf("DisableOpenMetrics should've been true: %v", m.DisableOpenMetrics)
+	}
+
+	m = &Metrics{}
+	d = caddyfile.NewTestDispenser(`metrics { bogus }`)
+	err = m.UnmarshalCaddyfile(d)
+	if err == nil {
+		t.Errorf("expected error: %v", err)
+	}
+}