Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
chendave committed Aug 9, 2021
1 parent 5b92f43 commit e7122e4
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 8 deletions.
10 changes: 3 additions & 7 deletions staging/src/k8s.io/component-base/metrics/testutil/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,6 @@ func validateMetricFamily(gatherer metrics.Gatherer, metricName string) (*dto.Me
}
}

if metricFamily == nil {
return nil, fmt.Errorf("metric %q not found", metricName)
}

if metricFamily.GetMetric() == nil {
return nil, fmt.Errorf("metric %q is empty", metricName)
}
Expand All @@ -278,7 +274,7 @@ func validateMetricFamily(gatherer metrics.Gatherer, metricName string) (*dto.Me

// GetHistogramVecFromGatherer collects a metric, that matches the input labelValue map
// from a gatherer implementing k8s.io/component-base/metrics.Gatherer interface.
// All metrics within the metricFamily will be assembled as a single HistogramVec. Quantle, Average etc. are caculated based on
// All metrics within the metricFamily will be assembled as a single HistogramVec. Quantile, Average etc. are caculated based on
// the accumulated data from the HistogramVec.
// Used only for testing purposes where we need to gather metrics directly from a running binary (without metrics endpoint).
func GetHistogramVecFromGatherer(gatherer metrics.Gatherer, metricName string, lvMap map[string]string) (HistogramVec, error) {
Expand All @@ -303,10 +299,10 @@ type LabeledHistogram struct {
Histogram Histogram
}

// GetHistogramFromGatherer collects a metric from a gatherer implementing k8s.io/component-base/metrics.Gatherer interface.
// GetHistogramsFromGatherer collects a metric from a gatherer implementing k8s.io/component-base/metrics.Gatherer interface.
// If the metric is a HistogramVec, each metric from the HistogramVec will be collected with a label map one by one.
// Used only for testing purposes where we need to gather metrics directly from a running binary (without metrics endpoint).
func GetHistogramFromGatherer(gatherer metrics.Gatherer, metricName string, lvMap map[string]string) ([]LabeledHistogram, error) {
func GetHistogramsFromGatherer(gatherer metrics.Gatherer, metricName string, lvMap map[string]string) ([]LabeledHistogram, error) {
metricFamily, err := validateMetricFamily(gatherer, metricName)
if err != nil {
return nil, err
Expand Down
39 changes: 39 additions & 0 deletions staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ import (
"reflect"
"testing"

"k8s.io/component-base/metrics"
"k8s.io/component-base/metrics/legacyregistry"
"k8s.io/utils/pointer"

"github.com/google/go-cmp/cmp"
"github.com/prometheus/client_golang/prometheus"
dto "github.com/prometheus/client_model/go"
)

Expand Down Expand Up @@ -512,3 +516,38 @@ func TestHistogramVec_Validate(t *testing.T) {
})
}
}

func TestGetHistogramFromGatherer(t *testing.T) {
// Have multiple labels defined here, but we only care about the first label, i.e. "extension_point".
labels := []string{"extension_point", "status"}
lvMap := map[string]string{"extension_point": "*"}
HistogramOpts := &metrics.HistogramOpts{
Namespace: "namespace",
Name: "metric_test_name",
Subsystem: "subsystem",
Help: "histogram help message",
Buckets: prometheus.DefBuckets,
}
vec := metrics.NewHistogramVec(HistogramOpts, labels)
legacyregistry.MustRegister(vec)
vec.WithLabelValues("filter", "0").Observe(1.5)
vec.WithLabelValues("score", "0").Observe(3.5)
metricName := fmt.Sprintf("%s_%s_%s", HistogramOpts.Namespace, HistogramOpts.Subsystem, HistogramOpts.Name)
histogramList, err := GetHistogramsFromGatherer(legacyregistry.DefaultGatherer, metricName, lvMap)
if err != nil {
t.Errorf("Got error %v", err)
}
expectedList := []LabeledHistogram{
{
Labels: map[string]string{"extension_point": "filter"},
Histogram: samples2Histogram([]float64{1.5}, prometheus.DefBuckets),
},
{
Labels: map[string]string{"extension_point": "score"},
Histogram: samples2Histogram([]float64{3.5}, prometheus.DefBuckets),
},
}
if diff := cmp.Diff(histogramList, expectedList); diff != "" {
t.Errorf("Got unexpected histogramList:\n%s", diff)
}
}
2 changes: 1 addition & 1 deletion test/integration/scheduler_perf/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func collectHistogramVec(metric string, labels map[string]string, lvMap map[stri

func collectHistogram(metric string, labels map[string]string, lvMap map[string]string) []DataItem {
var dataItems []DataItem
vec, err := testutil.GetHistogramFromGatherer(legacyregistry.DefaultGatherer, metric, lvMap)
vec, err := testutil.GetHistogramsFromGatherer(legacyregistry.DefaultGatherer, metric, lvMap)
if err != nil {
klog.Error(err)
return nil
Expand Down

0 comments on commit e7122e4

Please sign in to comment.