From 5cf911794f7de2e709d13ce98db3742cec4c05be Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Tue, 21 Dec 2021 16:09:20 -0500 Subject: [PATCH] Fix metrics test by ensuring each path has a unique name (#3443) --- server/service/endpoint_utils.go | 1 + server/service/handler_test.go | 117 ++++++++++++++++--------------- 2 files changed, 61 insertions(+), 57 deletions(-) diff --git a/server/service/endpoint_utils.go b/server/service/endpoint_utils.go index a8469b3442..5a481c64fc 100644 --- a/server/service/endpoint_utils.go +++ b/server/service/endpoint_utils.go @@ -324,6 +324,7 @@ func (e *UserAuthEndpointer) handle(path string, f handlerFunc, v interface{}, v endpoint := e.makeEndpoint(f, v) e.r.Handle(versionedPath, endpoint).Name(nameAndVerb).Methods(verb) for _, alias := range e.alternativePaths { + nameAndVerb := getNameFromPathAndVerb(verb, alias) versionedPath := strings.Replace(alias, "/_version_/", fmt.Sprintf("/{fleetversion:(?:%s)}/", strings.Join(versions, "|")), 1) e.r.Handle(versionedPath, endpoint).Name(nameAndVerb).Methods(verb) } diff --git a/server/service/handler_test.go b/server/service/handler_test.go index 43b5ab7c2c..38e61089af 100644 --- a/server/service/handler_test.go +++ b/server/service/handler_test.go @@ -14,6 +14,7 @@ import ( "github.com/fleetdm/fleet/v4/server/mock" kitlog "github.com/go-kit/kit/log" "github.com/gorilla/mux" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -248,6 +249,9 @@ func TestAPIRoutesMetrics(t *testing.T) { // collect the route names routeNames := make(map[string]bool) err = router.Walk(func(route *mux.Route, _ *mux.Router, _ []*mux.Route) error { + if _, ok := routeNames[route.GetName()]; ok { + t.Errorf("duplicate route name: %s", route.GetName()) + } routeNames[route.GetName()] = true return nil }) @@ -322,57 +326,57 @@ func TestAPIRoutesMetrics(t *testing.T) { "promhttp_metric_handler_requests_total": 0, } - //wantCounts := map[string]int{ - // "go_gc_duration_seconds": 5, // quantiles 0, .25, .5, .75 and 1 - // "go_gc_duration_seconds_sum": 1, - // "go_gc_duration_seconds_count": 1, - // "go_goroutines": 1, - // "go_info": 1, - // "go_memstats_alloc_bytes": 1, - // "go_memstats_alloc_bytes_total": 1, - // "go_memstats_buck_hash_sys_bytes": 1, - // "go_memstats_frees_total": 1, - // "go_memstats_gc_cpu_fraction": 1, - // "go_memstats_gc_sys_bytes": 1, - // "go_memstats_heap_alloc_bytes": 1, - // "go_memstats_heap_idle_bytes": 1, - // "go_memstats_heap_inuse_bytes": 1, - // "go_memstats_heap_objects": 1, - // "go_memstats_heap_released_bytes": 1, - // "go_memstats_heap_sys_bytes": 1, - // "go_memstats_last_gc_time_seconds": 1, - // "go_memstats_lookups_total": 1, - // "go_memstats_mallocs_total": 1, - // "go_memstats_mcache_inuse_bytes": 1, - // "go_memstats_mcache_sys_bytes": 1, - // "go_memstats_mspan_inuse_bytes": 1, - // "go_memstats_mspan_sys_bytes": 1, - // "go_memstats_next_gc_bytes": 1, - // "go_memstats_other_sys_bytes": 1, - // "go_memstats_stack_inuse_bytes": 1, - // "go_memstats_stack_sys_bytes": 1, - // "go_memstats_sys_bytes": 1, - // "go_threads": 1, - // "http_request_duration_seconds_bucket": len(reqs) * (len(prometheus.DefBuckets) + 1), // +1 for the last bucket, ending at +Inf - // "http_request_duration_seconds_sum": len(reqs), - // "http_request_duration_seconds_count": len(reqs), - // "http_request_size_bytes_bucket": len(reqs) * 6, // size of req size buckets - // "http_request_size_bytes_sum": len(reqs), - // "http_request_size_bytes_count": len(reqs), - // "http_requests_total": len(reqs), - // "http_response_size_bytes_bucket": len(reqs) * 6, // size of res size buckets - // "http_response_size_bytes_sum": len(reqs), - // "http_response_size_bytes_count": len(reqs), - // "process_cpu_seconds_total": 1, - // "process_max_fds": 1, - // "process_open_fds": 1, - // "process_resident_memory_bytes": 1, - // "process_start_time_seconds": 1, - // "process_virtual_memory_bytes": 1, - // "process_virtual_memory_max_bytes": 1, - // "promhttp_metric_handler_requests_in_flight": 1, - // "promhttp_metric_handler_requests_total": 3, // status codes 200, 500, 503 - //} + wantCounts := map[string]int{ + "go_gc_duration_seconds": 5, // quantiles 0, .25, .5, .75 and 1 + "go_gc_duration_seconds_sum": 1, + "go_gc_duration_seconds_count": 1, + "go_goroutines": 1, + "go_info": 1, + "go_memstats_alloc_bytes": 1, + "go_memstats_alloc_bytes_total": 1, + "go_memstats_buck_hash_sys_bytes": 1, + "go_memstats_frees_total": 1, + "go_memstats_gc_cpu_fraction": 1, + "go_memstats_gc_sys_bytes": 1, + "go_memstats_heap_alloc_bytes": 1, + "go_memstats_heap_idle_bytes": 1, + "go_memstats_heap_inuse_bytes": 1, + "go_memstats_heap_objects": 1, + "go_memstats_heap_released_bytes": 1, + "go_memstats_heap_sys_bytes": 1, + "go_memstats_last_gc_time_seconds": 1, + "go_memstats_lookups_total": 1, + "go_memstats_mallocs_total": 1, + "go_memstats_mcache_inuse_bytes": 1, + "go_memstats_mcache_sys_bytes": 1, + "go_memstats_mspan_inuse_bytes": 1, + "go_memstats_mspan_sys_bytes": 1, + "go_memstats_next_gc_bytes": 1, + "go_memstats_other_sys_bytes": 1, + "go_memstats_stack_inuse_bytes": 1, + "go_memstats_stack_sys_bytes": 1, + "go_memstats_sys_bytes": 1, + "go_threads": 1, + "http_request_duration_seconds_bucket": len(reqs) * (len(prometheus.DefBuckets) + 1), // +1 for the last bucket, ending at +Inf + "http_request_duration_seconds_sum": len(reqs), + "http_request_duration_seconds_count": len(reqs), + "http_request_size_bytes_bucket": len(reqs) * 6, // size of req size buckets + "http_request_size_bytes_sum": len(reqs), + "http_request_size_bytes_count": len(reqs), + "http_requests_total": len(reqs), + "http_response_size_bytes_bucket": len(reqs) * 6, // size of res size buckets + "http_response_size_bytes_sum": len(reqs), + "http_response_size_bytes_count": len(reqs), + "process_cpu_seconds_total": 1, + "process_max_fds": 1, + "process_open_fds": 1, + "process_resident_memory_bytes": 1, + "process_start_time_seconds": 1, + "process_virtual_memory_bytes": 1, + "process_virtual_memory_max_bytes": 1, + "promhttp_metric_handler_requests_in_flight": 1, + "promhttp_metric_handler_requests_total": 3, // status codes 200, 500, 503 + } s := bufio.NewScanner(rr.Body) for s.Scan() { @@ -410,12 +414,11 @@ func TestAPIRoutesMetrics(t *testing.T) { } require.NoError(t, s.Err()) - // TODO: improve count checks because it's easy for them to fail with the route changes - //for name, got := range metricCounts { - // want, ok := wantCounts[name] - // require.True(t, ok, "unexpected metric: %s", name) - // require.Equal(t, want, got, name) - //} + for name, got := range metricCounts { + want, ok := wantCounts[name] + require.True(t, ok, "unexpected metric: %s", name) + require.Equal(t, want, got, name) + } } var reSimpleVar, reNumVar = regexp.MustCompile(`\{(\w+)\}`), regexp.MustCompile(`\{\w+:[^\}]+\}`)