From fe129b4268abe24dc3b1338536282d28383affb6 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Thu, 19 Sep 2024 14:02:41 -0400 Subject: [PATCH] fix(metrics): Fix matching logic for authorize session endpoint (#5121) The target authorize session endpoint allows for alternative target identifiers besides the public id. For this to function correctly, the grpc gateway path definition for the endpoint deviates from the common pattern used for other endpoints that include ids. When this change was done, it was not accounted for in the logic used to capture the prometheus metrics for api requests to this particular endpoint, resulting in the metrics never incrementing for the authorize session endpoint. This fix updates the matching logic used for the metrics to ensure that the metrics are incremented for this endpoint. See: c149bc4a7998809e675d0ab6e30d03ef1d53bd0c --- .../daemon/controller/internal/metric/api.go | 8 +++--- .../controller/internal/metric/api_test.go | 27 ++++++++++++++++++- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/internal/daemon/controller/internal/metric/api.go b/internal/daemon/controller/internal/metric/api.go index ef4eb42aaa..614c3449de 100644 --- a/internal/daemon/controller/internal/metric/api.go +++ b/internal/daemon/controller/internal/metric/api.go @@ -101,12 +101,12 @@ func apiPathsAndMethods() map[string][]string { func buildRegexFromPath(p string) *regexp.Regexp { // We only care about how grpc-gateway will route to specific handlers. // As long as there is at least 1 character that is part of a path segment - // (not a '/', '?', or ':' we have identified an id for the sake of routing. - const idRegexp = "[^\\/\\?\\:]+" + // (not a '?', or ':' we have identified an id for the sake of routing. + const idRegexp = "[^\\?\\:]+" - // Replace any tag in the form of {id} or {auth_method_id} with the above + // Replace any tag in the form of {id}, {id**}, or {auth_method_id} with the above // regex so we can match paths to that when measuring requests. - pWithId := string(regexp.MustCompile("\\{[^\\}]*id\\}").ReplaceAll([]byte(p), []byte(idRegexp))) + pWithId := string(regexp.MustCompile("\\{[^\\}]*id(\\=\\*\\*)?\\}").ReplaceAll([]byte(p), []byte(idRegexp))) // Escape everything except for our id regexp. var seg []string diff --git a/internal/daemon/controller/internal/metric/api_test.go b/internal/daemon/controller/internal/metric/api_test.go index 7255af3409..10633cd5c1 100644 --- a/internal/daemon/controller/internal/metric/api_test.go +++ b/internal/daemon/controller/internal/metric/api_test.go @@ -41,12 +41,12 @@ func TestBuildRegexFromPath(t *testing.T) { "/v1/pathsomething/am_1234567890:authenticate", "/v1/pathsomething/{id}:authenticate", "/v1/pathsomething/{auth_method}:authenticate", + "/v1/pathsomething/am_1234567890/:authenticate", }, dont: []string{ "/v1/pathsomething:authenticate", "/v1/pathsomething:authenticate:authenticate", "/v1/pathsomething/:authenticate:authenticate", - "/v1/pathsomething/am_1234567890/:authenticate", "/v1/pathsomething/?whatabout=:authenticate", }, }, @@ -130,6 +130,31 @@ func TestPathLabel(t *testing.T) { in: "v1/accounts/a_1234567890:set-password", want: "/v1/accounts/{id}:set-password", }, + { + // using target id + in: "/v1/targets/tssh_12345789:authorize-session", + want: "/v1/targets/{id=**}:authorize-session", + }, + { + // using target name + in: "/v1/targets/foo-target:authorize-session", + want: "/v1/targets/{id=**}:authorize-session", + }, + { + // using target name with a space + in: "/v1/targets/foo target:authorize-session", + want: "/v1/targets/{id=**}:authorize-session", + }, + { + // using target name with a slash + in: "/v1/targets/foo/target:authorize-session", + want: "/v1/targets/{id=**}:authorize-session", + }, + { + // using alias + in: "/v1/targets/foo.test:authorize-session", + want: "/v1/targets/{id=**}:authorize-session", + }, { // mistype the custom action in: "/v1/accounts/a_1234567890:set-passwords",