feat(httpclient): Added request path normalization mechanism in metrics transport for better metrics cardinality

httpclient/README.md

@@ -9,13 +9,15 @@
 > Http client module based on [net/http](https://pkg.go.dev/net/http).
 
 <!-- TOC -->
+
 * [Installation](#installation)
 * [Documentation](#documentation)
-  * [Requests](#requests)
-  * [Transports](#transports)
-    * [BaseTransport](#basetransport)
-    * [LoggerTransport](#loggertransport)
-    * [MetricsTransport](#metricstransport)
+	* [Requests](#requests)
+	* [Transports](#transports)
+		* [BaseTransport](#basetransport)
+		* [LoggerTransport](#loggertransport)
+		* [MetricsTransport](#metricstransport)
+
 <!-- TOC -->
 
 ## Installation
@@ -195,18 +197,31 @@ var client, _ = httpclient.NewDefaultHttpClientFactory().Create(
 		transport.NewMetricsTransportWithConfig(
 			transport.NewBaseTransport(),
 			&transport.MetricsTransportConfig{
-				Registry:            prometheus.DefaultRegisterer, // metrics registry
-				Namespace:           "",                           // metrics namespace
-				Subsystem:           "",                           // metrics subsystem
-				Buckets:             prometheus.DefBuckets,        // metrics duration buckets
-				NormalizeHTTPStatus: true,                         // normalize the response HTTP code (ex: 201 => 2xx)
+				Registry:                  prometheus.DefaultRegisterer, // metrics registry
+				Namespace:                 "",                           // metrics namespace
+				Subsystem:                 "",                           // metrics subsystem
+				Buckets:                   prometheus.DefBuckets,        // metrics duration buckets
+				NormalizeRequestPath:      false,                        // normalize the request path following the masks given in NormalizePathMasks
+				NormalizeRequestPathMasks: map[string]string{},          // request path normalization masks (key: regex to match, value: mask to apply)
+				NormalizeResponseStatus:   true,                         // normalize the response HTTP code (ex: 201 => 2xx)
 			},
 		),
 	),
 )
 ```
 
-Notes:
+If no transport is provided for decoration in `transport.NewMetricsTransport(nil)`, the [BaseTransport](transport/base.go) will be used as base transport.
+
+If no registry is provided in the `config` in `transport.NewMetricsTransportWithConfig(nil, config)`, the `prometheus.DefaultRegisterer` will be used a metrics registry
+
+If the provided config provides `NormalizeRequestPath` to `true` and with the following `NormalizeRequestPathMasks`:
+
+```go
+map[string]string{
+    `/foo/(.+)/bar\?page=(.+)`: "/foo/{fooId}/bar?page={pageId}",
+},
+```
+
+Then if the request path is `/foo/1/bar?page=2`, the metric path label will be masked with `/foo/{fooId}/bar?page={pageId}`.
+
 
-- if no transport is provided for decoration in `transport.NewMetricsTransport(nil)`, the [BaseTransport](transport/base.go) will be used as base transport
-- if no registry is provided in the `config` in `transport.NewMetricsTransportWithConfig(nil, config)`, the `prometheus.DefaultRegisterer` will be used a metrics registry

httpclient/go.mod

@@ -20,6 +20,7 @@ require (
 	github.com/prometheus/client_model v0.5.0 // indirect
 	github.com/prometheus/common v0.48.0 // indirect
 	github.com/prometheus/procfs v0.12.0 // indirect
+	github.com/stretchr/objx v0.5.0 // indirect
 	go.opentelemetry.io/otel v1.16.0 // indirect
 	go.opentelemetry.io/otel/trace v1.16.0 // indirect
 	golang.org/x/sys v0.16.0 // indirect

httpclient/go.sum

@@ -6,6 +6,7 @@ github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj
 github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
+github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
@@ -32,7 +33,12 @@ github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjR
 github.com/rs/xid v1.4.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg=
 github.com/rs/zerolog v1.29.1 h1:cO+d60CHkknCbvzEWxP0S9K6KqyTjrCNUy1LdQLCGPc=
 github.com/rs/zerolog v1.29.1/go.mod h1:Le6ESbR7hc+DP6Lt1THiV8CQSdkkNrd3R0XbEgp3ZBU=
+github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
 github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
+github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
 github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 go.opentelemetry.io/otel v1.16.0 h1:Z7GVAX/UkAXPKsy94IU+i6thsQS4nb7LviLpnaNeW8s=
@@ -47,5 +53,6 @@ google.golang.org/protobuf v1.32.0 h1:pPC6BG5ex8PDFnkbrGU3EixyhKcQ2aDuBS36lqK/C7
 google.golang.org/protobuf v1.32.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

httpclient/normalization/path.go

@@ -0,0 +1,22 @@
+package normalization
+
+import (
+	"regexp"
+)
+
+// NormalizePath normalizes a path if matching one of the provided masks, or returns the original path instead.
+//
+// For example: NormalizePath(map[string]string{"/foo/(.+)", "/foo/{id}"}, "/foo/1") will return "/foo/{id}".
+func NormalizePath(masks map[string]string, path string) string {
+	for pattern, mask := range masks {
+		re, err := regexp.Compile(pattern)
+		if err == nil {
+			matched := re.MatchString(path)
+			if matched {
+				return mask
+			}
+		}
+	}
+
+	return path
+}

httpclient/normalization/path_test.go

@@ -0,0 +1,59 @@
+package normalization_test
+
+import (
+	"testing"
+
+	"github.com/ankorstore/yokai/httpclient/normalization"
+)
+
+func TestMask(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		masks map[string]string
+		path  string
+		want  string
+	}{
+		"primary mask applied": {
+			map[string]string{
+				`/foo/(.+)/bar\?page=(.+)#baz`: "/foo/{fooId}/bar?page={pageId}#baz",
+			},
+			"/foo/1/bar?page=1#baz",
+			"/foo/{fooId}/bar?page={pageId}#baz",
+		},
+		"secondary mask applied": {
+			map[string]string{
+				`/foo/(.+)/baz\?page=(.+)#baz`: "/foo/{fooId}/baz?page={pageId}#baz",
+				`/foo/(.+)/bar\?page=(.+)#baz`: "/foo/{fooId}/bar?page={pageId}#baz",
+			},
+			"/foo/1/bar?page=1#baz",
+			"/foo/{fooId}/bar?page={pageId}#baz",
+		},
+		"primary mask not applied": {
+			map[string]string{
+				`/foo/(.+)/bar\?page=(.+)#baz`: "/foo/{fooId}/bar?page={pageId}#baz",
+			},
+			"/foo/1/bar?pages=1#baz",
+			"/foo/1/bar?pages=1#baz",
+		},
+		"primary mask applied on invalid regexp": {
+			map[string]string{
+				`(.`: "/foo/{fooId}/bar?page={pageId}#baz",
+			},
+			"/foo/1/bar?page=1#baz",
+			"/foo/1/bar?page=1#baz",
+		},
+		"no mask applied on empty masks list": {
+			map[string]string{},
+			"/foo/1/bar?page=1#baz",
+			"/foo/1/bar?page=1#baz",
+		},
+	}
+
+	for name, tt := range tests {
+		got := normalization.NormalizePath(tt.masks, tt.path)
+		if got != tt.want {
+			t.Errorf("%s: expected %s, got %s", name, tt.want, got)
+		}
+	}
+}

httpclient/status/status.go → httpclient/normalization/status.go

@@ -1,7 +1,7 @@
-package status
+package normalization
 
-// NormalizeHTTPStatus normalizes an HTTP status code.
-func NormalizeHTTPStatus(status int) string {
+// NormalizeStatus normalizes an HTTP status code.
+func NormalizeStatus(status int) string {
 	switch {
 	case status < 200:
 		return "1xx"

httpclient/status/status_test.go → httpclient/normalization/status_test.go

@@ -1,9 +1,9 @@
-package status_test
+package normalization_test
 
 import (
 	"testing"
 
-	"github.com/ankorstore/yokai/httpclient/status"
+	"github.com/ankorstore/yokai/httpclient/normalization"
 )
 
 func TestNormalizeHTTPStatus(t *testing.T) {
@@ -22,7 +22,7 @@ func TestNormalizeHTTPStatus(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		got := status.NormalizeHTTPStatus(tt.code)
+		got := normalization.NormalizeStatus(tt.code)
 
 		if got != tt.want {
 			t.Errorf("expected %s, got %s", tt.want, got)

httpclient/transport/logger.go

@@ -60,6 +60,8 @@ func (t *LoggerTransport) Base() http.RoundTripper {
 }
 
 // RoundTrip performs a request / response round trip, based on the wrapped [http.RoundTripper].
+//
+//nolint:cyclop
 func (t *LoggerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	logger := log.CtxLogger(req.Context())
 
@@ -81,7 +83,14 @@ func (t *LoggerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	resp, err := t.transport.RoundTrip(req)
 	latency := time.Since(start).String()
 
+	if err != nil {
+		logger.Error().Err(err).Str("latency", latency).Msg("http client failure")
+
+		return resp, err
+	}
+
 	var respEvt *zerolog.Event
+
 	if t.config.LogResponseLevelFromResponseCode {
 		switch {
 		case resp.StatusCode >= http.StatusBadRequest && resp.StatusCode < http.StatusInternalServerError:

httpclient/transport/logger_test.go

@@ -3,6 +3,7 @@ package transport_test
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strconv"
@@ -13,8 +14,19 @@ import (
 	"github.com/ankorstore/yokai/log/logtest"
 	"github.com/rs/zerolog"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 )
 
+type transportMock struct {
+	mock.Mock
+}
+
+func (m *transportMock) RoundTrip(*http.Request) (*http.Response, error) {
+	args := m.Called()
+
+	return nil, args.Error(1)
+}
+
 func TestNewLoggerTransport(t *testing.T) {
 	t.Parallel()
 
@@ -201,3 +213,44 @@ func TestLoggerTransportRoundTripWithConfig(t *testing.T) {
 		"message":  "http client response",
 	})
 }
+
+func TestLoggerTransportRoundTripWithFailure(t *testing.T) {
+	t.Parallel()
+
+	logBuffer := logtest.NewDefaultTestLogBuffer()
+	logger, err := log.NewDefaultLoggerFactory().Create(
+		log.WithLevel(zerolog.DebugLevel),
+		log.WithOutputWriter(logBuffer),
+	)
+	assert.NoError(t, err)
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNoContent)
+	}))
+	defer server.Close()
+
+	req := httptest.NewRequest(http.MethodGet, server.URL, nil)
+	req = req.WithContext(logger.WithContext(context.Background()))
+
+	base := new(transportMock)
+	base.On("RoundTrip", mock.Anything).Return(nil, fmt.Errorf("custom http error"))
+
+	//nolint:bodyclose
+	resp, err := transport.NewLoggerTransport(base).RoundTrip(req)
+	assert.Nil(t, resp)
+	assert.Error(t, err)
+	assert.Equal(t, "custom http error", err.Error())
+
+	logtest.AssertHasLogRecord(t, logBuffer, map[string]interface{}{
+		"level":   "info",
+		"method":  "GET",
+		"url":     server.URL,
+		"message": "http client request",
+	})
+
+	logtest.AssertHasLogRecord(t, logBuffer, map[string]interface{}{
+		"error":   "custom http error",
+		"level":   "error",
+		"message": "http client failure",
+	})
+}