From 87be1b491e0f0298f1e96941ed93daa6c70c3011 Mon Sep 17 00:00:00 2001 From: Guilherme Santos <157053549+gsantos-hc@users.noreply.github.com> Date: Mon, 2 Dec 2024 10:19:40 -0500 Subject: [PATCH 1/2] feat(metrics): add requests and errors counters Add counters for the number of requests processed and the number of errors encountered. --- agent-inject/handler.go | 14 +++++++++++++- agent-inject/metrics.go | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/agent-inject/handler.go b/agent-inject/handler.go index f30282cb..6941527b 100644 --- a/agent-inject/handler.go +++ b/agent-inject/handler.go @@ -5,6 +5,7 @@ package agent_inject import ( "encoding/json" + "errors" "fmt" "io" "net/http" @@ -85,6 +86,7 @@ type Handler struct { // served via an HTTP server. func (h *Handler) Handle(w http.ResponseWriter, r *http.Request) { h.Log.Info("Request received", "Method", r.Method, "URL", r.URL) + var procErr error // Measure request processing duration and monitor request queue requestQueue.Inc() @@ -92,12 +94,14 @@ func (h *Handler) Handle(w http.ResponseWriter, r *http.Request) { defer func() { requestProcessingTime.Observe(float64(time.Since(requestStart).Milliseconds())) requestQueue.Dec() + incrementRequests(procErr) }() if ct := r.Header.Get("Content-Type"); ct != "application/json" { msg := fmt.Sprintf("Invalid content-type: %q", ct) http.Error(w, msg, http.StatusBadRequest) h.Log.Warn("warning for request", "Warn", msg, "Code", http.StatusBadRequest) + procErr = errors.New(msg) return } @@ -108,6 +112,7 @@ func (h *Handler) Handle(w http.ResponseWriter, r *http.Request) { msg := fmt.Sprintf("error reading request body: %s", err) http.Error(w, msg, http.StatusBadRequest) h.Log.Error("error on request", "Error", msg, "Code", http.StatusBadRequest) + procErr = errors.New(msg) return } } @@ -115,6 +120,7 @@ func (h *Handler) Handle(w http.ResponseWriter, r *http.Request) { msg := "Empty request body" http.Error(w, msg, http.StatusBadRequest) h.Log.Error("warning for request", "Warn", msg, "Code", http.StatusBadRequest) + procErr = errors.New(strings.ToLower(msg)) return } @@ -136,6 +142,7 @@ func (h *Handler) Handle(w http.ResponseWriter, r *http.Request) { msg := fmt.Sprintf("error decoding admission request: %s", err) http.Error(w, msg, http.StatusInternalServerError) h.Log.Error("error on request", "Error", msg, "Code", http.StatusInternalServerError) + procErr = errors.New(msg) return } else { mutateResp = h.Mutate(admReq.Request) @@ -156,12 +163,15 @@ func (h *Handler) Handle(w http.ResponseWriter, r *http.Request) { http.Error(w, msg, http.StatusInternalServerError) h.Log.Error("error on request", "Error", msg, "Code", http.StatusInternalServerError) incrementInjectionFailures(admReq.Request.Namespace) + procErr = errors.New(msg) return } if _, err := w.Write(resp); err != nil { - h.Log.Error("error writing response", "Error", err) + msg := "error writing response" + h.Log.Error(msg, "Error", err) incrementInjectionFailures(admReq.Request.Namespace) + procErr = errors.New(msg) return } @@ -170,6 +180,8 @@ func (h *Handler) Handle(w http.ResponseWriter, r *http.Request) { } else { incrementInjectionFailures(admReq.Request.Namespace) } + + return } type MutateResponse struct { diff --git a/agent-inject/metrics.go b/agent-inject/metrics.go index b2d9c24f..7f8745b5 100644 --- a/agent-inject/metrics.go +++ b/agent-inject/metrics.go @@ -18,6 +18,20 @@ const ( ) var ( + requestsReceived = prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: metricsNamespace, + Subsystem: metricsSubsystem, + Name: "requests_received_total", + Help: "Count of webhook requests received by the injector", + }) + + requestsErrored = prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: metricsNamespace, + Subsystem: metricsSubsystem, + Name: "requests_errored_total", + Help: "Count of webhook requests that could not be processed by the injector", + }) + requestQueue = prometheus.NewGauge(prometheus.GaugeOpts{ Namespace: metricsNamespace, Subsystem: metricsSubsystem, @@ -48,6 +62,13 @@ var ( }, []string{metricsLabelNamespace}) ) +func incrementRequests(err error) { + requestsReceived.Inc() + if err != nil { + requestsErrored.Inc() + } +} + func incrementInjections(namespace string, res MutateResponse) { // Injection type can be one of: init_and_sidecar (default); init_only; or sidecar_only typeLabel := metricsLabelTypeBoth @@ -69,6 +90,8 @@ func incrementInjectionFailures(namespace string) { func MustRegisterInjectorMetrics(registry prometheus.Registerer) { registry.MustRegister( + requestsReceived, + requestsErrored, requestQueue, requestProcessingTime, injectionsByNamespace, From 015b00217b6f9f52aeb5f17cdf45837811ac091d Mon Sep 17 00:00:00 2001 From: Guilherme Santos <157053549+gsantos-hc@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:07:45 -0500 Subject: [PATCH 2/2] test(metrics): unit test for incrementRequests --- agent-inject/metrics_test.go | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/agent-inject/metrics_test.go b/agent-inject/metrics_test.go index 65101877..94fa1a2f 100644 --- a/agent-inject/metrics_test.go +++ b/agent-inject/metrics_test.go @@ -4,6 +4,7 @@ package agent_inject import ( + "errors" "testing" "github.com/prometheus/client_golang/prometheus" @@ -12,7 +13,8 @@ import ( ) func Test_incrementInjections(t *testing.T) { - MustRegisterInjectorMetrics(prometheus.DefaultRegisterer) + reg := prometheus.NewRegistry() + MustRegisterInjectorMetrics(reg) tests := map[string]struct { namespace string @@ -65,3 +67,33 @@ func Test_incrementInjections(t *testing.T) { }) } } + +func Test_incrementRequests(t *testing.T) { + one := 1.0 + reg := prometheus.NewRegistry() + MustRegisterInjectorMetrics(reg) + + tests := map[string]struct { + err error + }{ + "valid_request": {err: nil}, + "invalid_content_type": {err: errors.New("Invalid content-type: ")}, + "error_reading_body": {err: errors.New("error reading request body: ")}, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + // Unlike CounterVec, Counter does not have a Reset() method. As a workaround, we can + // collect the before and after counts and assert that the difference is 0 or 1, as + // applicable. + reqsExpected := testutil.ToFloat64(requestsReceived) + one + errsExpected := testutil.ToFloat64(requestsErrored) + if test.err != nil { + errsExpected += one + } + + incrementRequests(test.err) + assert.Equal(t, reqsExpected, testutil.ToFloat64(requestsReceived)) + assert.Equal(t, errsExpected, testutil.ToFloat64(requestsErrored)) + }) + } +}