apply review notes. Add status to requests_total metric

internal/runtime/client/client.go

@@ -137,9 +137,6 @@ func (c *client) Discover(ctx context.Context, extensionConfig *runtimev1.Extens
 		return nil, errors.Wrapf(err, "failed to discover extension %q", extensionConfig.Name)
 	}
 
-	// Create response metric.
-	runtimemetrics.ResponseTotal.Observe(response, hookGVH)
-
 	// Check to see if the response is a failure and handle the failure accordingly.
 	if response.GetStatus() == runtimehooksv1.ResponseStatusFailure {
 		log.Info(fmt.Sprintf("failed to discover extension %q: got failure response with message %v", extensionConfig.Name, response.GetMessage()))
@@ -354,9 +351,6 @@ func (c *client) CallExtension(ctx context.Context, hook runtimecatalog.Hook, fo
 		return errors.Wrapf(err, "failed to call extension handler %q", name)
 	}
 
-	// Create response metric.
-	runtimemetrics.ResponseTotal.Observe(response, hookGVH)
-
 	// If the received response is a failure then return an error.
 	if response.GetStatus() == runtimehooksv1.ResponseStatusFailure {
 		log.Info(fmt.Sprintf("failed to call extension handler %q: got failure response with message %v", name, response.GetMessage()))
@@ -494,8 +488,13 @@ func httpCall(ctx context.Context, request, response runtime.Object, opts *httpC
 	})
 
 	resp, err := client.Do(httpRequest)
+	respStatus := "Unknown"
+
 	// Create http request metric.
-	runtimemetrics.RequestsTotal.Observe(httpRequest, resp, opts.hookGVH, err)
+	defer func() {
+		runtimemetrics.RequestsTotal.Observe(httpRequest, resp, opts.hookGVH, err, respStatus)
+	}()
+
 	if err != nil {
 		return errCallingExtensionHandler(
 			errors.Wrapf(err, "http call failed"),
@@ -530,6 +529,10 @@ func httpCall(ctx context.Context, request, response runtime.Object, opts *httpC
 		}
 	}
 
+	if responseObject, ok := response.(runtimehooksv1.ResponseObject); ok {
+		respStatus = string(responseObject.GetStatus())
+	}
+
 	return nil
 }
 

internal/runtime/metrics/metrics.go

@@ -27,14 +27,12 @@ import (
 	ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
 
 	runtimecatalog "sigs.k8s.io/cluster-api/exp/runtime/catalog"
-	runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
 )
 
 func init() {
 	// Register the metrics at the controller-runtime metrics registry.
 	ctrlmetrics.Registry.MustRegister(RequestsTotal.metric)
 	ctrlmetrics.Registry.MustRegister(RequestDuration.metric)
-	ctrlmetrics.Registry.MustRegister(ResponseTotal.metric)
 }
 
 // Metrics subsystem and all of the keys used by the Runtime SDK.
@@ -48,8 +46,8 @@ var (
 		prometheus.NewCounterVec(prometheus.CounterOpts{
 			Subsystem: runtimeSDKSubsystem,
 			Name:      "requests_total",
-			Help:      "Number of HTTP requests, partitioned by status code, host and hook.",
-		}, []string{"code", "host", "group", "version", "hook"}),
+			Help:      "Number of HTTP requests, partitioned by code, status, host and hook.",
+		}, []string{"code", "host", "group", "version", "hook", "status"}),
 	}
 	// RequestDuration reports the request latency in seconds.
 	RequestDuration = requestDurationObserver{
@@ -60,33 +58,15 @@ var (
 			Buckets:   prometheus.ExponentialBuckets(0.001, 2, 10),
 		}, []string{"host", "group", "version", "hook"}),
 	}
-	// ResponseTotal reports response results.
-	ResponseTotal = responseTotalObserver{
-		prometheus.NewCounterVec(prometheus.CounterOpts{
-			Subsystem: runtimeSDKSubsystem,
-			Name:      "response_total",
-			Help:      "Number of responses, partitioned by status and hook.",
-		}, []string{"status", "group", "version", "hook"}),
-	}
 )
 
-type responseTotalObserver struct {
-	metric *prometheus.CounterVec
-}
-
-// Observe observes a response and increments the metric for the given
-// response status and gvh.
-func (m *responseTotalObserver) Observe(resp runtimehooksv1.ResponseObject, gvh runtimecatalog.GroupVersionHook) {
-	m.metric.WithLabelValues(string(resp.GetStatus()), gvh.Group, gvh.Version, gvh.Hook).Inc()
-}
-
 type requestsTotalObserver struct {
 	metric *prometheus.CounterVec
 }
 
 // Observe observes a http request result and increments the metric for the given
-// error status code, host and gvh.
-func (m *requestsTotalObserver) Observe(req *http.Request, resp *http.Response, gvh runtimecatalog.GroupVersionHook, err error) {
+// error status, code, host and gvh.
+func (m *requestsTotalObserver) Observe(req *http.Request, resp *http.Response, gvh runtimecatalog.GroupVersionHook, err error, status string) {
 	host := req.URL.Host
 
 	// Errors can be arbitrary strings. Unbound label cardinality is not suitable for a metric
@@ -95,7 +75,7 @@ func (m *requestsTotalObserver) Observe(req *http.Request, resp *http.Response,
 	if err == nil {
 		code = strconv.Itoa(resp.StatusCode)
 	}
-	m.metric.WithLabelValues(code, host, gvh.Group, gvh.Version, gvh.Hook).Inc()
+	m.metric.WithLabelValues(code, host, gvh.Group, gvh.Version, gvh.Hook, status).Inc()
 }
 
 type requestDurationObserver struct {