promhttp: Correctly detect invalid metric and label names

Without this fix, the `InstrumentHandler...` middlewares get locked in
an endless loop in case of an invalid Collector, eating all the memory.

Signed-off-by: beorn7 <beorn@grafana.com>
This commit is contained in:
beorn7 2020-12-09 17:58:53 +01:00
parent 37c26edd5b
commit 98eb6cbf7c
2 changed files with 79 additions and 43 deletions

View File

@ -43,14 +43,14 @@ func InstrumentHandlerInFlight(g prometheus.Gauge, next http.Handler) http.Handl
// InstrumentHandlerDuration is a middleware that wraps the provided // InstrumentHandlerDuration is a middleware that wraps the provided
// http.Handler to observe the request duration with the provided ObserverVec. // http.Handler to observe the request duration with the provided ObserverVec.
// The ObserverVec must have zero, one, or two non-const non-curried labels. For // The ObserverVec must have valid metric and label names and must have zero,
// those, the only allowed label names are "code" and "method". The function // one, or two non-const non-curried labels. For those, the only allowed label
// panics otherwise. The Observe method of the Observer in the ObserverVec is // names are "code" and "method". The function panics otherwise. The Observe
// called with the request duration in seconds. Partitioning happens by HTTP // method of the Observer in the ObserverVec is called with the request duration
// status code and/or HTTP method if the respective instance label names are // in seconds. Partitioning happens by HTTP status code and/or HTTP method if
// present in the ObserverVec. For unpartitioned observations, use an // the respective instance label names are present in the ObserverVec. For
// ObserverVec with zero labels. Note that partitioning of Histograms is // unpartitioned observations, use an ObserverVec with zero labels. Note that
// expensive and should be used judiciously. // partitioning of Histograms is expensive and should be used judiciously.
// //
// If the wrapped Handler does not set a status code, a status code of 200 is assumed. // If the wrapped Handler does not set a status code, a status code of 200 is assumed.
// //
@ -80,11 +80,12 @@ func InstrumentHandlerDuration(obs prometheus.ObserverVec, next http.Handler) ht
// InstrumentHandlerCounter is a middleware that wraps the provided http.Handler // InstrumentHandlerCounter is a middleware that wraps the provided http.Handler
// to observe the request result with the provided CounterVec. The CounterVec // to observe the request result with the provided CounterVec. The CounterVec
// must have zero, one, or two non-const non-curried labels. For those, the only // must have valid metric and label names and must have zero, one, or two
// allowed label names are "code" and "method". The function panics // non-const non-curried labels. For those, the only allowed label names are
// otherwise. Partitioning of the CounterVec happens by HTTP status code and/or // "code" and "method". The function panics otherwise. Partitioning of the
// HTTP method if the respective instance label names are present in the // CounterVec happens by HTTP status code and/or HTTP method if the respective
// CounterVec. For unpartitioned counting, use a CounterVec with zero labels. // instance label names are present in the CounterVec. For unpartitioned
// counting, use a CounterVec with zero labels.
// //
// If the wrapped Handler does not set a status code, a status code of 200 is assumed. // If the wrapped Handler does not set a status code, a status code of 200 is assumed.
// //
@ -110,14 +111,15 @@ func InstrumentHandlerCounter(counter *prometheus.CounterVec, next http.Handler)
// InstrumentHandlerTimeToWriteHeader is a middleware that wraps the provided // InstrumentHandlerTimeToWriteHeader is a middleware that wraps the provided
// http.Handler to observe with the provided ObserverVec the request duration // http.Handler to observe with the provided ObserverVec the request duration
// until the response headers are written. The ObserverVec must have zero, one, // until the response headers are written. The ObserverVec must have valid
// or two non-const non-curried labels. For those, the only allowed label names // metric and label names and must have zero, one, or two non-const non-curried
// are "code" and "method". The function panics otherwise. The Observe method of // labels. For those, the only allowed label names are "code" and "method". The
// the Observer in the ObserverVec is called with the request duration in // function panics otherwise. The Observe method of the Observer in the
// seconds. Partitioning happens by HTTP status code and/or HTTP method if the // ObserverVec is called with the request duration in seconds. Partitioning
// respective instance label names are present in the ObserverVec. For // happens by HTTP status code and/or HTTP method if the respective instance
// unpartitioned observations, use an ObserverVec with zero labels. Note that // label names are present in the ObserverVec. For unpartitioned observations,
// partitioning of Histograms is expensive and should be used judiciously. // use an ObserverVec with zero labels. Note that partitioning of Histograms is
// expensive and should be used judiciously.
// //
// If the wrapped Handler panics before calling WriteHeader, no value is // If the wrapped Handler panics before calling WriteHeader, no value is
// reported. // reported.
@ -140,14 +142,14 @@ func InstrumentHandlerTimeToWriteHeader(obs prometheus.ObserverVec, next http.Ha
// InstrumentHandlerRequestSize is a middleware that wraps the provided // InstrumentHandlerRequestSize is a middleware that wraps the provided
// http.Handler to observe the request size with the provided ObserverVec. The // http.Handler to observe the request size with the provided ObserverVec. The
// ObserverVec must have zero, one, or two non-const non-curried labels. For // ObserverVec must have valid metric and label names and must have zero, one,
// those, the only allowed label names are "code" and "method". The function // or two non-const non-curried labels. For those, the only allowed label names
// panics otherwise. The Observe method of the Observer in the ObserverVec is // are "code" and "method". The function panics otherwise. The Observe method of
// called with the request size in bytes. Partitioning happens by HTTP status // the Observer in the ObserverVec is called with the request size in
// code and/or HTTP method if the respective instance label names are present in // bytes. Partitioning happens by HTTP status code and/or HTTP method if the
// the ObserverVec. For unpartitioned observations, use an ObserverVec with zero // respective instance label names are present in the ObserverVec. For
// labels. Note that partitioning of Histograms is expensive and should be used // unpartitioned observations, use an ObserverVec with zero labels. Note that
// judiciously. // partitioning of Histograms is expensive and should be used judiciously.
// //
// If the wrapped Handler does not set a status code, a status code of 200 is assumed. // If the wrapped Handler does not set a status code, a status code of 200 is assumed.
// //
@ -175,14 +177,14 @@ func InstrumentHandlerRequestSize(obs prometheus.ObserverVec, next http.Handler)
// InstrumentHandlerResponseSize is a middleware that wraps the provided // InstrumentHandlerResponseSize is a middleware that wraps the provided
// http.Handler to observe the response size with the provided ObserverVec. The // http.Handler to observe the response size with the provided ObserverVec. The
// ObserverVec must have zero, one, or two non-const non-curried labels. For // ObserverVec must have valid metric and label names and must have zero, one,
// those, the only allowed label names are "code" and "method". The function // or two non-const non-curried labels. For those, the only allowed label names
// panics otherwise. The Observe method of the Observer in the ObserverVec is // are "code" and "method". The function panics otherwise. The Observe method of
// called with the response size in bytes. Partitioning happens by HTTP status // the Observer in the ObserverVec is called with the response size in
// code and/or HTTP method if the respective instance label names are present in // bytes. Partitioning happens by HTTP status code and/or HTTP method if the
// the ObserverVec. For unpartitioned observations, use an ObserverVec with zero // respective instance label names are present in the ObserverVec. For
// labels. Note that partitioning of Histograms is expensive and should be used // unpartitioned observations, use an ObserverVec with zero labels. Note that
// judiciously. // partitioning of Histograms is expensive and should be used judiciously.
// //
// If the wrapped Handler does not set a status code, a status code of 200 is assumed. // If the wrapped Handler does not set a status code, a status code of 200 is assumed.
// //
@ -198,6 +200,11 @@ func InstrumentHandlerResponseSize(obs prometheus.ObserverVec, next http.Handler
}) })
} }
// checkLabels returns whether the provided Collector has a non-const,
// non-curried label named "code" and/or "method". It panics if the provided
// Collector does not have a Desc or has more than one Desc or its Desc is
// invalid. It also panics if the Collector has any non-const, non-curried
// labels that are not named "code" or "method".
func checkLabels(c prometheus.Collector) (code bool, method bool) { func checkLabels(c prometheus.Collector) (code bool, method bool) {
// TODO(beorn7): Remove this hacky way to check for instance labels // TODO(beorn7): Remove this hacky way to check for instance labels
// once Descriptors can have their dimensionality queried. // once Descriptors can have their dimensionality queried.
@ -225,6 +232,10 @@ func checkLabels(c prometheus.Collector) (code bool, method bool) {
close(descc) close(descc)
// Make sure the Collector has a valid Desc by registering it with a
// temporary registry.
prometheus.NewRegistry().MustRegister(c)
// Create a ConstMetric with the Desc. Since we don't know how many // Create a ConstMetric with the Desc. Since we don't know how many
// variable labels there are, try for as long as it needs. // variable labels there are, try for as long as it needs.
for err := errors.New("dummy"); err != nil; lvs = append(lvs, magicString) { for err := errors.New("dummy"); err != nil; lvs = append(lvs, magicString) {

View File

@ -25,6 +25,7 @@ import (
func TestLabelCheck(t *testing.T) { func TestLabelCheck(t *testing.T) {
scenarios := map[string]struct { scenarios := map[string]struct {
metricName string // Defaults to "c".
varLabels []string varLabels []string
constLabels []string constLabels []string
curriedLabels []string curriedLabels []string
@ -48,7 +49,7 @@ func TestLabelCheck(t *testing.T) {
curriedLabels: []string{}, curriedLabels: []string{},
ok: true, ok: true,
}, },
"cade and method as var labels": { "code and method as var labels": {
varLabels: []string{"method", "code"}, varLabels: []string{"method", "code"},
constLabels: []string{}, constLabels: []string{},
curriedLabels: []string{}, curriedLabels: []string{},
@ -60,6 +61,12 @@ func TestLabelCheck(t *testing.T) {
curriedLabels: []string{"dings", "bums"}, curriedLabels: []string{"dings", "bums"},
ok: true, ok: true,
}, },
"all labels used with an invalid const label name": {
varLabels: []string{"code", "method"},
constLabels: []string{"in-valid", "bar"},
curriedLabels: []string{"dings", "bums"},
ok: false,
},
"unsupported var label": { "unsupported var label": {
varLabels: []string{"foo"}, varLabels: []string{"foo"},
constLabels: []string{}, constLabels: []string{},
@ -96,17 +103,35 @@ func TestLabelCheck(t *testing.T) {
curriedLabels: []string{"method"}, curriedLabels: []string{"method"},
ok: false, ok: false,
}, },
"invalid name and otherwise empty": {
metricName: "in-valid",
varLabels: []string{},
constLabels: []string{},
curriedLabels: []string{},
ok: false,
},
"invalid name with all the otherwise valid labels": {
metricName: "in-valid",
varLabels: []string{"code", "method"},
constLabels: []string{"foo", "bar"},
curriedLabels: []string{"dings", "bums"},
ok: false,
},
} }
for name, sc := range scenarios { for name, sc := range scenarios {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
metricName := sc.metricName
if metricName == "" {
metricName = "c"
}
constLabels := prometheus.Labels{} constLabels := prometheus.Labels{}
for _, l := range sc.constLabels { for _, l := range sc.constLabels {
constLabels[l] = "dummy" constLabels[l] = "dummy"
} }
c := prometheus.NewCounterVec( c := prometheus.NewCounterVec(
prometheus.CounterOpts{ prometheus.CounterOpts{
Name: "c", Name: metricName,
Help: "c help", Help: "c help",
ConstLabels: constLabels, ConstLabels: constLabels,
}, },
@ -114,7 +139,7 @@ func TestLabelCheck(t *testing.T) {
) )
o := prometheus.ObserverVec(prometheus.NewHistogramVec( o := prometheus.ObserverVec(prometheus.NewHistogramVec(
prometheus.HistogramOpts{ prometheus.HistogramOpts{
Name: "c", Name: metricName,
Help: "c help", Help: "c help",
ConstLabels: constLabels, ConstLabels: constLabels,
}, },