diff --git a/pkg/custom-provider/metric_namer.go b/pkg/custom-provider/metric_namer.go index f65a115e..e65799a7 100644 --- a/pkg/custom-provider/metric_namer.go +++ b/pkg/custom-provider/metric_namer.go @@ -84,7 +84,7 @@ func (r *basicSeriesRegistry) SetSeries(newSeries []prom.Series) error { if strings.HasPrefix(series.Name, "container_") { r.namer.processContainerSeries(series, newInfo) } else if namespaceLabel, hasNamespaceLabel := series.Labels["namespace"]; hasNamespaceLabel && namespaceLabel != "" { - // TODO: handle metrics describing a namespace + // we also handle namespaced metrics here as part of the resource-association logic if err := r.namer.processNamespacedSeries(series, newInfo); err != nil { glog.Errorf("Unable to process namespaced series %q: %v", series.Name, err) continue @@ -123,7 +123,8 @@ func (r *basicSeriesRegistry) QueryForMetric(metricInfo provider.MetricInfo, nam defer r.mu.RUnlock() if len(resourceNames) == 0 { - // TODO: return error? panic? + glog.Errorf("no resource names requested while producing a query for metric %s", metricInfo.String()) + return 0, "", "", false } metricInfo, singularResource, err := r.namer.normalizeInfo(metricInfo) @@ -214,7 +215,7 @@ type seriesSpec struct { // normalizeInfo takes in some metricInfo an "normalizes" it to ensure a common GroupResource form. func (r *metricNamer) normalizeInfo(metricInfo provider.MetricInfo) (provider.MetricInfo, string, error) { // NB: we need to "normalize" the metricInfo's GroupResource so we have a consistent pluralization, etc - // TODO: move this to the boilerplate? + // TODO: move this to the boilerplate normalizedGroupRes, err := r.mapper.ResourceFor(metricInfo.GroupResource.WithVersion("")) if err != nil { return provider.MetricInfo{}, "", err @@ -247,7 +248,6 @@ func (n *metricNamer) processContainerSeries(series prom.Series, infos map[provi } info := provider.MetricInfo{ - // TODO: is the plural correct? GroupResource: schema.GroupResource{Resource: "pods"}, Namespaced: true, Metric: name, @@ -325,11 +325,9 @@ func (n *metricNamer) processRootScopedSeries(series prom.Series, infos map[prov // would return three GroupResources: "pods", "services", and "ingresses". // Returned MetricInfo is equilavent to the "normalized" info produced by normalizeInfo. func (n *metricNamer) groupResourcesFromSeries(series prom.Series) ([]schema.GroupResource, error) { - // TODO: do we need to cache this, or is ResourceFor good enough? var res []schema.GroupResource for label := range series.Labels { // TODO: figure out a way to let people specify a fully-qualified name in label-form - // TODO: will this work when missing a group? gvr, err := n.mapper.ResourceFor(schema.GroupVersionResource{Resource: string(label)}) if err != nil { if apimeta.IsNoMatchError(err) { diff --git a/pkg/custom-provider/provider.go b/pkg/custom-provider/provider.go index d46d38da..c0e67bd3 100644 --- a/pkg/custom-provider/provider.go +++ b/pkg/custom-provider/provider.go @@ -82,7 +82,7 @@ func NewPrometheusProvider(mapper apimeta.RESTMapper, kubeClient dynamic.ClientP SeriesRegistry: &basicSeriesRegistry{ namer: metricNamer{ - // TODO: populate this... + // TODO: populate the overrides list overrides: nil, mapper: mapper, }, @@ -133,8 +133,6 @@ func (p *prometheusProvider) metricsFor(valueSet pmodel.Vector, info provider.Me } res := []custom_metrics.MetricValue{} - // blech, EachListItem should pass an index -- - // it's an implementation detail that it happens to be sequential err := apimeta.EachListItem(list, func(item runtime.Object) error { objUnstructured := item.(*unstructured.Unstructured) objName := objUnstructured.GetName() @@ -173,7 +171,7 @@ func (p *prometheusProvider) buildQuery(info provider.MetricInfo, namespace stri fullQuery = prom.Selector(prom.Selector(fmt.Sprintf("rate(%s[%s])", baseQuery, pmodel.Duration(p.rateInterval).String()))) } - // TODO: too small of a rate interval will return no results... + // NB: too small of a rate interval will return no results... // sum over all other dimensions of this query (e.g. if we select on route, sum across all pods, // but if we select on pods, sum across all routes), and split by the dimension of our resource @@ -183,7 +181,6 @@ func (p *prometheusProvider) buildQuery(info provider.MetricInfo, namespace stri // TODO: use an actual context queryResults, err := p.promClient.Query(context.Background(), pmodel.Now(), fullQuery) if err != nil { - // TODO: interpret this somehow? glog.Errorf("unable to fetch metrics from prometheus: %v", err) // don't leak implementation details to the user return nil, apierr.NewInternalError(fmt.Errorf("unable to fetch metrics")) @@ -214,29 +211,24 @@ func (p *prometheusProvider) getSingle(info provider.MetricInfo, namespace, name func (p *prometheusProvider) getMultiple(info provider.MetricInfo, namespace string, selector labels.Selector) (*custom_metrics.MetricValueList, error) { // construct a client to list the names of objects matching the label selector - // TODO: figure out version? client, err := p.kubeClient.ClientForGroupVersionResource(info.GroupResource.WithVersion("")) if err != nil { glog.Errorf("unable to construct dynamic client to list matching resource names: %v", err) - // TODO: check for resource not found error? // don't leak implementation details to the user return nil, apierr.NewInternalError(fmt.Errorf("unable to list matching resources")) } // we can construct a this APIResource ourself, since the dynamic client only uses Name and Namespaced - // TODO: use discovery information instead apiRes := &metav1.APIResource{ Name: info.GroupResource.Resource, Namespaced: info.Namespaced, } // actually list the objects matching the label selector - // TODO: work for objects not in core v1 matchingObjectsRaw, err := client.Resource(apiRes, namespace). List(metav1.ListOptions{LabelSelector: selector.String()}) if err != nil { glog.Errorf("unable to list matching resource names: %v", err) - // TODO: check for resource not found error? // don't leak implementation details to the user return nil, apierr.NewInternalError(fmt.Errorf("unable to list matching resources")) } @@ -323,9 +315,6 @@ func (l *cachingMetricsLister) RunUntil(stopChan <-chan struct{}) { func (l *cachingMetricsLister) updateMetrics() error { startTime := pmodel.Now().Add(-1 * l.updateInterval) - // TODO: figure out a good way to add all Kubernetes-related metrics at once - // (i.e. how do we determine if something is a Kubernetes-related metric?) - // container-specific metrics from cAdvsior have their own form, and need special handling containerSel := prom.MatchSeries("", prom.NameMatches("^container_.*"), prom.LabelNeq("container_name", "POD"), prom.LabelNeq("namespace", ""), prom.LabelNeq("pod_name", "")) namespacedSel := prom.MatchSeries("", prom.LabelNeq("namespace", ""), prom.NameNotMatches("^container_.*"))