Skip to content

Commit 8b73bd9

Browse files
beorn7beorn7
authored
Merge pull request #823 from prometheus/beorn7/promhttp
promhttp: Correctly detect invalid metric and label names
2 parents 37c26ed + 98eb6cb commit 8b73bd9

File tree

2 files changed

+79
-43
lines changed

2 files changed

+79
-43
lines changed

Diff for: ‎prometheus/promhttp/instrument_server.go

+51-40
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,14 @@ func InstrumentHandlerInFlight(g prometheus.Gauge, next http.Handler) http.Handl
4343

4444
// InstrumentHandlerDuration is a middleware that wraps the provided
4545
// http.Handler to observe the request duration with the provided ObserverVec.
46-
// The ObserverVec must have zero, one, or two non-const non-curried labels. For
47-
// those, the only allowed label names are "code" and "method". The function
48-
// panics otherwise. The Observe method of the Observer in the ObserverVec is
49-
// called with the request duration in seconds. Partitioning happens by HTTP
50-
// status code and/or HTTP method if the respective instance label names are
51-
// present in the ObserverVec. For unpartitioned observations, use an
52-
// ObserverVec with zero labels. Note that partitioning of Histograms is
53-
// expensive and should be used judiciously.
46+
// The ObserverVec must have valid metric and label names and must have zero,
47+
// one, or two non-const non-curried labels. For those, the only allowed label
48+
// names are "code" and "method". The function panics otherwise. The Observe
49+
// method of the Observer in the ObserverVec is called with the request duration
50+
// in seconds. Partitioning happens by HTTP status code and/or HTTP method if
51+
// the respective instance label names are present in the ObserverVec. For
52+
// unpartitioned observations, use an ObserverVec with zero labels. Note that
53+
// partitioning of Histograms is expensive and should be used judiciously.
5454
//
5555
// If the wrapped Handler does not set a status code, a status code of 200 is assumed.
5656
//
@@ -79,12 +79,13 @@ func InstrumentHandlerDuration(obs prometheus.ObserverVec, next http.Handler) ht
7979
}
8080

8181
// InstrumentHandlerCounter is a middleware that wraps the provided http.Handler
82-
// to observe the request result with the provided CounterVec. The CounterVec
83-
// must have zero, one, or two non-const non-curried labels. For those, the only
84-
// allowed label names are "code" and "method". The function panics
85-
// otherwise. Partitioning of the CounterVec happens by HTTP status code and/or
86-
// HTTP method if the respective instance label names are present in the
87-
// CounterVec. For unpartitioned counting, use a CounterVec with zero labels.
82+
// to observe the request result with the provided CounterVec. The CounterVec
83+
// must have valid metric and label names and must have zero, one, or two
84+
// non-const non-curried labels. For those, the only allowed label names are
85+
// "code" and "method". The function panics otherwise. Partitioning of the
86+
// CounterVec happens by HTTP status code and/or HTTP method if the respective
87+
// instance label names are present in the CounterVec. For unpartitioned
88+
// counting, use a CounterVec with zero labels.
8889
//
8990
// If the wrapped Handler does not set a status code, a status code of 200 is assumed.
9091
//
@@ -110,14 +111,15 @@ func InstrumentHandlerCounter(counter *prometheus.CounterVec, next http.Handler)
110111

111112
// InstrumentHandlerTimeToWriteHeader is a middleware that wraps the provided
112113
// http.Handler to observe with the provided ObserverVec the request duration
113-
// until the response headers are written. The ObserverVec must have zero, one,
114-
// or two non-const non-curried labels. For those, the only allowed label names
115-
// are "code" and "method". The function panics otherwise. The Observe method of
116-
// the Observer in the ObserverVec is called with the request duration in
117-
// seconds. Partitioning happens by HTTP status code and/or HTTP method if the
118-
// respective instance label names are present in the ObserverVec. For
119-
// unpartitioned observations, use an ObserverVec with zero labels. Note that
120-
// partitioning of Histograms is expensive and should be used judiciously.
114+
// until the response headers are written. The ObserverVec must have valid
115+
// metric and label names and must have zero, one, or two non-const non-curried
116+
// labels. For those, the only allowed label names are "code" and "method". The
117+
// function panics otherwise. The Observe method of the Observer in the
118+
// ObserverVec is called with the request duration in seconds. Partitioning
119+
// happens by HTTP status code and/or HTTP method if the respective instance
120+
// label names are present in the ObserverVec. For unpartitioned observations,
121+
// use an ObserverVec with zero labels. Note that partitioning of Histograms is
122+
// expensive and should be used judiciously.
121123
//
122124
// If the wrapped Handler panics before calling WriteHeader, no value is
123125
// reported.
@@ -139,15 +141,15 @@ func InstrumentHandlerTimeToWriteHeader(obs prometheus.ObserverVec, next http.Ha
139141
}
140142

141143
// InstrumentHandlerRequestSize is a middleware that wraps the provided
142-
// http.Handler to observe the request size with the provided ObserverVec. The
143-
// ObserverVec must have zero, one, or two non-const non-curried labels. For
144-
// those, the only allowed label names are "code" and "method". The function
145-
// panics otherwise. The Observe method of the Observer in the ObserverVec is
146-
// called with the request size in bytes. Partitioning happens by HTTP status
147-
// code and/or HTTP method if the respective instance label names are present in
148-
// the ObserverVec. For unpartitioned observations, use an ObserverVec with zero
149-
// labels. Note that partitioning of Histograms is expensive and should be used
150-
// judiciously.
144+
// http.Handler to observe the request size with the provided ObserverVec. The
145+
// ObserverVec must have valid metric and label names and must have zero, one,
146+
// or two non-const non-curried labels. For those, the only allowed label names
147+
// are "code" and "method". The function panics otherwise. The Observe method of
148+
// the Observer in the ObserverVec is called with the request size in
149+
// bytes. Partitioning happens by HTTP status code and/or HTTP method if the
150+
// respective instance label names are present in the ObserverVec. For
151+
// unpartitioned observations, use an ObserverVec with zero labels. Note that
152+
// partitioning of Histograms is expensive and should be used judiciously.
151153
//
152154
// If the wrapped Handler does not set a status code, a status code of 200 is assumed.
153155
//
@@ -174,15 +176,15 @@ func InstrumentHandlerRequestSize(obs prometheus.ObserverVec, next http.Handler)
174176
}
175177

176178
// InstrumentHandlerResponseSize is a middleware that wraps the provided
177-
// http.Handler to observe the response size with the provided ObserverVec. The
178-
// ObserverVec must have zero, one, or two non-const non-curried labels. For
179-
// those, the only allowed label names are "code" and "method". The function
180-
// panics otherwise. The Observe method of the Observer in the ObserverVec is
181-
// called with the response size in bytes. Partitioning happens by HTTP status
182-
// code and/or HTTP method if the respective instance label names are present in
183-
// the ObserverVec. For unpartitioned observations, use an ObserverVec with zero
184-
// labels. Note that partitioning of Histograms is expensive and should be used
185-
// judiciously.
179+
// http.Handler to observe the response size with the provided ObserverVec. The
180+
// ObserverVec must have valid metric and label names and must have zero, one,
181+
// or two non-const non-curried labels. For those, the only allowed label names
182+
// are "code" and "method". The function panics otherwise. The Observe method of
183+
// the Observer in the ObserverVec is called with the response size in
184+
// bytes. Partitioning happens by HTTP status code and/or HTTP method if the
185+
// respective instance label names are present in the ObserverVec. For
186+
// unpartitioned observations, use an ObserverVec with zero labels. Note that
187+
// partitioning of Histograms is expensive and should be used judiciously.
186188
//
187189
// If the wrapped Handler does not set a status code, a status code of 200 is assumed.
188190
//
@@ -198,6 +200,11 @@ func InstrumentHandlerResponseSize(obs prometheus.ObserverVec, next http.Handler
198200
})
199201
}
200202

203+
// checkLabels returns whether the provided Collector has a non-const,
204+
// non-curried label named "code" and/or "method". It panics if the provided
205+
// Collector does not have a Desc or has more than one Desc or its Desc is
206+
// invalid. It also panics if the Collector has any non-const, non-curried
207+
// labels that are not named "code" or "method".
201208
func checkLabels(c prometheus.Collector) (code bool, method bool) {
202209
// TODO(beorn7): Remove this hacky way to check for instance labels
203210
// once Descriptors can have their dimensionality queried.
@@ -225,6 +232,10 @@ func checkLabels(c prometheus.Collector) (code bool, method bool) {
225232

226233
close(descc)
227234

235+
// Make sure the Collector has a valid Desc by registering it with a
236+
// temporary registry.
237+
prometheus.NewRegistry().MustRegister(c)
238+
228239
// Create a ConstMetric with the Desc. Since we don't know how many
229240
// variable labels there are, try for as long as it needs.
230241
for err := errors.New("dummy"); err != nil; lvs = append(lvs, magicString) {

Diff for: ‎prometheus/promhttp/instrument_server_test.go

+28-3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
func TestLabelCheck(t *testing.T) {
2727
scenarios := map[string]struct {
28+
metricName string // Defaults to "c".
2829
varLabels []string
2930
constLabels []string
3031
curriedLabels []string
@@ -48,7 +49,7 @@ func TestLabelCheck(t *testing.T) {
4849
curriedLabels: []string{},
4950
ok: true,
5051
},
51-
"cade and method as var labels": {
52+
"code and method as var labels": {
5253
varLabels: []string{"method", "code"},
5354
constLabels: []string{},
5455
curriedLabels: []string{},
@@ -60,6 +61,12 @@ func TestLabelCheck(t *testing.T) {
6061
curriedLabels: []string{"dings", "bums"},
6162
ok: true,
6263
},
64+
"all labels used with an invalid const label name": {
65+
varLabels: []string{"code", "method"},
66+
constLabels: []string{"in-valid", "bar"},
67+
curriedLabels: []string{"dings", "bums"},
68+
ok: false,
69+
},
6370
"unsupported var label": {
6471
varLabels: []string{"foo"},
6572
constLabels: []string{},
@@ -96,25 +103,43 @@ func TestLabelCheck(t *testing.T) {
96103
curriedLabels: []string{"method"},
97104
ok: false,
98105
},
106+
"invalid name and otherwise empty": {
107+
metricName: "in-valid",
108+
varLabels: []string{},
109+
constLabels: []string{},
110+
curriedLabels: []string{},
111+
ok: false,
112+
},
113+
"invalid name with all the otherwise valid labels": {
114+
metricName: "in-valid",
115+
varLabels: []string{"code", "method"},
116+
constLabels: []string{"foo", "bar"},
117+
curriedLabels: []string{"dings", "bums"},
118+
ok: false,
119+
},
99120
}
100121

101122
for name, sc := range scenarios {
102123
t.Run(name, func(t *testing.T) {
124+
metricName := sc.metricName
125+
if metricName == "" {
126+
metricName = "c"
127+
}
103128
constLabels := prometheus.Labels{}
104129
for _, l := range sc.constLabels {
105130
constLabels[l] = "dummy"
106131
}
107132
c := prometheus.NewCounterVec(
108133
prometheus.CounterOpts{
109-
Name: "c",
134+
Name: metricName,
110135
Help: "c help",
111136
ConstLabels: constLabels,
112137
},
113138
append(sc.varLabels, sc.curriedLabels...),
114139
)
115140
o := prometheus.ObserverVec(prometheus.NewHistogramVec(
116141
prometheus.HistogramOpts{
117-
Name: "c",
142+
Name: metricName,
118143
Help: "c help",
119144
ConstLabels: constLabels,
120145
},

0 commit comments

Comments
 (0)