Skip to content

Commit 442f07c

Browse files
lucaslopezflucaslopezf
authored
GetRoutePattern: Handle the corner case when the ctx doesnt exist or the path is undefined (#118)
* Fix GetRoutePattern for subrouters * Handle the corner case when the ctx doesnt exist or the path is undefined * minor change * add subrouter to metrics * minor fix
1 parent b336502 commit 442f07c

File tree

4 files changed

+101
-20
lines changed

4 files changed

+101
-20
lines changed

‎pkg/zrouter/zmiddlewares/cache.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ import (
1414
)
1515

1616
const (
17-
cacheKeyPrefix = "zrouter_cache"
18-
cacheSetsMetric = "cache_sets"
19-
cacheHitsMetric = "cache_hits"
20-
cacheMissesMetric = "cache_misses"
21-
getRequestBodyMetric = "get_request_body"
17+
cacheKeyPrefix = "zrouter_cache"
18+
cacheSetsMetric = "cache_sets"
19+
cacheHitsMetric = "cache_hits"
20+
cacheMissesMetric = "cache_misses"
21+
getRequestBodyErrorMetric = "get_request_body_error"
2222
)
2323

2424
type CacheProcessedPath struct {
@@ -71,7 +71,7 @@ func constructCacheKey(fullURL string, r *http.Request, metricServer metrics.Tas
7171
if shouldProcessRequestBody(r.Method) {
7272
body, err := getRequestBody(r)
7373
if err != nil {
74-
if metricErr := metricServer.IncrementMetric(getRequestBodyMetric, GetRoutePattern(r)); metricErr != nil {
74+
if metricErr := metricServer.IncrementMetric(getRequestBodyErrorMetric, GetSubRoutePattern(r), GetRoutePattern(r)); metricErr != nil {
7575
logger.GetLoggerFromContext(r.Context()).Errorf("Error incrementing get_request_body metric: %v", metricErr)
7676
}
7777
return "", err
@@ -91,14 +91,14 @@ func tryServeFromCache(w http.ResponseWriter, r *http.Request, cache zcache.ZCac
9191
w.Header().Set(domain.ContentTypeHeader, domain.ContentTypeApplicationJSON)
9292
_, _ = w.Write(cachedResponse)
9393

94-
if err = metricServer.IncrementMetric(cacheHitsMetric, GetRoutePattern(r)); err != nil {
94+
if err = metricServer.IncrementMetric(cacheHitsMetric, GetSubRoutePattern(r), GetRoutePattern(r)); err != nil {
9595
logger.GetLoggerFromContext(r.Context()).Errorf("Error incrementing cache_hits metric: %v", err)
9696
}
9797

9898
return true
9999
}
100100

101-
if err = metricServer.IncrementMetric(cacheMissesMetric, GetRoutePattern(r)); err != nil {
101+
if err = metricServer.IncrementMetric(cacheMissesMetric, GetSubRoutePattern(r), GetRoutePattern(r)); err != nil {
102102
logger.GetLoggerFromContext(r.Context()).Errorf("Error incrementing cache_misses metric: %v", err)
103103
}
104104

‎pkg/zrouter/zmiddlewares/common.go

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,21 @@ package zmiddlewares
22

33
import (
44
"bytes"
5+
"context"
56
"crypto/sha256"
67
"encoding/hex"
78
"github.com/go-chi/chi/v5"
9+
"github.com/zondax/golem/pkg/logger"
810
"io"
911
"net/http"
1012
"regexp"
1113
"strings"
1214
)
1315

16+
const (
17+
undefinedPath = "undefined"
18+
)
19+
1420
func PathToRegexp(path string) *regexp.Regexp {
1521
escapedPath := regexp.QuoteMeta(path)
1622
escapedPath = strings.ReplaceAll(escapedPath, "\\{", "{")
@@ -22,21 +28,60 @@ func PathToRegexp(path string) *regexp.Regexp {
2228

2329
func GetRoutePattern(r *http.Request) string {
2430
rctx := chi.RouteContext(r.Context())
31+
if rctx == nil {
32+
return undefinedPath
33+
}
34+
2535
if pattern := rctx.RoutePattern(); pattern != "" && !strings.HasSuffix(pattern, "*") {
2636
return pattern
2737
}
2838

2939
routePath := r.URL.Path
3040
tctx := chi.NewRouteContext()
3141
if !rctx.Routes.Match(tctx, r.Method, routePath) {
32-
// No matching pattern, so just return the request path.
33-
return routePath
42+
return undefinedPath
3443
}
3544

3645
// tctx has the updated pattern, since Match mutates it
3746
return tctx.RoutePattern()
3847
}
3948

49+
func GetSubRoutePattern(r *http.Request) string {
50+
rctx := chi.RouteContext(r.Context())
51+
if rctx == nil {
52+
return undefinedPath
53+
}
54+
55+
routePattern := rctx.RoutePattern()
56+
if strings.Contains(rctx.RoutePattern(), "*") {
57+
return routePattern
58+
}
59+
60+
return getRoutePrefix(r.Context(), routePattern)
61+
}
62+
63+
func getRoutePrefix(ctx context.Context, route string) string {
64+
if !strings.HasPrefix(route, "/") {
65+
route = "/" + route
66+
}
67+
68+
segments := strings.Split(route, "/")
69+
70+
// The first segment is empty due to the leading "/", so we check the second segment
71+
if len(segments) > 2 {
72+
// The first real segment is at index 1, return it with "/*"
73+
return "/" + segments[1] + "/*"
74+
}
75+
76+
if len(segments) == 2 && segments[1] != "" {
77+
// There's only one segment in the route, return it with "/*"
78+
return "/" + segments[1] + "/*"
79+
}
80+
81+
logger.GetLoggerFromContext(ctx).Errorf("Cannot detect the route prefix for %s", route)
82+
return "/*"
83+
}
84+
4085
func getRequestBody(r *http.Request) ([]byte, error) {
4186
bodyBytes, err := io.ReadAll(r.Body)
4287
if err != nil {

‎pkg/zrouter/zmiddlewares/common_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package zmiddlewares
22

33
import (
44
"bytes"
5+
"context"
56
"github.com/go-chi/chi/v5"
67
"github.com/stretchr/testify/assert"
78
"io"
@@ -42,6 +43,13 @@ func TestGetRoutePatternIncludingSubrouters(t *testing.T) {
4243
wMain := httptest.NewRecorder()
4344
r.ServeHTTP(wMain, reqMain)
4445
assert.Equal(t, http.StatusOK, wMain.Code, "The expected status code for main router should be 200 OK.")
46+
47+
// Test request for the subrouter route when the path is undefined
48+
reqSub = httptest.NewRequest("GET", "/test/undefinedRoute", nil)
49+
wSub = httptest.NewRecorder()
50+
r.ServeHTTP(wSub, reqSub)
51+
assert.Equal(t, http.StatusNotFound, wSub.Code, "The expected status code for an undefined route should be 404 Not Found.")
52+
assert.Equal(t, undefinedPath, GetRoutePattern(reqSub))
4553
}
4654

4755
func TestGetRequestBody(t *testing.T) {
@@ -66,3 +74,27 @@ func TestGenerateBodyHash(t *testing.T) {
6674
hash := generateBodyHash([]byte(testContent))
6775
assert.Equal(t, expectedHash, hash)
6876
}
77+
78+
func TestGetRoutePrefix(t *testing.T) {
79+
tests := []struct {
80+
route string
81+
wantPrefix string
82+
}{
83+
{"/transactions/erc20/{address}", "/transactions/*"},
84+
{"transactions/adasd", "/transactions/*"},
85+
{"/account/12345", "/account/*"},
86+
{"/address/something/else", "/address/*"},
87+
{"dynamic-config", "/dynamic-config/*"},
88+
{"/search/this/item", "/search/*"},
89+
{"/stats", "/stats/*"},
90+
{"/tipset/0", "/tipset/*"},
91+
{"/tools/convert", "/tools/*"},
92+
}
93+
94+
for _, test := range tests {
95+
got := getRoutePrefix(context.Background(), test.route)
96+
if got != test.wantPrefix {
97+
t.Errorf("getRoutePrefix(%q) = %q, want %q", test.route, got, test.wantPrefix)
98+
}
99+
}
100+
}

‎pkg/zrouter/zmiddlewares/metrics.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const (
1818
pathLabel = "path"
1919
methodLabel = "method"
2020
statusLabel = "status"
21+
subRouteLabel = "sub_route"
2122
)
2223

2324
func RegisterRequestMetrics(metricsServer metrics.TaskMetrics) []error {
@@ -29,17 +30,19 @@ func RegisterRequestMetrics(metricsServer metrics.TaskMetrics) []error {
2930
}
3031
}
3132

32-
register(totalRequestsMetricName, "Total number of HTTP requests made.", []string{"method", "path", "status"}, &collectors.Counter{})
33-
register(durationMillisecondsMetricName, "Duration of HTTP requests in milliseconds.", []string{"method", "path", "status"}, &collectors.Gauge{})
34-
register(responseSizeMetricName, "Size of HTTP response in bytes.", []string{"method", "path", "status"}, &collectors.Histogram{})
35-
register(activeConnectionsMetricName, "Number of active HTTP connections.", []string{"method", "path"}, &collectors.Gauge{})
33+
register(totalRequestsMetricName, "Total number of HTTP requests made.", []string{subRouteLabel, methodLabel, pathLabel, statusLabel}, &collectors.Counter{})
34+
register(durationMillisecondsMetricName, "Duration of HTTP requests in milliseconds.", []string{subRouteLabel, methodLabel, pathLabel, statusLabel}, &collectors.Gauge{})
35+
register(responseSizeMetricName, "Size of HTTP response in bytes.", []string{subRouteLabel, methodLabel, pathLabel, statusLabel}, &collectors.Histogram{})
36+
register(activeConnectionsMetricName, "Number of active HTTP connections.", []string{subRouteLabel, methodLabel, pathLabel}, &collectors.Gauge{})
37+
38+
register(getRequestBodyErrorMetric, "Register get request body error.", []string{subRouteLabel, pathLabel}, &collectors.Counter{})
3639

3740
cacheHitsMetricName := cacheHitsMetric
3841
cacheMissesMetricName := cacheMissesMetric
3942
cacheSetsMetricName := cacheSetsMetric
40-
register(cacheHitsMetricName, "Number of cache hits.", []string{pathLabel}, &collectors.Counter{})
41-
register(cacheMissesMetricName, "Number of cache misses.", []string{pathLabel}, &collectors.Counter{})
42-
register(cacheSetsMetricName, "Number of responses added to the cache.", []string{pathLabel}, &collectors.Counter{})
43+
register(cacheHitsMetricName, "Number of cache hits.", []string{subRouteLabel, pathLabel}, &collectors.Counter{})
44+
register(cacheMissesMetricName, "Number of cache misses.", []string{subRouteLabel, pathLabel}, &collectors.Counter{})
45+
register(cacheSetsMetricName, "Number of responses added to the cache.", []string{subRouteLabel, pathLabel}, &collectors.Counter{})
4346

4447
return errs
4548
}
@@ -51,11 +54,12 @@ func RequestMetrics(metricsServer metrics.TaskMetrics) Middleware {
5154
return func(next http.Handler) http.Handler {
5255
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
5356
path := GetRoutePattern(r)
57+
subRoute := GetSubRoutePattern(r)
5458
startTime := time.Now()
5559

5660
mu.Lock()
5761
activeConnections++
58-
if err := metricsServer.UpdateMetric(activeConnectionsMetricName, float64(activeConnections), r.Method, path); err != nil {
62+
if err := metricsServer.UpdateMetric(activeConnectionsMetricName, float64(activeConnections), subRoute, r.Method, path); err != nil {
5963
logger.GetLoggerFromContext(r.Context()).Errorf("error updating active connections metric: %v", err.Error())
6064
}
6165
mu.Unlock()
@@ -65,7 +69,7 @@ func RequestMetrics(metricsServer metrics.TaskMetrics) Middleware {
6569

6670
mu.Lock()
6771
activeConnections--
68-
if err := metricsServer.UpdateMetric(activeConnectionsMetricName, float64(activeConnections), r.Method, path); err != nil {
72+
if err := metricsServer.UpdateMetric(activeConnectionsMetricName, float64(activeConnections), subRoute, r.Method, path); err != nil {
6973
logger.GetLoggerFromContext(r.Context()).Errorf("error updating active connections metric: %v", err.Error())
7074
}
7175
mu.Unlock()
@@ -75,7 +79,7 @@ func RequestMetrics(metricsServer metrics.TaskMetrics) Middleware {
7579
responseStatus := mrw.status
7680
bytesWritten := mrw.written
7781

78-
labels := []string{r.Method, path, strconv.Itoa(responseStatus)}
82+
labels := []string{subRoute, r.Method, path, strconv.Itoa(responseStatus)}
7983

8084
if err := metricsServer.UpdateMetric(durationMillisecondsMetricName, duration, labels...); err != nil {
8185
logger.GetLoggerFromContext(r.Context()).Errorf("error updating request duration metric: %v", err.Error())

0 commit comments

Comments
 (0)