services/horizon: Strip regexes from routes before sending to prometh…

…eus (#3459)

* Strip regexes from routes before sending to prometheus

Fixes #3436

* Escape the regex values instead of removing them, when sending to prometheus

* Update changelog for pull 3459

* use assert.Equal for consistency

services/horizon/CHANGELOG.md

@@ -6,6 +6,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).
 ## Unreleased
 
 * Add an endpoint which determines if Horizon is healthy enough to receive traffic ([3435](https://github.com/stellar/go/pull/3435)).
+* Sanitize route regular expressions for Prometheus metrics ([3459](https://github.com/stellar/go/pull/3459)).
 
 ## v2.0.0
 

services/horizon/internal/httpx/middleware.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"net/http"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -129,12 +130,26 @@ func getClientData(r *http.Request, headerName string) string {
 	return value
 }
 
+var routeRegexp = regexp.MustCompile("{([^:}]*):[^}]*}")
+
+// https://prometheus.io/docs/instrumenting/exposition_formats/
+// label_value can be any sequence of UTF-8 characters, but the backslash (\),
+// double-quote ("), and line feed (\n) characters have to be escaped as \\,
+// \", and \n, respectively.
+func sanitizeMetricRoute(routePattern string) string {
+	route := routeRegexp.ReplaceAllString(routePattern, "{$1}")
+	route = strings.ReplaceAll(route, "\\", "\\\\")
+	route = strings.ReplaceAll(route, "\"", "\\\"")
+	route = strings.ReplaceAll(route, "\n", "\\n")
+	return route
+}
+
 func logEndOfRequest(ctx context.Context, r *http.Request, requestDurationSummary *prometheus.SummaryVec, duration time.Duration, mw middleware.WrapResponseWriter, streaming bool) {
-	routePattern := chi.RouteContext(r.Context()).RoutePattern()
+	route := sanitizeMetricRoute(chi.RouteContext(r.Context()).RoutePattern())
 	// Can be empty when request did not reached the final route (ex. blocked by
 	// a middleware). More info: https://github.com/go-chi/chi/issues/270
-	if routePattern == "" {
-		routePattern = "undefined"
+	if route == "" {
+		route = "undefined"
 	}
 
 	referer := r.Referer()
@@ -155,15 +170,15 @@ func logEndOfRequest(ctx context.Context, r *http.Request, requestDurationSummar
 		"ip_port":        r.RemoteAddr,
 		"method":         r.Method,
 		"path":           r.URL.String(),
-		"route":          routePattern,
+		"route":          route,
 		"status":         mw.Status(),
 		"streaming":      streaming,
 		"referer":        referer,
 	}).Info("Finished request")
 
 	requestDurationSummary.With(prometheus.Labels{
 		"status":    strconv.FormatInt(int64(mw.Status()), 10),
-		"route":     routePattern,
+		"route":     route,
 		"streaming": strconv.FormatBool(streaming),
 		"method":    r.Method,
 	}).Observe(float64(duration.Seconds()))

services/horizon/internal/httpx/middleware_test.go

@@ -0,0 +1,47 @@
+package httpx
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMiddlewareSanitizesRoutesForPrometheus(t *testing.T) {
+	for _, setup := range []struct {
+		name     string
+		route    string
+		expected string
+	}{
+		{
+			"normal routes",
+			"/accounts",
+			"/accounts",
+		},
+		{
+			"non-regex params",
+			"/claimable_balances/{id}",
+			"/claimable_balances/{id}",
+		},
+		{
+			"named regexes",
+			"/accounts/{account_id:\\w+}/effects",
+			"/accounts/{account_id}/effects",
+		},
+		{
+			"unnamed regexes",
+			"/accounts/{\\w+}/effects",
+			"/accounts/{\\\\w+}/effects",
+		},
+		{
+			// Not likely used in routes, but just safer for prom metrics anyway
+			"quotes",
+			"/{\"}",
+			"/{\\\"}",
+		},
+	} {
+		t.Run(setup.name, func(t *testing.T) {
+			assert.Equal(t, setup.expected, sanitizeMetricRoute(setup.route))
+		})
+	}
+
+}