recommender: fix instrumentation and add tests

controllers/datadoghq/metrics.go

@@ -29,7 +29,10 @@ const (
 	transitionPromLabel        = "transition"
 	lifecycleStatus            = "lifecycle_status"
 	monitorName                = "monitor_name"
-	monitorNamespace           = "monotor_namespace"
+	monitorNamespace           = "monitor_namespace"
+	clientPromLabel            = "client"
+	methodPromLabel            = "method"
+	codePromLabel              = "code"
 	// Label values
 	downscaleCappingPromLabelVal = "downscale_capping"
 	upscaleCappingPromLabelVal   = "upscale_capping"
@@ -263,19 +266,19 @@ var (
 			Help:    "Tracks the latencies for HTTP requests.",
 			Buckets: []float64{0.1, 0.3, 0.6, 1, 3, 6, 9, 20},
 		},
-		[]string{"client", "method", "code"},
+		[]string{clientPromLabel, methodPromLabel, codePromLabel},
 	)
 	requestsTotal = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "http_client_requests_total",
 			Help: "Tracks the number of HTTP requests.",
-		}, []string{"client", "method", "code"},
+		}, []string{clientPromLabel, methodPromLabel, codePromLabel},
 	)
 	responseInflight = prometheus.NewGaugeVec(
 		prometheus.GaugeOpts{
 			Name: "http_client_requests_inflight",
 			Help: "Tracks the number of client requests currently in progress.",
-		}, []string{"client"},
+		}, []string{clientPromLabel},
 	)
 )
 

controllers/datadoghq/recommender.go

@@ -82,7 +82,7 @@ func (r *RecommenderClientImpl) instrumentedClient(recommender string) *http.Cli
 }
 
 func instrumentRoundTripper(recommender string, transport http.RoundTripper) http.RoundTripper {
-	labels := prometheus.Labels{"recommender": recommender}
+	labels := prometheus.Labels{clientPromLabel: recommender}
 
 	return promhttp.InstrumentRoundTripperCounter(
 		requestsTotal.MustCurryWith(labels),
@@ -103,26 +103,26 @@ func instrumentRoundTripper(recommender string, transport http.RoundTripper) htt
 func (r *RecommenderClientImpl) GetReplicaRecommendation(request *ReplicaRecommendationRequest) (*ReplicaRecommendationResponse, error) {
 	reco := request.Recommender
 	if reco == nil {
-		return &ReplicaRecommendationResponse{}, fmt.Errorf("recommender spec is required")
+		return nil, fmt.Errorf("recommender spec is required")
 	}
 
 	u, err := url.Parse(reco.URL)
 	if err != nil {
-		return &ReplicaRecommendationResponse{}, fmt.Errorf("error parsing url: %w", err)
+		return nil, fmt.Errorf("error parsing url: %w", err)
 	}
 
 	if u.Scheme != "http" && u.Scheme != "https" {
-		return &ReplicaRecommendationResponse{}, fmt.Errorf("only http and https schemes are supported")
+		return nil, fmt.Errorf("only http and https schemes are supported")
 	}
 
 	req, err := buildWorkloadRecommendationRequest(request)
 	if err != nil {
-		return &ReplicaRecommendationResponse{}, err
+		return nil, err
 	}
 
 	payload, err := protojson.Marshal(req)
 	if err != nil {
-		return &ReplicaRecommendationResponse{}, fmt.Errorf("error marshaling request: %w", err)
+		return nil, fmt.Errorf("error marshaling request: %w", err)
 	}
 
 	// TODO: We might want to make the timeout configurable later.
@@ -136,7 +136,7 @@ func (r *RecommenderClientImpl) GetReplicaRecommendation(request *ReplicaRecomme
 	httpReq.Header.Set("User-Agent", "wpa-controller")
 
 	if err != nil {
-		return &ReplicaRecommendationResponse{}, fmt.Errorf("error creating request: %w", err)
+		return nil, fmt.Errorf("error creating request: %w", err)
 	}
 	resp, err := client.Do(httpReq)
 
@@ -147,22 +147,22 @@ func (r *RecommenderClientImpl) GetReplicaRecommendation(request *ReplicaRecomme
 	}()
 
 	if err != nil {
-		return &ReplicaRecommendationResponse{}, fmt.Errorf("error sending request: %w", err)
+		return nil, fmt.Errorf("error sending request: %w", err)
 	}
 
 	if resp.StatusCode != http.StatusOK {
-		return &ReplicaRecommendationResponse{}, fmt.Errorf("unexpected response code: %d", resp.StatusCode)
+		return nil, fmt.Errorf("unexpected response code: %d", resp.StatusCode)
 	}
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return &ReplicaRecommendationResponse{}, fmt.Errorf("error reading response: %w", err)
+		return nil, fmt.Errorf("error reading response: %w", err)
 	}
 
 	reply := &autoscaling.WorkloadRecommendationReply{}
 	err = protojson.Unmarshal(body, reply)
 	if err != nil {
-		return &ReplicaRecommendationResponse{}, fmt.Errorf("error unmarshaling response: %w", err)
+		return nil, fmt.Errorf("error unmarshaling response: %w", err)
 	}
 
 	return buildReplicaRecommendationResponse(reply)

controllers/datadoghq/recommender_test.go

@@ -5,7 +5,13 @@
 
 package datadoghq
 
-import "time"
+import (
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
 
 func NewMockRecommenderClient() *RecommenderClientMock {
 	return &RecommenderClientMock{
@@ -24,3 +30,19 @@ type RecommenderClientMock struct {
 	ReturnedResponse ReplicaRecommendationResponse
 	Error            error
 }
+
+// TODO: Add more tests for the HTTP client
+func TestRecommenderClient(t *testing.T) {
+	client := NewRecommenderClient(http.DefaultClient)
+	// This should not work with empty requests.
+	resp, err := client.GetReplicaRecommendation(&ReplicaRecommendationRequest{})
+	require.Error(t, err)
+	require.Nil(t, resp)
+}
+
+func TestInstrumentation(t *testing.T) {
+	client := http.DefaultClient
+	client.Transport = instrumentRoundTripper("http://test", http.DefaultTransport)
+	// This simply makes sure the instrumentation does crash.
+	_, _ = client.Get("fake")
+}