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

promhttp: Correctly detect invalid metric and label names #823

Merged
merged 1 commit into from Dec 9, 2020
Merged

promhttp: Correctly detect invalid metric and label names #823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 51 additions & 40 deletions prometheus/promhttp/instrument_server.go
View file
Open in desktop
Expand Up @@ -43,14 +43,14 @@ func InstrumentHandlerInFlight(g prometheus.Gauge, next http.Handler) http.Handl

// InstrumentHandlerDuration is a middleware that wraps the provided
// 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
// those, the only allowed label names are "code" and "method". The function
// panics otherwise. The Observe method of the Observer in the ObserverVec is
// called with the request duration in seconds. Partitioning happens by HTTP
// status code and/or HTTP method if the respective instance label names are
// present in the ObserverVec. For unpartitioned observations, use an
// ObserverVec with zero labels. Note that partitioning of Histograms is
// expensive and should be used judiciously.
// The ObserverVec must have valid metric and label names and must have zero,
// one, or two non-const non-curried labels. For those, the only allowed label
// names are "code" and "method". The function panics otherwise. The Observe
// method of the Observer in the ObserverVec is called with the request duration
// in seconds. Partitioning happens by HTTP status code and/or HTTP method if
// the respective instance label names are present in the ObserverVec. For
// unpartitioned observations, use an ObserverVec with zero labels. Note that
// 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.
//
Expand Down Expand Up @@ -79,12 +79,13 @@ func InstrumentHandlerDuration(obs prometheus.ObserverVec, next http.Handler) ht
}

// InstrumentHandlerCounter is a middleware that wraps the provided http.Handler
// 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
// allowed label names are "code" and "method". The function panics
// otherwise. Partitioning of the CounterVec happens by HTTP status code and/or
// HTTP method if the respective instance label names are present in the
// CounterVec. For unpartitioned counting, use a CounterVec with zero labels.
// to observe the request result with the provided CounterVec. The CounterVec
// must have valid metric and label names and must have zero, one, or two
// non-const non-curried labels. For those, the only allowed label names are
// "code" and "method". The function panics otherwise. Partitioning of the
// CounterVec happens by HTTP status code and/or HTTP method if the respective
// 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.
//
Expand All @@ -110,14 +111,15 @@ func InstrumentHandlerCounter(counter *prometheus.CounterVec, next http.Handler)

// InstrumentHandlerTimeToWriteHeader is a middleware that wraps the provided
// http.Handler to observe with the provided ObserverVec the request duration
// until the response headers are written. The ObserverVec must have zero, one,
// or two non-const non-curried labels. For those, the only allowed label names
// are "code" and "method". The function panics otherwise. The Observe method of
// the Observer in the ObserverVec is called with the request duration in
// seconds. Partitioning happens by HTTP status code and/or HTTP method if the
// respective instance label names are present in the ObserverVec. For
// unpartitioned observations, use an ObserverVec with zero labels. Note that
// partitioning of Histograms is expensive and should be used judiciously.
// until the response headers are written. The ObserverVec must have valid
// metric and label names and must have zero, one, or two non-const non-curried
// labels. For those, the only allowed label names are "code" and "method". The
// function panics otherwise. The Observe method of the Observer in the
// ObserverVec is called with the request duration in seconds. Partitioning
// happens by HTTP status code and/or HTTP method if the respective instance
// label names are present in the ObserverVec. For unpartitioned observations,
// 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
// reported.
Expand All @@ -139,15 +141,15 @@ func InstrumentHandlerTimeToWriteHeader(obs prometheus.ObserverVec, next http.Ha
}

// InstrumentHandlerRequestSize is a middleware that wraps the provided
// 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
// those, the only allowed label names are "code" and "method". The function
// panics otherwise. The Observe method of the Observer in the ObserverVec is
// called with the request size in bytes. Partitioning happens by HTTP status
// code and/or HTTP method if the respective instance label names are present in
// the ObserverVec. For unpartitioned observations, use an ObserverVec with zero
// labels. Note that partitioning of Histograms is expensive and should be used
// judiciously.
// http.Handler to observe the request size with the provided ObserverVec. The
// ObserverVec must have valid metric and label names and must have zero, one,
// or two non-const non-curried labels. For those, the only allowed label names
// are "code" and "method". The function panics otherwise. The Observe method of
// the Observer in the ObserverVec is called with the request size in
// bytes. Partitioning happens by HTTP status code and/or HTTP method if the
// respective instance label names are present in the ObserverVec. For
// unpartitioned observations, use an ObserverVec with zero labels. Note that
// 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.
//
Expand All @@ -174,15 +176,15 @@ func InstrumentHandlerRequestSize(obs prometheus.ObserverVec, next http.Handler)
}

// InstrumentHandlerResponseSize is a middleware that wraps the provided
// 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
// those, the only allowed label names are "code" and "method". The function
// panics otherwise. The Observe method of the Observer in the ObserverVec is
// called with the response size in bytes. Partitioning happens by HTTP status
// code and/or HTTP method if the respective instance label names are present in
// the ObserverVec. For unpartitioned observations, use an ObserverVec with zero
// labels. Note that partitioning of Histograms is expensive and should be used
// judiciously.
// http.Handler to observe the response size with the provided ObserverVec. The
// ObserverVec must have valid metric and label names and must have zero, one,
// or two non-const non-curried labels. For those, the only allowed label names
// are "code" and "method". The function panics otherwise. The Observe method of
// the Observer in the ObserverVec is called with the response size in
// bytes. Partitioning happens by HTTP status code and/or HTTP method if the
// respective instance label names are present in the ObserverVec. For
// unpartitioned observations, use an ObserverVec with zero labels. Note that
// 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.
//
Expand All @@ -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) {
// TODO(beorn7): Remove this hacky way to check for instance labels
// once Descriptors can have their dimensionality queried.
Expand Down Expand Up @@ -225,6 +232,10 @@ func checkLabels(c prometheus.Collector) (code bool, method bool) {

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
// variable labels there are, try for as long as it needs.
for err := errors.New("dummy"); err != nil; lvs = append(lvs, magicString) {
Expand Down
31 changes: 28 additions & 3 deletions prometheus/promhttp/instrument_server_test.go
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ import (

func TestLabelCheck(t *testing.T) {
scenarios := map[string]struct {
metricName string // Defaults to "c".
varLabels []string
constLabels []string
curriedLabels []string
Expand All @@ -48,7 +49,7 @@ func TestLabelCheck(t *testing.T) {
curriedLabels: []string{},
ok: true,
},
"cade and method as var labels": {
"code and method as var labels": {
varLabels: []string{"method", "code"},
constLabels: []string{},
curriedLabels: []string{},
Expand All @@ -60,6 +61,12 @@ func TestLabelCheck(t *testing.T) {
curriedLabels: []string{"dings", "bums"},
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": {
varLabels: []string{"foo"},
constLabels: []string{},
Expand Down Expand Up @@ -96,25 +103,43 @@ func TestLabelCheck(t *testing.T) {
curriedLabels: []string{"method"},
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 {
t.Run(name, func(t *testing.T) {
metricName := sc.metricName
if metricName == "" {
metricName = "c"
}
constLabels := prometheus.Labels{}
for _, l := range sc.constLabels {
constLabels[l] = "dummy"
}
c := prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "c",
Name: metricName,
Help: "c help",
ConstLabels: constLabels,
},
append(sc.varLabels, sc.curriedLabels...),
)
o := prometheus.ObserverVec(prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Name: "c",
Name: metricName,
Help: "c help",
ConstLabels: constLabels,
},
Expand Down