review metrics

command/serve.go

@@ -7,11 +7,12 @@ import (
 	"gitea.illuad.fr/adrien/middleman"
 	"gitea.illuad.fr/adrien/middleman/flag"
 	"gitea.illuad.fr/adrien/middleman/pkg/fastrp"
+	"gitea.illuad.fr/adrien/middleman/pkg/metrics"
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/rs/zerolog/log"
 	"github.com/urfave/cli/v3"
 	"golang.org/x/sync/errgroup"
+	"io"
 	"net"
 	"net/http"
 	"net/netip"
@@ -109,6 +110,7 @@ var (
 // Serve describes the serve command.
 func Serve(group *errgroup.Group) *cli.Command {
 	s.group = group
+	metrics.RegisterPrometheus()
 	return &cli.Command{
 		Name:        ServeCmd,
 		Aliases:     middleman.PluckAlias(ServeCmd, ServeCmd),
@@ -133,56 +135,46 @@ func Serve(group *errgroup.Group) *cli.Command {
 	}
 }
 
-// https://www.civo.com/learn/build-your-own-prometheus-exporter-in-go
 func (ph *ProxyHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	middleman.RequestHandled.Inc()
-	timer := prometheus.NewTimer(middleman.ProcessingRequest.WithLabelValues(r.URL.Path))
+	host, _, _ := net.SplitHostPort(r.RemoteAddr)
+	metrics.IncomingHTTPRequestCounterVec.WithLabelValues(host, r.Method, r.URL.Path).Inc()
+	timer := prometheus.NewTimer(metrics.ProcessingDurationHistogramVec.WithLabelValues(host, r.Method, r.URL.Path))
 	defer timer.ObserveDuration()
-	// metric: incoming request handled
 	log.Debug().Str("remote_addr", r.RemoteAddr).Str("method", r.Method).Str("path", r.URL.Path).Msg("incoming request")
 	if mr, ok := containerMethodRegex["*"]; ok {
-		// metric: wildcard container match
-		if code := ph.checkMethodAndRegex(mr, r, ""); code != http.StatusOK {
-			// metric: request failed to match the regex for wildcard container
+		if code := ph.checkMethodAndRegex(mr, r, host); code != http.StatusOK {
 			http.Error(w, http.StatusText(code), code)
 			return
 		}
-		// metric: request successfully match the regex for a wildcard container
 		ph.frp.ServeHTTP(w, r)
 		return
 	}
-	// metric: non wildcard container match
-	host, _, _ := net.SplitHostPort(r.RemoteAddr)
 	for containerName, mr := range containerMethodRegex {
 		if ph.isContainerAuthorized(containerName, host) {
 			if code := ph.checkMethodAndRegex(mr, r, containerName); code != http.StatusOK {
-				// metric: request failed to match the regex for a non wildcard container
 				http.Error(w, http.StatusText(code), code)
 				return
 			}
-			// metric: request successfully match the regex for a non wildcard container
 			ph.frp.ServeHTTP(w, r)
 			return
 		}
 	}
-	// metric: container/client? is not authorized to dial with the proxy
-	logDeniedRequest(r, http.StatusUnauthorized, "this container is not on the list of authorized ones")
+	logDeniedRequest(r, http.StatusUnauthorized, "this client is not on the list of authorized ones")
 	http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
 }
 
 func (ph *ProxyHandler) isContainerAuthorized(containerName, host string) bool {
+	start := time.Now()
 	resolvedIPs, err := net.LookupIP(containerName)
 	if err != nil {
-		// metric: failed to resolve
 		return false
 	}
+	metrics.NameResolutionDurationGauge.WithLabelValues(containerName).Set(time.Since(start).Seconds())
 	for resolvedIP := range slices.Values(resolvedIPs) {
-		if resolvedIP.Equal(net.ParseIP(host)) { // Should I check net.ParseIP do not return a nil value?
-			// metric: container is authorized
+		if resolvedIP.Equal(net.ParseIP(host)) {
 			return true
 		}
 	}
-	// metric: container is unauthorized
 	return false
 }
 
@@ -201,25 +193,25 @@ func logAuthorizedRequest(r *http.Request, containerName, message string) {
 		Int("status_code", http.StatusOK).
 		Str("status_text", http.StatusText(http.StatusOK))
 	if containerName != "" {
-		l.Str("container_name", containerName)
+		l.Str("dns_name", containerName)
 	}
 	l.Msg(message)
 }
 
-func (ph *ProxyHandler) checkMethodAndRegex(mr methodRegex, r *http.Request, containerName string) int {
+func (ph *ProxyHandler) checkMethodAndRegex(mr methodRegex, r *http.Request, client string) int {
 	req, ok := mr[r.Method]
 	if !ok {
-		middleman.ProcessedRequest.WithLabelValues(r.Method, r.URL.Path, strconv.Itoa(http.StatusMethodNotAllowed))
-		logDeniedRequest(r, http.StatusMethodNotAllowed, "this HTTP method is not in the list of those authorized for this container")
+		metrics.ProcessedHTTPRequestCounterVec.WithLabelValues(client, r.Method, r.URL.Path, strconv.Itoa(http.StatusMethodNotAllowed)).Inc()
+		logDeniedRequest(r, http.StatusMethodNotAllowed, "this HTTP method is not in the list of those authorized for this client")
 		return http.StatusMethodNotAllowed
 	}
 	if !req.MatchString(r.URL.Path) {
-		middleman.ProcessedRequest.WithLabelValues(r.Method, r.URL.Path, strconv.Itoa(http.StatusForbidden))
+		metrics.ProcessedHTTPRequestCounterVec.WithLabelValues(client, r.Method, r.URL.Path, strconv.Itoa(http.StatusForbidden)).Inc()
 		logDeniedRequest(r, http.StatusForbidden, "this path does not match any regular expression for this HTTP method")
 		return http.StatusForbidden
 	}
-	middleman.ProcessedRequest.WithLabelValues(r.Method, r.URL.Path, strconv.Itoa(http.StatusOK))
-	logAuthorizedRequest(r, containerName, "incoming request matches a registered regular expression")
+	metrics.ProcessedHTTPRequestCounterVec.WithLabelValues(client, r.Method, r.URL.Path, strconv.Itoa(http.StatusOK)).Inc()
+	logAuthorizedRequest(r, client, "incoming request matches a registered regular expression")
 	return http.StatusOK
 }
 
@@ -228,10 +220,8 @@ func (s serve) action(ctx context.Context, command *cli.Command) error {
 	if err := ctx.Err(); err != nil {
 		return err
 	}
-	registry := prometheus.NewRegistry()
 	metricsMux := http.NewServeMux()
-	metricsMux.Handle("/metrics", promhttp.HandlerFor(registry, promhttp.HandlerOpts{}))
-	registry.MustRegister(middleman.RequestHandled, middleman.ProcessingRequest, middleman.ProcessedRequest)
+	metricsMux.Handle("/metrics", metrics.PrometheusHandler())
 	metricsSrv := http.Server{
 		Addr:    ":8107",
 		Handler: metricsMux,
@@ -252,13 +242,13 @@ func (s serve) action(ctx context.Context, command *cli.Command) error {
 		return err
 	}
 	dummyURL, _ := url.Parse("http://dummy")
-	srv := &http.Server{ // #nosec: G112
+	mainSrv := &http.Server{ // #nosec: G112
 		Handler: &ProxyHandler{
 			frp: fastrp.NewRP("unix", command.String(dockerSocketPathFlagName), dummyURL),
 		},
 	}
 	s.group.Go(func() error {
-		if err = srv.Serve(l); err != nil && !errors.Is(err, http.ErrServerClosed) {
+		if err = mainSrv.Serve(l); err != nil && !errors.Is(err, http.ErrServerClosed) {
 			return err
 		}
 		return nil
@@ -288,8 +278,9 @@ func (s serve) action(ctx context.Context, command *cli.Command) error {
 			return ctx.Err()
 		})
 	}
-	mux := http.NewServeMux()
-	mux.HandleFunc(command.String(healthEndpointFlagName), func(w http.ResponseWriter, r *http.Request) {
+	healthMux := http.NewServeMux()
+	healthMux.HandleFunc(command.String(healthEndpointFlagName), func(w http.ResponseWriter, r *http.Request) {
+		start := time.Now()
 		log.Trace().Str("from", r.RemoteAddr).Msg("incoming healthcheck request")
 		if r.Method != http.MethodHead {
 			w.WriteHeader(http.StatusMethodNotAllowed)
@@ -298,10 +289,11 @@ func (s serve) action(ctx context.Context, command *cli.Command) error {
 		if err = unixDial(command.String(dockerSocketPathFlagName)); err != nil {
 			w.WriteHeader(http.StatusServiceUnavailable)
 		}
+		metrics.HealthcheckResponseTimeGauge.Set(time.Since(start).Seconds())
 	})
 	healthSrv := http.Server{
 		Addr:              ap.healthAddrPort.String(),
-		Handler:           mux,
+		Handler:           healthMux,
 		ReadTimeout:       3 * time.Second,
 		ReadHeaderTimeout: 3 * time.Second,
 		WriteTimeout:      3 * time.Second,
@@ -328,13 +320,16 @@ func (s serve) action(ctx context.Context, command *cli.Command) error {
 	<-ctx.Done()
 	shutdownCtx, cancel := context.WithTimeout(context.Background(), command.Duration(shutdownTimeoutFlagName))
 	defer cancel()
-	if err = metricsSrv.Close(); err != nil {
-		log.Err(err).Send()
+	closeServers(&metricsSrv, &healthSrv)
+	return mainSrv.Shutdown(shutdownCtx)
+}
+
+func closeServers(closers ...io.Closer) {
+	for closer := range slices.Values(closers) {
+		if err := closer.Close(); err != nil {
+			log.Err(err).Send()
+		}
 	}
-	if err = healthSrv.Close(); err != nil {
-		log.Err(err).Send()
-	}
-	return srv.Shutdown(shutdownCtx)
 }
 
 func parseAddrPorts(srvAddrPort, healthAddrPort string) (addrPorts, error) {
@@ -456,9 +451,8 @@ func addRequests() cli.Flag {
 		Category: "network",
 		Usage:    "add requests",
 		Sources:  middleman.PluckEnvVar(EnvVarPrefix, addRequestsFlagName),
-		// Local:    true, // Required to trigger the Action when this flag is set via the environment variable, see https://github.com/urfave/cli/issues/2041.
-		Value:   []string{"*:" + defaultAllowedRequests},
-		Aliases: middleman.PluckAlias(ServeCmd, addRequestsFlagName),
+		Value:    []string{"*:" + defaultAllowedRequests},
+		Aliases:  middleman.PluckAlias(ServeCmd, addRequestsFlagName),
 		Action: func(ctx context.Context, command *cli.Command, requests []string) error {
 			clear(containerMethodRegex)
 			for _, request := range requests {
@@ -486,7 +480,7 @@ func addRequests() cli.Flag {
 	}
 }
 
-func registerMethodRegex(containerName, urlRegex string, httpMethods []string) error {
+func registerMethodRegex(client, urlRegex string, httpMethods []string) error {
 	r, err := regexp.Compile(urlRegex)
 	if err != nil {
 		return err
@@ -495,10 +489,11 @@ func registerMethodRegex(containerName, urlRegex string, httpMethods []string) e
 		if _, ok := validHTTPMethods[httpMethod]; !ok {
 			return fmt.Errorf("%s is not a valid HTTP method", httpMethod)
 		}
-		if containerMethodRegex[containerName] == nil {
-			containerMethodRegex[containerName] = make(methodRegex)
+		if containerMethodRegex[client] == nil {
+			containerMethodRegex[client] = make(methodRegex)
 		}
-		containerMethodRegex[containerName][httpMethod] = r
+		containerMethodRegex[client][httpMethod] = r
+		metrics.RegisteredRegexCounterVec.WithLabelValues(client, httpMethod, r.String()).Inc()
 	}
 	return nil
 }

metric.go

@@ -1,19 +0,0 @@
-package middleman
-
-import "github.com/prometheus/client_golang/prometheus"
-
-var (
-	RequestHandled = prometheus.NewCounter(prometheus.CounterOpts{
-		Name: "http_requests_handled_total",
-		Help: "Total number of HTTP requests handled",
-	})
-	ProcessingRequest = prometheus.NewHistogramVec(prometheus.HistogramOpts{
-		Name: "time_spent_processing_request_seconds",
-		Help: "Number of seconds spent processing a request",
-	}, []string{"path"})
-	ProcessedRequest = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Name:        "http_requests_processed_total",
-		Help:        "Total number of HTTP requests processed",
-		ConstLabels: nil,
-	}, []string{"method", "path", "status"})
-)

pkg/metrics/prometheus.go

@@ -0,0 +1,74 @@
+package metrics
+
+import (
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promhttp"
+	"net/http"
+	"sync"
+)
+
+const (
+	namespace                      = "middleman"
+	registeredRegexPrefix          = "registered_regex_"
+	registeredRegexTotalName       = registeredRegexPrefix + "total"
+	incomingHTTPRequestPrefix      = "incoming_http_requests_"
+	incomingHTTPRequestTotalName   = incomingHTTPRequestPrefix + "total"
+	processingDurationPrefix       = "processing_duration_"
+	processingDurationName         = processingDurationPrefix + "seconds"
+	processedHTTPRequestsPrefix    = "processed_http_requests_"
+	processedHTTPRequestsTotalName = processedHTTPRequestsPrefix + "total"
+	healthcheckResponseTimePrefix  = "healthcheck_response_time_"
+	healthcheckResponseTimeName    = healthcheckResponseTimePrefix + "seconds"
+	nameResolutionDurationPrefix   = "name_resolution_duration_"
+	nameResolutionDurationName     = nameResolutionDurationPrefix + "seconds"
+)
+
+var (
+	once                      sync.Once
+	promRegistry              = prometheus.NewRegistry()
+	RegisteredRegexCounterVec = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: namespace,
+		Name:      registeredRegexTotalName,
+		Help:      "Total number of registered regex",
+	}, []string{"client", "http_method", "regex"})
+	IncomingHTTPRequestCounterVec = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: namespace,
+		Name:      incomingHTTPRequestTotalName,
+		Help:      "Total number of incoming HTTP requests",
+	}, []string{"client", "http_method", "path"})
+	ProcessingDurationHistogramVec = prometheus.NewHistogramVec(prometheus.HistogramOpts{
+		Namespace: namespace,
+		Name:      processingDurationName,
+		Help:      "Number of seconds to process an incoming request",
+	}, []string{"client", "http_method", "path"})
+	ProcessedHTTPRequestCounterVec = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: namespace,
+		Name:      processedHTTPRequestsTotalName,
+		Help:      "Total number of processed HTTP requests",
+	}, []string{"client", "http_methods", "path", "status_code"})
+	HealthcheckResponseTimeGauge = prometheus.NewGauge(prometheus.GaugeOpts{
+		Namespace: namespace,
+		Name:      healthcheckResponseTimeName,
+		Help:      "Number of seconds to perform healthcheck request",
+	})
+	NameResolutionDurationGauge = prometheus.NewGaugeVec(prometheus.GaugeOpts{
+		Namespace: namespace,
+		Name:      nameResolutionDurationName,
+		Help:      "Number of seconds to perform name resolution using the local resolver",
+	}, []string{"client"})
+)
+
+func PrometheusHandler() http.Handler {
+	return promhttp.HandlerFor(promRegistry, promhttp.HandlerOpts{})
+}
+
+func RegisterPrometheus() {
+	once.Do(func() {
+		promRegistry.MustRegister(RegisteredRegexCounterVec)
+		promRegistry.MustRegister(IncomingHTTPRequestCounterVec)
+		promRegistry.MustRegister(ProcessingDurationHistogramVec)
+		promRegistry.MustRegister(ProcessedHTTPRequestCounterVec)
+		promRegistry.MustRegister(HealthcheckResponseTimeGauge)
+		promRegistry.MustRegister(NameResolutionDurationGauge)
+	})
+}