Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetRoutePattern: Handle the corner case when the ctx doesnt exist or the path is undefined #118

Merged
merged 6 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions pkg/zrouter/zmiddlewares/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import (
)

const (
cacheKeyPrefix = "zrouter_cache"
cacheSetsMetric = "cache_sets"
cacheHitsMetric = "cache_hits"
cacheMissesMetric = "cache_misses"
getRequestBodyMetric = "get_request_body"
cacheKeyPrefix = "zrouter_cache"
cacheSetsMetric = "cache_sets"
cacheHitsMetric = "cache_hits"
cacheMissesMetric = "cache_misses"
getRequestBodyErrorMetric = "get_request_body_error"
)

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

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

return true
}

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

Expand Down
49 changes: 47 additions & 2 deletions pkg/zrouter/zmiddlewares/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,21 @@ package zmiddlewares

import (
"bytes"
"context"
"crypto/sha256"
"encoding/hex"
"github.com/go-chi/chi/v5"
"github.com/zondax/golem/pkg/logger"
"io"
"net/http"
"regexp"
"strings"
)

const (
undefinedPath = "undefined"
)

func PathToRegexp(path string) *regexp.Regexp {
escapedPath := regexp.QuoteMeta(path)
escapedPath = strings.ReplaceAll(escapedPath, "\\{", "{")
Expand All @@ -22,21 +28,60 @@ func PathToRegexp(path string) *regexp.Regexp {

func GetRoutePattern(r *http.Request) string {
rctx := chi.RouteContext(r.Context())
if rctx == nil {
return undefinedPath
}

if pattern := rctx.RoutePattern(); pattern != "" && !strings.HasSuffix(pattern, "*") {
return pattern
}

routePath := r.URL.Path
tctx := chi.NewRouteContext()
if !rctx.Routes.Match(tctx, r.Method, routePath) {
// No matching pattern, so just return the request path.
return routePath
return undefinedPath
}

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

func GetSubRoutePattern(r *http.Request) string {
rctx := chi.RouteContext(r.Context())
if rctx == nil {
return undefinedPath
}

routePattern := rctx.RoutePattern()
if strings.Contains(rctx.RoutePattern(), "*") {
return routePattern
}

return getRoutePrefix(r.Context(), routePattern)
}

func getRoutePrefix(ctx context.Context, route string) string {
if !strings.HasPrefix(route, "/") {
route = "/" + route
}

segments := strings.Split(route, "/")

// The first segment is empty due to the leading "/", so we check the second segment
if len(segments) > 2 {
// The first real segment is at index 1, return it with "/*"
return "/" + segments[1] + "/*"
}

if len(segments) == 2 && segments[1] != "" {
// There's only one segment in the route, return it with "/*"
return "/" + segments[1] + "/*"
}

logger.GetLoggerFromContext(ctx).Errorf("Cannot detect the route prefix for %s", route)
return "/*"
}

func getRequestBody(r *http.Request) ([]byte, error) {
bodyBytes, err := io.ReadAll(r.Body)
if err != nil {
Expand Down
32 changes: 32 additions & 0 deletions pkg/zrouter/zmiddlewares/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package zmiddlewares

import (
"bytes"
"context"
"github.com/go-chi/chi/v5"
"github.com/stretchr/testify/assert"
"io"
Expand Down Expand Up @@ -42,6 +43,13 @@ func TestGetRoutePatternIncludingSubrouters(t *testing.T) {
wMain := httptest.NewRecorder()
r.ServeHTTP(wMain, reqMain)
assert.Equal(t, http.StatusOK, wMain.Code, "The expected status code for main router should be 200 OK.")

// Test request for the subrouter route when the path is undefined
reqSub = httptest.NewRequest("GET", "/test/undefinedRoute", nil)
wSub = httptest.NewRecorder()
r.ServeHTTP(wSub, reqSub)
assert.Equal(t, http.StatusNotFound, wSub.Code, "The expected status code for an undefined route should be 404 Not Found.")
assert.Equal(t, undefinedPath, GetRoutePattern(reqSub))
}

func TestGetRequestBody(t *testing.T) {
Expand All @@ -66,3 +74,27 @@ func TestGenerateBodyHash(t *testing.T) {
hash := generateBodyHash([]byte(testContent))
assert.Equal(t, expectedHash, hash)
}

func TestGetRoutePrefix(t *testing.T) {
tests := []struct {
route string
wantPrefix string
}{
{"/transactions/erc20/{address}", "/transactions/*"},
{"transactions/adasd", "/transactions/*"},
{"/account/12345", "/account/*"},
{"/address/something/else", "/address/*"},
{"dynamic-config", "/dynamic-config/*"},
{"/search/this/item", "/search/*"},
{"/stats", "/stats/*"},
{"/tipset/0", "/tipset/*"},
{"/tools/convert", "/tools/*"},
}

for _, test := range tests {
got := getRoutePrefix(context.Background(), test.route)
if got != test.wantPrefix {
t.Errorf("getRoutePrefix(%q) = %q, want %q", test.route, got, test.wantPrefix)
}
}
}
24 changes: 14 additions & 10 deletions pkg/zrouter/zmiddlewares/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const (
pathLabel = "path"
methodLabel = "method"
statusLabel = "status"
subRouteLabel = "sub_route"
)

func RegisterRequestMetrics(metricsServer metrics.TaskMetrics) []error {
Expand All @@ -29,17 +30,19 @@ func RegisterRequestMetrics(metricsServer metrics.TaskMetrics) []error {
}
}

register(totalRequestsMetricName, "Total number of HTTP requests made.", []string{"method", "path", "status"}, &collectors.Counter{})
register(durationMillisecondsMetricName, "Duration of HTTP requests in milliseconds.", []string{"method", "path", "status"}, &collectors.Gauge{})
register(responseSizeMetricName, "Size of HTTP response in bytes.", []string{"method", "path", "status"}, &collectors.Histogram{})
register(activeConnectionsMetricName, "Number of active HTTP connections.", []string{"method", "path"}, &collectors.Gauge{})
register(totalRequestsMetricName, "Total number of HTTP requests made.", []string{subRouteLabel, methodLabel, pathLabel, statusLabel}, &collectors.Counter{})
register(durationMillisecondsMetricName, "Duration of HTTP requests in milliseconds.", []string{subRouteLabel, methodLabel, pathLabel, statusLabel}, &collectors.Gauge{})
register(responseSizeMetricName, "Size of HTTP response in bytes.", []string{subRouteLabel, methodLabel, pathLabel, statusLabel}, &collectors.Histogram{})
register(activeConnectionsMetricName, "Number of active HTTP connections.", []string{subRouteLabel, methodLabel, pathLabel}, &collectors.Gauge{})

register(getRequestBodyErrorMetric, "Register get request body error.", []string{subRouteLabel, pathLabel}, &collectors.Counter{})

cacheHitsMetricName := cacheHitsMetric
cacheMissesMetricName := cacheMissesMetric
cacheSetsMetricName := cacheSetsMetric
register(cacheHitsMetricName, "Number of cache hits.", []string{pathLabel}, &collectors.Counter{})
register(cacheMissesMetricName, "Number of cache misses.", []string{pathLabel}, &collectors.Counter{})
register(cacheSetsMetricName, "Number of responses added to the cache.", []string{pathLabel}, &collectors.Counter{})
register(cacheHitsMetricName, "Number of cache hits.", []string{subRouteLabel, pathLabel}, &collectors.Counter{})
register(cacheMissesMetricName, "Number of cache misses.", []string{subRouteLabel, pathLabel}, &collectors.Counter{})
register(cacheSetsMetricName, "Number of responses added to the cache.", []string{subRouteLabel, pathLabel}, &collectors.Counter{})

return errs
}
Expand All @@ -51,11 +54,12 @@ func RequestMetrics(metricsServer metrics.TaskMetrics) Middleware {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
path := GetRoutePattern(r)
subRoute := GetSubRoutePattern(r)
startTime := time.Now()

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

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

labels := []string{r.Method, path, strconv.Itoa(responseStatus)}
labels := []string{subRoute, r.Method, path, strconv.Itoa(responseStatus)}

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