Skip to content

Commit bacea9a

Browse files
authored
routing: initialize all error metrics with zero values (#3579)
- Initialize reasonCounts map with all available error types set to 0 - Add TestProcessRouteDefsMetricsInitialization to verify metrics are properly initialized and reset Signed-off-by: Veronika Volokitina <v.volokitinaa@gmail.com>
1 parent 9421217 commit bacea9a

3 files changed

Lines changed: 106 additions & 1 deletion

File tree

routing/datasource.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,17 @@ func mapPredicates(cps []PredicateSpec) map[string]PredicateSpec {
526526
// processes a set of route definitions for the routing table
527527
func processRouteDefs(o *Options, defs []*eskip.Route) (routes []*Route, invalidDefs []*eskip.Route) {
528528
cpm := mapPredicates(o.Predicates)
529-
reasonCounts := make(map[string]int)
529+
// Initialize reasonCounts with all available error codes to ensure metric values can be set to 0
530+
// when the corresponding route error is resolved
531+
reasonCounts := map[string]int{
532+
errUnknownFilter.Code(): 0,
533+
errInvalidFilterParams.Code(): 0,
534+
errUnknownPredicate.Code(): 0,
535+
errInvalidPredicateParams.Code(): 0,
536+
errFailedBackendSplit.Code(): 0,
537+
errInvalidMatcher.Code(): 0,
538+
"other": 0,
539+
}
530540

531541
for _, def := range defs {
532542
route, err := processRouteDef(o, cpm, def)

routing/datasource_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
"github.com/stretchr/testify/assert"
13+
"github.com/zalando/skipper/eskip"
1314
"github.com/zalando/skipper/filters"
1415
"github.com/zalando/skipper/filters/builtin"
1516
"github.com/zalando/skipper/logging"
@@ -733,3 +734,96 @@ func testRouteValidationReasonMetricsWithPrometheus(t *testing.T, routes string,
733734
}
734735
}
735736
}
737+
738+
func TestProcessRouteDefsMetricsInitialization(t *testing.T) {
739+
pm := metrics.NewPrometheus(metrics.Options{})
740+
path := "/metrics"
741+
742+
mux := http.NewServeMux()
743+
pm.RegisterHandler(path, mux)
744+
745+
getMetricsOutput := func() string {
746+
req := httptest.NewRequest("GET", path, nil)
747+
w := httptest.NewRecorder()
748+
mux.ServeHTTP(w, req)
749+
750+
resp := w.Result()
751+
assert.Equal(t, http.StatusOK, resp.StatusCode)
752+
753+
body, err := io.ReadAll(resp.Body)
754+
assert.NoError(t, err)
755+
return string(body)
756+
}
757+
758+
t.Log("Phase 1: Initial state - all metrics should be present with 0 values")
759+
fr := make(filters.Registry)
760+
fr.Register(builtin.NewSetPath())
761+
762+
opts := &routing.Options{
763+
FilterRegistry: fr,
764+
Predicates: []routing.PredicateSpec{primitive.NewTrue(), query.New()},
765+
Metrics: pm,
766+
Log: &logging.DefaultLog{},
767+
}
768+
769+
t.Log("Test with all valid routes")
770+
validRoutes := []*eskip.Route{
771+
{Id: "valid1", Path: "/foo", Backend: "https://example.org"},
772+
{Id: "valid2", Predicates: []*eskip.Predicate{{Name: "Path", Args: []interface{}{"/bar"}}}, Backend: "https://example.org"},
773+
}
774+
775+
routes, invalidRoutes := routing.ExportProcessRouteDefs(opts, validRoutes)
776+
assert.Len(t, routes, 2)
777+
assert.Len(t, invalidRoutes, 0)
778+
779+
output1 := getMetricsOutput()
780+
assert.Contains(t, output1, `skipper_route_invalid{reason="unknown_filter"} 0`)
781+
assert.Contains(t, output1, `skipper_route_invalid{reason="invalid_filter_params"} 0`)
782+
assert.Contains(t, output1, `skipper_route_invalid{reason="unknown_predicate"} 0`)
783+
assert.Contains(t, output1, `skipper_route_invalid{reason="invalid_predicate_params"} 0`)
784+
assert.Contains(t, output1, `skipper_route_invalid{reason="failed_backend_split"} 0`)
785+
assert.Contains(t, output1, `skipper_route_invalid{reason="invalid_matcher"} 0`)
786+
assert.Contains(t, output1, `skipper_route_invalid{reason="other"} 0`)
787+
788+
t.Log("Phase 2: Test with various error types")
789+
invalidRoutesSet := []*eskip.Route{
790+
{Id: "unknownFilter", Predicates: []*eskip.Predicate{{Name: "Path", Args: []interface{}{"/test"}}}, Filters: []*eskip.Filter{{Name: "unknownFilter"}}, Backend: "https://example.org"},
791+
{Id: "invalidFilterParams", Predicates: []*eskip.Predicate{{Name: "Path", Args: []interface{}{"/test2"}}}, Filters: []*eskip.Filter{{Name: "setPath"}}, Backend: "https://example.org"},
792+
{Id: "unknownPredicate", Predicates: []*eskip.Predicate{{Name: "UnknownPredicate"}}, Backend: "https://example.org"},
793+
{Id: "invalidPredicateParams", Predicates: []*eskip.Predicate{{Name: "QueryParam"}}, Backend: "https://example.org"},
794+
{Id: "failedBackendSplit", Predicates: []*eskip.Predicate{{Name: "Path", Args: []interface{}{"/test3"}}}, Backend: "invalid-url"},
795+
{Id: "valid3", Predicates: []*eskip.Predicate{{Name: "Path", Args: []interface{}{"/test4"}}}, Backend: "https://example.org"},
796+
}
797+
798+
routes2, invalidRoutes2 := routing.ExportProcessRouteDefs(opts, invalidRoutesSet)
799+
assert.Len(t, routes2, 1)
800+
assert.Len(t, invalidRoutes2, 5)
801+
802+
output2 := getMetricsOutput()
803+
assert.Contains(t, output2, `skipper_route_invalid{reason="unknown_filter"} 1`)
804+
assert.Contains(t, output2, `skipper_route_invalid{reason="invalid_filter_params"} 1`)
805+
assert.Contains(t, output2, `skipper_route_invalid{reason="unknown_predicate"} 1`)
806+
assert.Contains(t, output2, `skipper_route_invalid{reason="invalid_predicate_params"} 1`)
807+
assert.Contains(t, output2, `skipper_route_invalid{reason="failed_backend_split"} 1`)
808+
assert.Contains(t, output2, `skipper_route_invalid{reason="invalid_matcher"} 0`)
809+
assert.Contains(t, output2, `skipper_route_invalid{reason="other"} 0`)
810+
811+
t.Log("Phase 3: Test routes are fixed - all metrics should reset to 0")
812+
allValidRoutes := []*eskip.Route{
813+
{Id: "fixed1", Predicates: []*eskip.Predicate{{Name: "Path", Args: []interface{}{"/fixed1"}}}, Backend: "https://example.org"},
814+
{Id: "fixed2", Predicates: []*eskip.Predicate{{Name: "Path", Args: []interface{}{"/fixed2"}}}, Backend: "https://example.org"},
815+
}
816+
817+
routes3, invalidRoutes3 := routing.ExportProcessRouteDefs(opts, allValidRoutes)
818+
assert.Len(t, routes3, 2)
819+
assert.Len(t, invalidRoutes3, 0)
820+
821+
output3 := getMetricsOutput()
822+
assert.Contains(t, output3, `skipper_route_invalid{reason="unknown_filter"} 0`)
823+
assert.Contains(t, output3, `skipper_route_invalid{reason="invalid_filter_params"} 0`)
824+
assert.Contains(t, output3, `skipper_route_invalid{reason="unknown_predicate"} 0`)
825+
assert.Contains(t, output3, `skipper_route_invalid{reason="invalid_predicate_params"} 0`)
826+
assert.Contains(t, output3, `skipper_route_invalid{reason="failed_backend_split"} 0`)
827+
assert.Contains(t, output3, `skipper_route_invalid{reason="invalid_matcher"} 0`)
828+
assert.Contains(t, output3, `skipper_route_invalid{reason="other"} 0`)
829+
}

routing/export_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
var (
1010
ExportProcessRouteDef = processRouteDef
11+
ExportProcessRouteDefs = processRouteDefs
1112
ExportNewMatcher = newMatcher
1213
ExportMatch = (*matcher).match
1314
ExportProcessPredicates = processPredicates

0 commit comments

Comments
 (0)