From 7605e2a6572b8648f7ce251a3b066fafcd01e7aa Mon Sep 17 00:00:00 2001
From: Ahmad Karimi <39967326+therealak12@users.noreply.github.com>
Date: Thu, 21 Mar 2024 10:46:49 +0330
Subject: [PATCH] Conditionally set url label for 404 responses (#111)

* conditionally set url label for 404 responses
---
 echoprometheus/prometheus.go      |  6 ++++-
 echoprometheus/prometheus_test.go | 39 +++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/echoprometheus/prometheus.go b/echoprometheus/prometheus.go
index 89ae77e..cc864c8 100644
--- a/echoprometheus/prometheus.go
+++ b/echoprometheus/prometheus.go
@@ -73,6 +73,10 @@ type MiddlewareConfig struct {
 	AfterNext func(c echo.Context, err error)
 
 	timeNow func() time.Time
+
+	// If DoNotUseRequestPathFor404 is true, all 404 responses (due to non-matching route) will have the same `url` label and
+	// thus won't generate new metrics.
+	DoNotUseRequestPathFor404 bool
 }
 
 type LabelValueFunc func(c echo.Context, err error) string
@@ -246,7 +250,7 @@ func (conf MiddlewareConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
 			}
 
 			url := c.Path() // contains route path ala `/users/:id`
-			if url == "" {
+			if url == "" && !conf.DoNotUseRequestPathFor404 {
 				// as of Echo v4.10.1 path is empty for 404 cases (when router did not find any matching routes)
 				// in this case we use actual path from request to have some distinction in Prometheus
 				url = c.Request().URL.Path
diff --git a/echoprometheus/prometheus_test.go b/echoprometheus/prometheus_test.go
index e4397b4..5418360 100644
--- a/echoprometheus/prometheus_test.go
+++ b/echoprometheus/prometheus_test.go
@@ -275,6 +275,45 @@ func TestRunPushGatewayGatherer(t *testing.T) {
 
 	assert.EqualError(t, err, "code=400, message=post metrics request did not succeed")
 	assert.True(t, receivedMetrics)
+	unregisterDefaults("myapp")
+}
+
+// TestSetPathFor404NoMatchingRoute tests that the url is not included in the metric when
+// the 404 response is due to no matching route
+func TestSetPathFor404NoMatchingRoute(t *testing.T) {
+	e := echo.New()
+
+	e.Use(NewMiddlewareWithConfig(MiddlewareConfig{DoNotUseRequestPathFor404: true, Subsystem: defaultSubsystem}))
+	e.GET("/metrics", NewHandler())
+
+	assert.Equal(t, http.StatusNotFound, request(e, "/nonExistentPath"))
+
+	s, code := requestBody(e, "/metrics")
+	assert.Equal(t, http.StatusOK, code)
+	assert.Contains(t, s, fmt.Sprintf(`%s_request_duration_seconds_count{code="404",host="example.com",method="GET",url=""} 1`, defaultSubsystem))
+	assert.NotContains(t, s, fmt.Sprintf(`%s_request_duration_seconds_count{code="404",host="example.com",method="GET",url="/nonExistentPath"} 1`, defaultSubsystem))
+
+	unregisterDefaults(defaultSubsystem)
+}
+
+// TestSetPathFor404Logic tests that the url is included in the metric when the 404 response is due to logic
+func TestSetPathFor404Logic(t *testing.T) {
+	unregisterDefaults("myapp")
+	e := echo.New()
+
+	e.Use(NewMiddlewareWithConfig(MiddlewareConfig{DoNotUseRequestPathFor404: true, Subsystem: defaultSubsystem}))
+	e.GET("/metrics", NewHandler())
+
+	e.GET("/sample", echo.NotFoundHandler)
+
+	assert.Equal(t, http.StatusNotFound, request(e, "/sample"))
+
+	s, code := requestBody(e, "/metrics")
+	assert.Equal(t, http.StatusOK, code)
+	assert.NotContains(t, s, fmt.Sprintf(`%s_request_duration_seconds_count{code="404",host="example.com",method="GET",url=""} 1`, defaultSubsystem))
+	assert.Contains(t, s, fmt.Sprintf(`%s_request_duration_seconds_count{code="404",host="example.com",method="GET",url="/sample"} 1`, defaultSubsystem))
+
+	unregisterDefaults(defaultSubsystem)
 }
 
 func requestBody(e *echo.Echo, path string) (string, int) {