Skip to content

Commit 7fcf9f7

Browse files
beorn7beorn7
committed
Merge pull request #97 from prometheus/beorn7/metric-family-injection
Allow the metric family injection hook to merge with existing metric fam...
2 parents 811a3a9 + 4ab527f commit 7fcf9f7

File tree

2 files changed

+159
-73
lines changed

2 files changed

+159
-73
lines changed

‎prometheus/registry.go‎

Lines changed: 71 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -158,14 +158,19 @@ func Unregister(c Collector) bool {
158158
// SetMetricFamilyInjectionHook sets a function that is called whenever metrics
159159
// are collected. The hook function must be set before metrics collection begins
160160
// (i.e. call SetMetricFamilyInjectionHook before setting the HTTP handler.) The
161-
// MetricFamily protobufs returned by the hook function areadded to the
162-
// delivered metrics. Each returned MetricFamily must have a unique name (also
163-
// taking into account the MetricFamilies created in the regular way).
161+
// MetricFamily protobufs returned by the hook function are merged with the
162+
// metrics collected in the usual way.
164163
//
165164
// This is a way to directly inject MetricFamily protobufs managed and owned by
166-
// the caller. The caller has full responsibility. No sanity checks are
167-
// performed on the returned protobufs (besides the name checks described
168-
// above). The function must be callable at any time and concurrently.
165+
// the caller. The caller has full responsibility. As no registration of the
166+
// injected metrics has happened, there is no descriptor to check against, and
167+
// there are no registration-time checks. If collect-time checks are disabled
168+
// (see function EnableCollectChecks), no sanity checks are performed on the
169+
// returned protobufs at all. If collect-checks are enabled, type and uniqueness
170+
// checks are performed, but no further consistency checks (which would require
171+
// knowledge of a metric descriptor).
172+
//
173+
// The function must be callable at any time and concurrently.
169174
func SetMetricFamilyInjectionHook(hook func() []*dto.MetricFamily) {
170175
defRegistry.metricFamilyInjectionHook = hook
171176
}
@@ -462,10 +467,26 @@ func (r *registry) writePB(w io.Writer, writeEncoded encoder) (int, error) {
462467

463468
if r.metricFamilyInjectionHook != nil {
464469
for _, mf := range r.metricFamilyInjectionHook() {
465-
if _, exists := metricFamiliesByName[mf.GetName()]; exists {
466-
return 0, fmt.Errorf("metric family with duplicate name injected: %s", mf)
470+
existingMF, exists := metricFamiliesByName[mf.GetName()]
471+
if !exists {
472+
metricFamiliesByName[mf.GetName()] = mf
473+
if r.collectChecksEnabled {
474+
for _, m := range mf.Metric {
475+
if err := r.checkConsistency(mf, m, nil, metricHashes); err != nil {
476+
return 0, err
477+
}
478+
}
479+
}
480+
continue
481+
}
482+
for _, m := range mf.Metric {
483+
if r.collectChecksEnabled {
484+
if err := r.checkConsistency(existingMF, m, nil, metricHashes); err != nil {
485+
return 0, err
486+
}
487+
}
488+
existingMF.Metric = append(existingMF.Metric, m)
467489
}
468-
metricFamiliesByName[mf.GetName()] = mf
469490
}
470491
}
471492

@@ -506,11 +527,42 @@ func (r *registry) checkConsistency(metricFamily *dto.MetricFamily, dtoMetric *d
506527
)
507528
}
508529

530+
// Is the metric unique (i.e. no other metric with the same name and the same label values)?
531+
h := fnv.New64a()
532+
var buf bytes.Buffer
533+
buf.WriteString(metricFamily.GetName())
534+
buf.WriteByte(model.SeparatorByte)
535+
h.Write(buf.Bytes())
536+
for _, lp := range dtoMetric.Label {
537+
buf.Reset()
538+
buf.WriteString(lp.GetValue())
539+
buf.WriteByte(model.SeparatorByte)
540+
h.Write(buf.Bytes())
541+
}
542+
metricHash := h.Sum64()
543+
if _, exists := metricHashes[metricHash]; exists {
544+
return fmt.Errorf(
545+
"collected metric %q was collected before with the same name and label values",
546+
dtoMetric,
547+
)
548+
}
549+
metricHashes[metricHash] = struct{}{}
550+
551+
if desc == nil {
552+
return nil // Nothing left to check if we have no desc.
553+
}
554+
509555
// Desc consistency with metric family.
556+
if metricFamily.GetName() != desc.fqName {
557+
return fmt.Errorf(
558+
"collected metric %q has name %q but should have %q",
559+
dtoMetric, metricFamily.GetName(), desc.fqName,
560+
)
561+
}
510562
if metricFamily.GetHelp() != desc.help {
511563
return fmt.Errorf(
512564
"collected metric %q has help %q but should have %q",
513-
dtoMetric, desc.help, metricFamily.GetHelp(),
565+
dtoMetric, metricFamily.GetHelp(), desc.help,
514566
)
515567
}
516568

@@ -540,27 +592,6 @@ func (r *registry) checkConsistency(metricFamily *dto.MetricFamily, dtoMetric *d
540592
}
541593
}
542594

543-
// Is the metric unique (i.e. no other metric with the same name and the same label values)?
544-
h := fnv.New64a()
545-
var buf bytes.Buffer
546-
buf.WriteString(desc.fqName)
547-
buf.WriteByte(model.SeparatorByte)
548-
h.Write(buf.Bytes())
549-
for _, lp := range dtoMetric.Label {
550-
buf.Reset()
551-
buf.WriteString(lp.GetValue())
552-
buf.WriteByte(model.SeparatorByte)
553-
h.Write(buf.Bytes())
554-
}
555-
metricHash := h.Sum64()
556-
if _, exists := metricHashes[metricHash]; exists {
557-
return fmt.Errorf(
558-
"collected metric %q was collected before with the same name and label values",
559-
dtoMetric,
560-
)
561-
}
562-
metricHashes[metricHash] = struct{}{}
563-
564595
r.mtx.RLock() // Remaining checks need the read lock.
565596
defer r.mtx.RUnlock()
566597

@@ -695,6 +726,15 @@ func (s metricSorter) Swap(i, j int) {
695726
}
696727

697728
func (s metricSorter) Less(i, j int) bool {
729+
if len(s[i].Label) != len(s[j].Label) {
730+
// This should not happen. The metrics are
731+
// inconsistent. However, we have to deal with the fact, as
732+
// people might use custom collectors or metric family injection
733+
// to create inconsistent metrics. So let's simply compare the
734+
// number of labels in this case. That will still yield
735+
// reproducible sorting.
736+
return len(s[i].Label) < len(s[j].Label)
737+
}
698738
for n, lp := range s[i].Label {
699739
vi := lp.GetValue()
700740
vj := s[j].Label[n].GetValue()

‎prometheus/registry_test.go‎

Lines changed: 88 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -61,31 +61,29 @@ func testHandler(t testing.TB) {
6161

6262
varintBuf := make([]byte, binary.MaxVarintLen32)
6363

64-
externalMetricFamily := []*dto.MetricFamily{
65-
{
66-
Name: proto.String("externalname"),
67-
Help: proto.String("externaldocstring"),
68-
Type: dto.MetricType_COUNTER.Enum(),
69-
Metric: []*dto.Metric{
70-
{
71-
Label: []*dto.LabelPair{
72-
{
73-
Name: proto.String("externallabelname"),
74-
Value: proto.String("externalval1"),
75-
},
76-
{
77-
Name: proto.String("externalconstname"),
78-
Value: proto.String("externalconstvalue"),
79-
},
64+
externalMetricFamily := &dto.MetricFamily{
65+
Name: proto.String("externalname"),
66+
Help: proto.String("externaldocstring"),
67+
Type: dto.MetricType_COUNTER.Enum(),
68+
Metric: []*dto.Metric{
69+
{
70+
Label: []*dto.LabelPair{
71+
{
72+
Name: proto.String("externallabelname"),
73+
Value: proto.String("externalval1"),
8074
},
81-
Counter: &dto.Counter{
82-
Value: proto.Float64(1),
75+
{
76+
Name: proto.String("externalconstname"),
77+
Value: proto.String("externalconstvalue"),
8378
},
8479
},
80+
Counter: &dto.Counter{
81+
Value: proto.Float64(1),
82+
},
8583
},
8684
},
8785
}
88-
marshaledExternalMetricFamily, err := proto.Marshal(externalMetricFamily[0])
86+
marshaledExternalMetricFamily, err := proto.Marshal(externalMetricFamily)
8987
if err != nil {
9088
t.Fatal(err)
9189
}
@@ -216,16 +214,42 @@ metric: <
216214
expectedMetricFamilyAsProtoCompactText := []byte(`name:"name" help:"docstring" type:COUNTER metric:<label:<name:"constname" value:"constvalue" > label:<name:"labelname" value:"val1" > counter:<value:1 > > metric:<label:<name:"constname" value:"constvalue" > label:<name:"labelname" value:"val2" > counter:<value:1 > >
217215
`)
218216

217+
externalMetricFamilyWithSameName := &dto.MetricFamily{
218+
Name: proto.String("name"),
219+
Help: proto.String("inconsistent help string does not matter here"),
220+
Type: dto.MetricType_COUNTER.Enum(),
221+
Metric: []*dto.Metric{
222+
{
223+
Label: []*dto.LabelPair{
224+
{
225+
Name: proto.String("constname"),
226+
Value: proto.String("constvalue"),
227+
},
228+
{
229+
Name: proto.String("labelname"),
230+
Value: proto.String("different_val"),
231+
},
232+
},
233+
Counter: &dto.Counter{
234+
Value: proto.Float64(42),
235+
},
236+
},
237+
},
238+
}
239+
240+
expectedMetricFamilyMergedWithExternalAsProtoCompactText := []byte(`name:"name" help:"docstring" type:COUNTER metric:<label:<name:"constname" value:"constvalue" > label:<name:"labelname" value:"different_val" > counter:<value:42 > > metric:<label:<name:"constname" value:"constvalue" > label:<name:"labelname" value:"val1" > counter:<value:1 > > metric:<label:<name:"constname" value:"constvalue" > label:<name:"labelname" value:"val2" > counter:<value:1 > >
241+
`)
242+
219243
type output struct {
220244
headers map[string]string
221245
body []byte
222246
}
223247

224248
var scenarios = []struct {
225-
headers map[string]string
226-
out output
227-
withCounter bool
228-
withExternalMF bool
249+
headers map[string]string
250+
out output
251+
collector Collector
252+
externalMF []*dto.MetricFamily
229253
}{
230254
{ // 0
231255
headers: map[string]string{
@@ -281,7 +305,7 @@ metric: <
281305
},
282306
body: expectedMetricFamilyAsText,
283307
},
284-
withCounter: true,
308+
collector: metricVec,
285309
},
286310
{ // 5
287311
headers: map[string]string{
@@ -293,7 +317,7 @@ metric: <
293317
},
294318
body: expectedMetricFamilyAsBytes,
295319
},
296-
withCounter: true,
320+
collector: metricVec,
297321
},
298322
{ // 6
299323
headers: map[string]string{
@@ -305,7 +329,7 @@ metric: <
305329
},
306330
body: externalMetricFamilyAsText,
307331
},
308-
withExternalMF: true,
332+
externalMF: []*dto.MetricFamily{externalMetricFamily},
309333
},
310334
{ // 7
311335
headers: map[string]string{
@@ -317,7 +341,7 @@ metric: <
317341
},
318342
body: externalMetricFamilyAsBytes,
319343
},
320-
withExternalMF: true,
344+
externalMF: []*dto.MetricFamily{externalMetricFamily},
321345
},
322346
{ // 8
323347
headers: map[string]string{
@@ -335,8 +359,8 @@ metric: <
335359
[]byte{},
336360
),
337361
},
338-
withCounter: true,
339-
withExternalMF: true,
362+
collector: metricVec,
363+
externalMF: []*dto.MetricFamily{externalMetricFamily},
340364
},
341365
{ // 9
342366
headers: map[string]string{
@@ -359,7 +383,7 @@ metric: <
359383
},
360384
body: expectedMetricFamilyAsText,
361385
},
362-
withCounter: true,
386+
collector: metricVec,
363387
},
364388
{ // 11
365389
headers: map[string]string{
@@ -377,8 +401,8 @@ metric: <
377401
[]byte{},
378402
),
379403
},
380-
withCounter: true,
381-
withExternalMF: true,
404+
collector: metricVec,
405+
externalMF: []*dto.MetricFamily{externalMetricFamily},
382406
},
383407
{ // 12
384408
headers: map[string]string{
@@ -396,8 +420,8 @@ metric: <
396420
[]byte{},
397421
),
398422
},
399-
withCounter: true,
400-
withExternalMF: true,
423+
collector: metricVec,
424+
externalMF: []*dto.MetricFamily{externalMetricFamily},
401425
},
402426
{ // 13
403427
headers: map[string]string{
@@ -415,8 +439,8 @@ metric: <
415439
[]byte{},
416440
),
417441
},
418-
withCounter: true,
419-
withExternalMF: true,
442+
collector: metricVec,
443+
externalMF: []*dto.MetricFamily{externalMetricFamily},
420444
},
421445
{ // 14
422446
headers: map[string]string{
@@ -434,20 +458,42 @@ metric: <
434458
[]byte{},
435459
),
436460
},
437-
withCounter: true,
438-
withExternalMF: true,
461+
collector: metricVec,
462+
externalMF: []*dto.MetricFamily{externalMetricFamily},
463+
},
464+
{ // 15
465+
headers: map[string]string{
466+
"Accept": "application/vnd.google.protobuf;proto=io.prometheus.client.MetricFamily;encoding=compact-text",
467+
},
468+
out: output{
469+
headers: map[string]string{
470+
"Content-Type": `application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily; encoding=compact-text`,
471+
},
472+
body: bytes.Join(
473+
[][]byte{
474+
externalMetricFamilyAsProtoCompactText,
475+
expectedMetricFamilyMergedWithExternalAsProtoCompactText,
476+
},
477+
[]byte{},
478+
),
479+
},
480+
collector: metricVec,
481+
externalMF: []*dto.MetricFamily{
482+
externalMetricFamily,
483+
externalMetricFamilyWithSameName,
484+
},
439485
},
440486
}
441487
for i, scenario := range scenarios {
442488
registry := newRegistry()
443489
registry.collectChecksEnabled = true
444490

445-
if scenario.withCounter {
446-
registry.Register(metricVec)
491+
if scenario.collector != nil {
492+
registry.Register(scenario.collector)
447493
}
448-
if scenario.withExternalMF {
494+
if scenario.externalMF != nil {
449495
registry.metricFamilyInjectionHook = func() []*dto.MetricFamily {
450-
return externalMetricFamily
496+
return scenario.externalMF
451497
}
452498
}
453499
writer := &fakeResponseWriter{

0 commit comments

Comments
 (0)