Extract label-resource logic to separate package

This extracts the label-to-resource conversion to a separte "naming" package for
use across different providers.

Early versions of the commit were done by antoinne85 in #79.

pkg/custom-provider/metric_namer.go

@@ -5,17 +5,14 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
-	"sync"
 	"text/template"
 
-	"github.com/golang/glog"
-	"github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	prom "github.com/directxman12/k8s-prometheus-adapter/pkg/client"
 	"github.com/directxman12/k8s-prometheus-adapter/pkg/config"
-	pmodel "github.com/prometheus/common/model"
+	"github.com/directxman12/k8s-prometheus-adapter/pkg/naming"
 )
 
 var nsGroupResource = schema.GroupResource{Resource: "namespaces"}
@@ -34,86 +31,13 @@ type MetricNamer interface {
 	// constrains beyond the series query.  It's assumed that the series given
 	// already matche the series query.
 	FilterSeries(series []prom.Series) []prom.Series
-	// ResourcesForSeries returns the group-resources associated with the given series,
-	// as well as whether or not the given series has the "namespace" resource).
-	ResourcesForSeries(series prom.Series) (res []schema.GroupResource, namespaced bool)
-	// LabelForResource returns the appropriate label for the given resource.
-	LabelForResource(resource schema.GroupResource) (pmodel.LabelName, error)
 	// MetricNameForSeries returns the name (as presented in the API) for a given series.
 	MetricNameForSeries(series prom.Series) (string, error)
 	// QueryForSeries returns the query for a given series (not API metric name), with
 	// the given namespace name (if relevant), resource, and resource names.
 	QueryForSeries(series string, resource schema.GroupResource, namespace string, names ...string) (prom.Selector, error)
-}
 
-// labelGroupResExtractor extracts schema.GroupResources from series labels.
+	naming.ResourceConverter
-type labelGroupResExtractor struct {
-	regex *regexp.Regexp
-
-	resourceInd int
-	groupInd    *int
-	mapper      apimeta.RESTMapper
-}
-
-// newLabelGroupResExtractor creates a new labelGroupResExtractor for labels whose form
-// matches the given template.  It does so by creating a regular expression from the template,
-// so anything in the template which limits resource or group name length will cause issues.
-func newLabelGroupResExtractor(labelTemplate *template.Template) (*labelGroupResExtractor, error) {
-	labelRegexBuff := new(bytes.Buffer)
-	if err := labelTemplate.Execute(labelRegexBuff, schema.GroupResource{"(?P<group>.+?)", "(?P<resource>.+?)"}); err != nil {
-		return nil, fmt.Errorf("unable to convert label template to matcher: %v", err)
-	}
-	if labelRegexBuff.Len() == 0 {
-		return nil, fmt.Errorf("unable to convert label template to matcher: empty template")
-	}
-	labelRegexRaw := "^" + labelRegexBuff.String() + "$"
-	labelRegex, err := regexp.Compile(labelRegexRaw)
-	if err != nil {
-		return nil, fmt.Errorf("unable to convert label template to matcher: %v", err)
-	}
-
-	var groupInd *int
-	var resInd *int
-
-	for i, name := range labelRegex.SubexpNames() {
-		switch name {
-		case "group":
-			ind := i // copy to avoid iteration variable reference
-			groupInd = &ind
-		case "resource":
-			ind := i // copy to avoid iteration variable reference
-			resInd = &ind
-		}
-	}
-
-	if resInd == nil {
-		return nil, fmt.Errorf("must include at least `{{.Resource}}` in the label template")
-	}
-
-	return &labelGroupResExtractor{
-		regex:       labelRegex,
-		resourceInd: *resInd,
-		groupInd:    groupInd,
-	}, nil
-}
-
-// GroupResourceForLabel extracts a schema.GroupResource from the given label, if possible.
-// The second argument indicates whether or not a potential group-resource was found in this label.
-func (e *labelGroupResExtractor) GroupResourceForLabel(lbl pmodel.LabelName) (schema.GroupResource, bool) {
-	matchGroups := e.regex.FindStringSubmatch(string(lbl))
-	if matchGroups != nil {
-		group := ""
-		if e.groupInd != nil {
-			group = matchGroups[*e.groupInd]
-		}
-
-		return schema.GroupResource{
-			Group:    group,
-			Resource: matchGroups[e.resourceInd],
-		}, true
-	}
-
-	return schema.GroupResource{}, false
 }
 
 func (r *metricNamer) Selector() prom.Selector {
@@ -161,17 +85,12 @@ func (m *reMatcher) Matches(val string) bool {
 
 type metricNamer struct {
 	seriesQuery          prom.Selector
-	labelTemplate        *template.Template
-	labelResExtractor    *labelGroupResExtractor
 	metricsQueryTemplate *template.Template
 	nameMatches          *regexp.Regexp
 	nameAs               string
 	seriesMatchers       []*reMatcher
 
-	labelResourceMu sync.RWMutex
+	naming.ResourceConverter
-	labelToResource map[pmodel.LabelName]schema.GroupResource
-	resourceToLabel map[schema.GroupResource]pmodel.LabelName
-	mapper          apimeta.RESTMapper
 }
 
 // queryTemplateArgs are the arguments for the metrics query template.
@@ -247,121 +166,6 @@ func (n *metricNamer) QueryForSeries(series string, resource schema.GroupResourc
 	return prom.Selector(queryBuff.String()), nil
 }
 
-func (n *metricNamer) ResourcesForSeries(series prom.Series) ([]schema.GroupResource, bool) {
-	// use an updates map to avoid having to drop the read lock to update the cache
-	// until the end.  Since we'll probably have few updates after the first run,
-	// this should mean that we rarely have to hold the write lock.
-	var resources []schema.GroupResource
-	updates := make(map[pmodel.LabelName]schema.GroupResource)
-	namespaced := false
-
-	// use an anon func to get the right defer behavior
-	func() {
-		n.labelResourceMu.RLock()
-		defer n.labelResourceMu.RUnlock()
-
-		for lbl := range series.Labels {
-			var groupRes schema.GroupResource
-			var ok bool
-
-			// check if we have an override
-			if groupRes, ok = n.labelToResource[lbl]; ok {
-				resources = append(resources, groupRes)
-			} else if groupRes, ok = updates[lbl]; ok {
-				resources = append(resources, groupRes)
-			} else if n.labelResExtractor != nil {
-				// if not, check if it matches the form we expect, and if so,
-				// convert to a group-resource.
-				if groupRes, ok = n.labelResExtractor.GroupResourceForLabel(lbl); ok {
-					info, _, err := provider.CustomMetricInfo{GroupResource: groupRes}.Normalized(n.mapper)
-					if err != nil {
-						// this is likely to show up for a lot of labels, so make it a verbose info log
-						glog.V(9).Infof("unable to normalize group-resource %s from label %q, skipping: %v", groupRes.String(), lbl, err)
-						continue
-					}
-
-					groupRes = info.GroupResource
-					resources = append(resources, groupRes)
-					updates[lbl] = groupRes
-				}
-			}
-
-			if groupRes == nsGroupResource {
-				namespaced = true
-			}
-		}
-	}()
-
-	// update the cache for next time.  This should only be called by discovery,
-	// so we don't really have to worry about the grap between read and write locks
-	// (plus, we don't care if someone else updates the cache first, since the results
-	// are necessarily the same, so at most we've done extra work).
-	if len(updates) > 0 {
-		n.labelResourceMu.Lock()
-		defer n.labelResourceMu.Unlock()
-
-		for lbl, groupRes := range updates {
-			n.labelToResource[lbl] = groupRes
-		}
-	}
-
-	return resources, namespaced
-}
-
-func (n *metricNamer) LabelForResource(resource schema.GroupResource) (pmodel.LabelName, error) {
-	n.labelResourceMu.RLock()
-	// check if we have a cached copy or override
-	lbl, ok := n.resourceToLabel[resource]
-	n.labelResourceMu.RUnlock() // release before we call makeLabelForResource
-	if ok {
-		return lbl, nil
-	}
-
-	// NB: we don't actually care about the gap between releasing read lock
-	// and acquiring the write lock -- if we do duplicate work sometimes, so be
-	// it, as long as we're correct.
-
-	// otherwise, use the template and save the result
-	lbl, err := n.makeLabelForResource(resource)
-	if err != nil {
-		return "", fmt.Errorf("unable to convert resource %s into label: %v", resource.String(), err)
-	}
-	return lbl, nil
-}
-
-// makeLabelForResource constructs a label name for the given resource, and saves the result.
-// It must *not* be called under an existing lock.
-func (n *metricNamer) makeLabelForResource(resource schema.GroupResource) (pmodel.LabelName, error) {
-	if n.labelTemplate == nil {
-		return "", fmt.Errorf("no generic resource label form specified for this metric")
-	}
-	buff := new(bytes.Buffer)
-
-	singularRes, err := n.mapper.ResourceSingularizer(resource.Resource)
-	if err != nil {
-		return "", fmt.Errorf("unable to singularize resource %s: %v", resource.String(), err)
-	}
-	convResource := schema.GroupResource{
-		Group:    groupNameSanitizer.Replace(resource.Group),
-		Resource: singularRes,
-	}
-
-	if err := n.labelTemplate.Execute(buff, convResource); err != nil {
-		return "", err
-	}
-	if buff.Len() == 0 {
-		return "", fmt.Errorf("empty label produced by label template")
-	}
-	lbl := pmodel.LabelName(buff.String())
-
-	n.labelResourceMu.Lock()
-	defer n.labelResourceMu.Unlock()
-
-	n.resourceToLabel[resource] = lbl
-	n.labelToResource[lbl] = resource
-	return lbl, nil
-}
-
 func (n *metricNamer) MetricNameForSeries(series prom.Series) (string, error) {
 	matches := n.nameMatches.FindStringSubmatchIndex(series.Name)
 	if matches == nil {
@@ -376,21 +180,6 @@ func NamersFromConfig(cfg *config.MetricsDiscoveryConfig, mapper apimeta.RESTMap
 	namers := make([]MetricNamer, len(cfg.Rules))
 
 	for i, rule := range cfg.Rules {
-		var labelTemplate *template.Template
-		var labelResExtractor *labelGroupResExtractor
-		var err error
-		if rule.Resources.Template != "" {
-			labelTemplate, err = template.New("resource-label").Delims("<<", ">>").Parse(rule.Resources.Template)
-			if err != nil {
-				return nil, fmt.Errorf("unable to parse label template %q associated with series query %q: %v", rule.Resources.Template, rule.SeriesQuery, err)
-			}
-
-			labelResExtractor, err = newLabelGroupResExtractor(labelTemplate)
-			if err != nil {
-				return nil, fmt.Errorf("unable to generate label format from template %q associated with series query %q: %v", rule.Resources.Template, rule.SeriesQuery, err)
-			}
-		}
-
 		metricsQueryTemplate, err := template.New("metrics-query").Delims("<<", ">>").Parse(rule.MetricsQuery)
 		if err != nil {
 			return nil, fmt.Errorf("unable to parse metrics query template %q associated with series query %q: %v", rule.MetricsQuery, rule.SeriesQuery, err)
@@ -437,35 +226,18 @@ func NamersFromConfig(cfg *config.MetricsDiscoveryConfig, mapper apimeta.RESTMap
 			}
 		}
 
+		resConv, err := naming.NewResourceConverter(rule.Resources.Template, rule.Resources.Overrides, mapper)
+		if err != nil {
+			return nil, err
+		}
+
 		namer := &metricNamer{
 			seriesQuery:          prom.Selector(rule.SeriesQuery),
-			labelTemplate:        labelTemplate,
-			labelResExtractor:    labelResExtractor,
 			metricsQueryTemplate: metricsQueryTemplate,
-			mapper:               mapper,
 			nameMatches:          nameMatches,
 			nameAs:               nameAs,
 			seriesMatchers:       seriesMatchers,
-
+			ResourceConverter:    resConv,
-			labelToResource: make(map[pmodel.LabelName]schema.GroupResource),
-			resourceToLabel: make(map[schema.GroupResource]pmodel.LabelName),
-		}
-
-		// invert the structure for consistency with the template
-		for lbl, groupRes := range rule.Resources.Overrides {
-			infoRaw := provider.CustomMetricInfo{
-				GroupResource: schema.GroupResource{
-					Group:    groupRes.Group,
-					Resource: groupRes.Resource,
-				},
-			}
-			info, _, err := infoRaw.Normalized(mapper)
-			if err != nil {
-				return nil, fmt.Errorf("unable to normalize group-resource %v: %v", groupRes, err)
-			}
-
-			namer.labelToResource[pmodel.LabelName(lbl)] = info.GroupResource
-			namer.resourceToLabel[info.GroupResource] = pmodel.LabelName(lbl)
 		}
 
 		namers[i] = namer

pkg/naming/lbl_res.go

@@ -0,0 +1,83 @@
+package naming
+
+import (
+	"bytes"
+	"fmt"
+	"regexp"
+	"text/template"
+
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+
+	pmodel "github.com/prometheus/common/model"
+)
+
+// labelGroupResExtractor extracts schema.GroupResources from series labels.
+type labelGroupResExtractor struct {
+	regex *regexp.Regexp
+
+	resourceInd int
+	groupInd    *int
+	mapper      apimeta.RESTMapper
+}
+
+// newLabelGroupResExtractor creates a new labelGroupResExtractor for labels whose form
+// matches the given template.  It does so by creating a regular expression from the template,
+// so anything in the template which limits resource or group name length will cause issues.
+func newLabelGroupResExtractor(labelTemplate *template.Template) (*labelGroupResExtractor, error) {
+	labelRegexBuff := new(bytes.Buffer)
+	if err := labelTemplate.Execute(labelRegexBuff, schema.GroupResource{"(?P<group>.+?)", "(?P<resource>.+?)"}); err != nil {
+		return nil, fmt.Errorf("unable to convert label template to matcher: %v", err)
+	}
+	if labelRegexBuff.Len() == 0 {
+		return nil, fmt.Errorf("unable to convert label template to matcher: empty template")
+	}
+	labelRegexRaw := "^" + labelRegexBuff.String() + "$"
+	labelRegex, err := regexp.Compile(labelRegexRaw)
+	if err != nil {
+		return nil, fmt.Errorf("unable to convert label template to matcher: %v", err)
+	}
+
+	var groupInd *int
+	var resInd *int
+
+	for i, name := range labelRegex.SubexpNames() {
+		switch name {
+		case "group":
+			ind := i // copy to avoid iteration variable reference
+			groupInd = &ind
+		case "resource":
+			ind := i // copy to avoid iteration variable reference
+			resInd = &ind
+		}
+	}
+
+	if resInd == nil {
+		return nil, fmt.Errorf("must include at least `{{.Resource}}` in the label template")
+	}
+
+	return &labelGroupResExtractor{
+		regex:       labelRegex,
+		resourceInd: *resInd,
+		groupInd:    groupInd,
+	}, nil
+}
+
+// GroupResourceForLabel extracts a schema.GroupResource from the given label, if possible.
+// The second return value indicates whether or not a potential group-resource was found in this label.
+func (e *labelGroupResExtractor) GroupResourceForLabel(lbl pmodel.LabelName) (schema.GroupResource, bool) {
+	matchGroups := e.regex.FindStringSubmatch(string(lbl))
+	if matchGroups != nil {
+		group := ""
+		if e.groupInd != nil {
+			group = matchGroups[*e.groupInd]
+		}
+
+		return schema.GroupResource{
+			Group:    group,
+			Resource: matchGroups[e.resourceInd],
+		}, true
+	}
+
+	return schema.GroupResource{}, false
+}

pkg/naming/resource_converter.go

@@ -0,0 +1,200 @@
+package naming
+
+import (
+	"bytes"
+	"fmt"
+	"strings"
+	"sync"
+	"text/template"
+
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+
+	prom "github.com/directxman12/k8s-prometheus-adapter/pkg/client"
+	"github.com/directxman12/k8s-prometheus-adapter/pkg/config"
+	"github.com/golang/glog"
+	"github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider"
+	pmodel "github.com/prometheus/common/model"
+)
+
+var (
+	groupNameSanitizer = strings.NewReplacer(".", "_", "-", "_")
+	nsGroupResource    = schema.GroupResource{Resource: "namespaces"}
+)
+
+// ResourceConverter knows the relationship between Kubernetes group-resources and Prometheus labels,
+// and can convert between the two for any given label or series.
+type ResourceConverter interface {
+	// ResourcesForSeries returns the group-resources associated with the given series,
+	// as well as whether or not the given series has the "namespace" resource).
+	ResourcesForSeries(series prom.Series) (res []schema.GroupResource, namespaced bool)
+	// LabelForResource returns the appropriate label for the given resource.
+	LabelForResource(resource schema.GroupResource) (pmodel.LabelName, error)
+}
+
+type resourceConverter struct {
+	labelResourceMu   sync.RWMutex
+	labelToResource   map[pmodel.LabelName]schema.GroupResource
+	resourceToLabel   map[schema.GroupResource]pmodel.LabelName
+	labelResExtractor *labelGroupResExtractor
+	mapper            apimeta.RESTMapper
+	labelTemplate     *template.Template
+}
+
+// NewResourceConverter creates a ResourceConverter based on a generic template plus any overrides.
+// Either overrides or the template may be empty, but not both.
+func NewResourceConverter(resourceTemplate string, overrides map[string]config.GroupResource, mapper apimeta.RESTMapper) (ResourceConverter, error) {
+	converter := &resourceConverter{
+		labelToResource: make(map[pmodel.LabelName]schema.GroupResource),
+		resourceToLabel: make(map[schema.GroupResource]pmodel.LabelName),
+		mapper:          mapper,
+	}
+
+	if resourceTemplate != "" {
+		labelTemplate, err := template.New("resource-label").Delims("<<", ">>").Parse(resourceTemplate)
+		if err != nil {
+			return converter, fmt.Errorf("unable to parse label template %q: %v", resourceTemplate, err)
+		}
+		converter.labelTemplate = labelTemplate
+
+		labelResExtractor, err := newLabelGroupResExtractor(labelTemplate)
+		if err != nil {
+			return converter, fmt.Errorf("unable to generate label format from template %q: %v", resourceTemplate, err)
+		}
+		converter.labelResExtractor = labelResExtractor
+	}
+
+	// invert the structure for consistency with the template
+	for lbl, groupRes := range overrides {
+		infoRaw := provider.CustomMetricInfo{
+			GroupResource: schema.GroupResource{
+				Group:    groupRes.Group,
+				Resource: groupRes.Resource,
+			},
+		}
+		info, _, err := infoRaw.Normalized(converter.mapper)
+		if err != nil {
+			return nil, fmt.Errorf("unable to normalize group-resource %v: %v", groupRes, err)
+		}
+
+		converter.labelToResource[pmodel.LabelName(lbl)] = info.GroupResource
+		converter.resourceToLabel[info.GroupResource] = pmodel.LabelName(lbl)
+	}
+
+	return converter, nil
+}
+
+func (r *resourceConverter) LabelForResource(resource schema.GroupResource) (pmodel.LabelName, error) {
+	r.labelResourceMu.RLock()
+	// check if we have a cached copy or override
+	lbl, ok := r.resourceToLabel[resource]
+	r.labelResourceMu.RUnlock() // release before we call makeLabelForResource
+	if ok {
+		return lbl, nil
+	}
+
+	// NB: we don't actually care about the gap between releasing read lock
+	// and acquiring the write lock -- if we do duplicate work sometimes, so be
+	// it, as long as we're correct.
+
+	// otherwise, use the template and save the result
+	lbl, err := r.makeLabelForResource(resource)
+	if err != nil {
+		return "", fmt.Errorf("unable to convert resource %s into label: %v", resource.String(), err)
+	}
+	return lbl, nil
+}
+
+// makeLabelForResource constructs a label name for the given resource, and saves the result.
+// It must *not* be called under an existing lock.
+func (r *resourceConverter) makeLabelForResource(resource schema.GroupResource) (pmodel.LabelName, error) {
+	if r.labelTemplate == nil {
+		return "", fmt.Errorf("no generic resource label form specified for this metric")
+	}
+	buff := new(bytes.Buffer)
+
+	singularRes, err := r.mapper.ResourceSingularizer(resource.Resource)
+	if err != nil {
+		return "", fmt.Errorf("unable to singularize resource %s: %v", resource.String(), err)
+	}
+	convResource := schema.GroupResource{
+		Group:    groupNameSanitizer.Replace(resource.Group),
+		Resource: singularRes,
+	}
+
+	if err := r.labelTemplate.Execute(buff, convResource); err != nil {
+		return "", err
+	}
+	if buff.Len() == 0 {
+		return "", fmt.Errorf("empty label produced by label template")
+	}
+	lbl := pmodel.LabelName(buff.String())
+
+	r.labelResourceMu.Lock()
+	defer r.labelResourceMu.Unlock()
+
+	r.resourceToLabel[resource] = lbl
+	r.labelToResource[lbl] = resource
+	return lbl, nil
+}
+
+func (r *resourceConverter) ResourcesForSeries(series prom.Series) ([]schema.GroupResource, bool) {
+	// use an updates map to avoid having to drop the read lock to update the cache
+	// until the end.  Since we'll probably have few updates after the first run,
+	// this should mean that we rarely have to hold the write lock.
+	var resources []schema.GroupResource
+	updates := make(map[pmodel.LabelName]schema.GroupResource)
+	namespaced := false
+
+	// use an anon func to get the right defer behavior
+	func() {
+		r.labelResourceMu.RLock()
+		defer r.labelResourceMu.RUnlock()
+
+		for lbl := range series.Labels {
+			var groupRes schema.GroupResource
+			var ok bool
+
+			// check if we have an override
+			if groupRes, ok = r.labelToResource[lbl]; ok {
+				resources = append(resources, groupRes)
+			} else if groupRes, ok = updates[lbl]; ok {
+				resources = append(resources, groupRes)
+			} else if r.labelResExtractor != nil {
+				// if not, check if it matches the form we expect, and if so,
+				// convert to a group-resource.
+				if groupRes, ok = r.labelResExtractor.GroupResourceForLabel(lbl); ok {
+					info, _, err := provider.CustomMetricInfo{GroupResource: groupRes}.Normalized(r.mapper)
+					if err != nil {
+						// this is likely to show up for a lot of labels, so make it a verbose info log
+						glog.V(9).Infof("unable to normalize group-resource %s from label %q, skipping: %v", groupRes.String(), lbl, err)
+						continue
+					}
+
+					groupRes = info.GroupResource
+					resources = append(resources, groupRes)
+					updates[lbl] = groupRes
+				}
+			}
+
+			if groupRes == nsGroupResource {
+				namespaced = true
+			}
+		}
+	}()
+
+	// update the cache for next time.  This should only be called by discovery,
+	// so we don't really have to worry about the gap between read and write locks
+	// (plus, we don't care if someone else updates the cache first, since the results
+	// are necessarily the same, so at most we've done extra work).
+	if len(updates) > 0 {
+		r.labelResourceMu.Lock()
+		defer r.labelResourceMu.Unlock()
+
+		for lbl, groupRes := range updates {
+			r.labelToResource[lbl] = groupRes
+		}
+	}
+
+	return resources, namespaced
+}