fix: report metrics from API Server struct

The StatsD reported was never being set and as a result, no metrics were
being reported from inside the API Server. Use an explict APIServerDeps
struct and pass in the StatsD reporter. Use static checks to ensure all
fields are always set in souce code.

Bump golangci-lint version to 1.53.3.

.github/workflows/lint.yml

@@ -20,5 +20,5 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.52.2
+          version: v1.53.3
           args: --config=".golangci-prod.toml" --new-from-rev=HEAD~1 --max-same-issues=0 --max-issues-per-linter=0

.golangci-prod.toml

@@ -77,6 +77,10 @@
     # problems with the error wrapping scheme introduced in Go 1.13.
     "errorlint",
 
+    # https://github.com/GaijinEntertainment/go-exhaustruct
+    # Checks if all structure fields are initialized
+    "exhaustruct",
+
     # https://github.com/ashanbrown/forbidigo
     # Forbids identifiers.
     "forbidigo",
@@ -195,6 +199,12 @@
     # Default: false
     check-type-assertions = true
 
+  [linters-settings.exhaustruct]
+    # List of regular expressions to match struct packages and names.
+    # If this list is empty, all structs are tested.
+    # Default: []
+    include = ["github\\.com/goto/compass/internal/server/v1beta1\\.APIServerDeps"]
+
   [linters-settings.govet]
     # Report about shadowed variables.
     # Default: false
@@ -569,10 +579,6 @@
     text = 'declaration of "(err|ctx)" shadows declaration at'
     linters = ["govet"]
 
-  [[issues.exclude-rules]]
-    text = "can be replaced by http\\.Status.*"
-    linters = ["usestdlibvars"]
-
   [[issues.exclude-rules]]
     path = "cli/.*"
     linters = ["dupl"]
@@ -586,6 +592,7 @@
       "lll",
       "gocognit",
       "goconst",
+      "exhaustruct",
     ]
 
   [[issues.exclude-rules]]

.golangci.toml

@@ -73,6 +73,10 @@
     # problems with the error wrapping scheme introduced in Go 1.13.
     "errorlint",
 
+    # https://github.com/GaijinEntertainment/go-exhaustruct
+    # Checks if all structure fields are initialized
+    "exhaustruct",
+
     # https://github.com/ashanbrown/forbidigo
     # Forbids identifiers.
     "forbidigo",
@@ -168,6 +172,12 @@
     # Default: false
     check-type-assertions = true
 
+  [linters-settings.exhaustruct]
+    # List of regular expressions to match struct packages and names.
+    # If this list is empty, all structs are tested.
+    # Default: []
+    include = ["github\\.com/goto/compass/internal/server/v1beta1\\.APIServerDeps"]
+
   [linters-settings.govet]
     # Report about shadowed variables.
     # Default: false
@@ -395,10 +405,6 @@
     text = 'declaration of "(err|ctx)" shadows declaration at'
     linters = ["govet"]
 
-  [[issues.exclude-rules]]
-    text = "can be replaced by http\\.Status.*"
-    linters = ["usestdlibvars"]
-
   [[issues.exclude-rules]]
     path = "cli/.*"
     linters = ["dupl"]
@@ -412,6 +418,7 @@
       "lll",
       "gocognit",
       "goconst",
+      "exhaustruct",
     ]
 
   [[issues.exclude-rules]]

internal/server/server.go

@@ -73,15 +73,16 @@ func Serve(
 	tagTemplateService handlersv1beta1.TagTemplateService,
 	userService handlersv1beta1.UserService,
 ) error {
-	v1beta1Handler := handlersv1beta1.NewAPIServer(
-		logger,
-		assetService,
-		starService,
-		discussionService,
-		tagService,
-		tagTemplateService,
-		userService,
-	)
+	v1beta1Handler := handlersv1beta1.NewAPIServer(handlersv1beta1.APIServerDeps{
+		AssetSvc:       assetService,
+		StarSvc:        starService,
+		DiscussionSvc:  discussionService,
+		TagSvc:         tagService,
+		TagTemplateSvc: tagTemplateService,
+		UserSvc:        userService,
+		Logger:         logger,
+		StatsD:         statsdReporter,
+	})
 
 	healthHandler := health.NewHandler()
 

internal/server/v1beta1/asset_test.go

@@ -199,7 +199,7 @@ func TestGetAllAssets(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockAssetSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetAllAssets(ctx, tc.Request)
 			code := status.Code(err)
@@ -338,7 +338,7 @@ func TestGetAssetByID(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockAssetSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetAssetByID(ctx, &compassv1beta1.GetAssetByIDRequest{Id: assetID})
 			code := status.Code(err)
@@ -542,7 +542,7 @@ func TestUpsertAsset(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockAssetSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.UpsertAsset(ctx, tc.Request)
 			code := status.Code(err)
@@ -891,7 +891,7 @@ func TestUpsertPatchAsset(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockAssetSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.UpsertPatchAsset(ctx, tc.Request)
 			code := status.Code(err)
@@ -970,7 +970,7 @@ func TestDeleteAsset(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockAssetSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			_, err := handler.DeleteAsset(ctx, &compassv1beta1.DeleteAssetRequest{Id: tc.AssetID})
 			code := status.Code(err)
@@ -1071,7 +1071,7 @@ func TestGetAssetStargazers(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, nil, mockStarSvc, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{StarSvc: mockStarSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetAssetStargazers(ctx, tc.Request)
 			code := status.Code(err)
@@ -1202,7 +1202,7 @@ func TestGetAssetVersionHistory(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockAssetSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetAssetVersionHistory(ctx, tc.Request)
 			code := status.Code(err)
@@ -1322,7 +1322,7 @@ func TestGetAssetByVersion(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockAssetSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetAssetByVersion(ctx, tc.Request)
 			code := status.Code(err)
@@ -1495,7 +1495,7 @@ func TestCreateAssetProbe(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockAssetSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.CreateAssetProbe(ctx, tc.Request)
 			code := status.Code(err)

internal/server/v1beta1/comment_test.go

@@ -105,7 +105,7 @@ func TestCreateComment(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, nil, nil, mockSvc, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.CreateComment(ctx, tc.Request)
 			code := status.Code(err)
@@ -232,7 +232,7 @@ func TestGetAllComments(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, nil, nil, mockSvc, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetAllComments(ctx, tc.Request)
 			code := status.Code(err)
@@ -359,7 +359,7 @@ func TestGetComment(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, nil, nil, mockSvc, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetComment(ctx, tc.Request)
 			code := status.Code(err)
@@ -504,7 +504,7 @@ func TestUpdateComment(t *testing.T) {
 			defer mockSvc.AssertExpectations(t)
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
-			handler := NewAPIServer(logger, nil, nil, mockSvc, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			_, err := handler.UpdateComment(ctx, tc.Request)
 			code := status.Code(err)
@@ -614,7 +614,7 @@ func TestDeleteComment(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, nil, nil, mockSvc, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			_, err := handler.DeleteComment(ctx, tc.Request)
 			code := status.Code(err)

internal/server/v1beta1/discussion_test.go

@@ -124,7 +124,7 @@ func TestGetAllDiscussions(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, nil, nil, mockSvc, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetAllDiscussions(ctx, tc.Request)
 			code := status.Code(err)
@@ -253,7 +253,7 @@ func TestCreateDiscussion(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, nil, nil, mockSvc, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 			_, err := handler.CreateDiscussion(ctx, tc.Request)
 			code := status.Code(err)
 			if code != tc.ExpectStatus {
@@ -352,7 +352,7 @@ func TestGetDiscussion(t *testing.T) {
 			defer mockSvc.AssertExpectations(t)
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
-			handler := NewAPIServer(logger, nil, nil, mockSvc, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetDiscussion(ctx, tc.Request)
 			code := status.Code(err)
@@ -491,7 +491,7 @@ func TestPatchDiscussion(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, nil, nil, mockSvc, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 			_, err := handler.PatchDiscussion(ctx, tc.Request)
 			code := status.Code(err)
 			if code != tc.ExpectStatus {

internal/server/v1beta1/lineage_test.go

@@ -61,7 +61,7 @@ func TestGetLineageGraph(t *testing.T) {
 			mockSvc.EXPECT().GetLineage(ctx, nodeURN, asset.LineageQuery{Level: level, Direction: direction}).Return(lineage, nil)
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.GetGraph(ctx, &compassv1beta1.GetGraphRequest{
 				Urn:       nodeURN,

internal/server/v1beta1/search_test.go

@@ -226,7 +226,7 @@ func TestSearch(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.SearchAssets(ctx, tc.Request)
 			code := status.Code(err)
@@ -323,7 +323,7 @@ func TestSuggest(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			got, err := handler.SuggestAssets(ctx, tc.Request)
 			code := status.Code(err)
@@ -495,7 +495,7 @@ func TestGroupAssets(t *testing.T) {
 
 			mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil)
 
-			handler := NewAPIServer(logger, mockSvc, nil, nil, nil, nil, mockUserSvc)
+			handler := NewAPIServer(APIServerDeps{AssetSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			expected, err := handler.GroupAssets(ctx, tc.Request)
 			code := status.Code(err)

internal/server/v1beta1/server.go

@@ -28,23 +28,27 @@ type APIServer struct {
 
 var errMissingUserInfo = errors.New("missing user information")
 
-func NewAPIServer(
-	logger log.Logger,
-	assetService AssetService,
-	starService StarService,
-	discussionService DiscussionService,
-	tagService TagService,
-	tagTemplateService TagTemplateService,
-	userService UserService,
-) *APIServer {
+type APIServerDeps struct {
+	AssetSvc       AssetService
+	StarSvc        StarService
+	DiscussionSvc  DiscussionService
+	TagSvc         TagService
+	TagTemplateSvc TagTemplateService
+	UserSvc        UserService
+	Logger         log.Logger
+	StatsD         StatsDClient
+}
+
+func NewAPIServer(d APIServerDeps) *APIServer {
 	return &APIServer{
-		assetService:       assetService,
-		starService:        starService,
-		discussionService:  discussionService,
-		tagService:         tagService,
-		tagTemplateService: tagTemplateService,
-		userService:        userService,
-		logger:             logger,
+		assetService:       d.AssetSvc,
+		starService:        d.StarSvc,
+		discussionService:  d.DiscussionSvc,
+		tagService:         d.TagSvc,
+		tagTemplateService: d.TagTemplateSvc,
+		userService:        d.UserSvc,
+		statsDReporter:     d.StatsD,
+		logger:             d.Logger,
 	}
 }
 

internal/server/v1beta1/server_test.go

@@ -61,7 +61,7 @@ func TestValidateUserInCtx(t *testing.T) {
 				tc.Setup(ctx, mockUserSvc)
 			}
 
-			handler := handlersv1beta1.NewAPIServer(logger, nil, mockStarSvc, nil, nil, nil, mockUserSvc)
+			handler := handlersv1beta1.NewAPIServer(handlersv1beta1.APIServerDeps{StarSvc: mockStarSvc, UserSvc: mockUserSvc, Logger: logger})
 
 			_, err := handler.ValidateUserInCtx(ctx)
 			code := status.Code(err)