Fixed getting latency metrics (#1855) (#1969)

Co-authored-by: Mark Mandel <[email protected]>
googleforgames · Feb 2, 2021 · 1cc5d8a · 1cc5d8a
1 parent 3cf898a
commit 1cc5d8a
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 38 deletions.
diff --git a/pkg/gameserverallocations/allocator.go b/pkg/gameserverallocations/allocator.go
@@ -37,6 +37,7 @@ import (
 	"agones.dev/agones/pkg/util/runtime"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"go.opencensus.io/tag"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials"
@@ -188,7 +189,17 @@ func (c *Allocator) Sync(stop <-chan struct{}) error {
 }
 
 // Allocate CRDHandler for allocating a gameserver.
-func (c *Allocator) Allocate(gsa *allocationv1.GameServerAllocation, stop <-chan struct{}) (k8sruntime.Object, error) {
+func (c *Allocator) Allocate(gsa *allocationv1.GameServerAllocation, stop <-chan struct{}) (out k8sruntime.Object, err error) {
+	ctx := context.Background()
+	latency := c.newMetrics(ctx)
+	defer func() {
+		if err != nil {
+			latency.setError()
+		}
+		latency.record()
+	}()
+	latency.setRequest(gsa)
+
 	// server side validation
 	if causes, ok := gsa.Validate(); !ok {
 		s := &metav1.Status{
@@ -215,8 +226,6 @@ func (c *Allocator) Allocate(gsa *allocationv1.GameServerAllocation, stop <-chan
 	}
 
 	// If multi-cluster setting is enabled, allocate base on the multicluster allocation policy.
-	var out *allocationv1.GameServerAllocation
-	var err error
 	if gsa.Spec.MultiClusterSetting.Enabled {
 		out, err = c.applyMultiClusterAllocation(gsa, stop)
 	} else {
@@ -227,6 +236,7 @@ func (c *Allocator) Allocate(gsa *allocationv1.GameServerAllocation, stop <-chan
 		c.loggerForGameServerAllocation(gsa).WithError(err).Error("allocation failed")
 		return nil, err
 	}
+	latency.setResponse(out)
 
 	return out, nil
 }
@@ -611,6 +621,20 @@ func (c *Allocator) getRandomlySelectedGS(gsa *allocationv1.GameServerAllocation
 	return &bestGSList[index]
 }
 
+// newMetrics creates a new gsa latency recorder.
+func (c *Allocator) newMetrics(ctx context.Context) *metrics {
+	ctx, err := tag.New(ctx, latencyTags...)
+	if err != nil {
+		c.baseLogger.WithError(err).Warn("failed to tag latency recorder.")
+	}
+	return &metrics{
+		ctx:              ctx,
+		gameServerLister: c.readyGameServerCache.gameServerLister,
+		logger:           c.baseLogger,
+		start:            time.Now(),
+	}
+}
+
 func addPort(ip string) string {
 	if strings.Contains(ip, ":") {
 		return ip

diff --git a/pkg/gameserverallocations/controller.go b/pkg/gameserverallocations/controller.go
@@ -15,23 +15,14 @@
 package gameserverallocations
 
 import (
-	"context"
 	"io/ioutil"
 	"mime"
 	"net/http"
 	"time"
 
-	allocationv1 "agones.dev/agones/pkg/apis/allocation/v1"
-	"agones.dev/agones/pkg/client/clientset/versioned"
-	"agones.dev/agones/pkg/client/informers/externalversions"
-	"agones.dev/agones/pkg/gameservers"
-	"agones.dev/agones/pkg/util/apiserver"
-	"agones.dev/agones/pkg/util/https"
-	"agones.dev/agones/pkg/util/runtime"
 	"github.com/heptiolabs/healthcheck"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"go.opencensus.io/tag"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8sruntime "k8s.io/apimachinery/pkg/runtime"
@@ -41,6 +32,14 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 	typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/tools/record"
+
+	allocationv1 "agones.dev/agones/pkg/apis/allocation/v1"
+	"agones.dev/agones/pkg/client/clientset/versioned"
+	"agones.dev/agones/pkg/client/informers/externalversions"
+	"agones.dev/agones/pkg/gameservers"
+	"agones.dev/agones/pkg/util/apiserver"
+	"agones.dev/agones/pkg/util/https"
+	"agones.dev/agones/pkg/util/runtime"
 )
 
 // Controller is a the GameServerAllocation controller
@@ -112,14 +111,6 @@ func (c *Controller) Run(_ int, stop <-chan struct{}) error {
 }
 
 func (c *Controller) processAllocationRequest(w http.ResponseWriter, r *http.Request, namespace string, stop <-chan struct{}) (err error) {
-	latency := c.newMetrics(r.Context())
-	defer func() {
-		if err != nil {
-			latency.setError()
-		}
-		latency.record()
-	}()
-
 	if r.Body != nil {
 		defer r.Body.Close() // nolint: errcheck
 	}
@@ -129,7 +120,6 @@ func (c *Controller) processAllocationRequest(w http.ResponseWriter, r *http.Req
 	if r.Method != http.MethodPost {
 		log.Warn("allocation handler only supports POST")
 		http.Error(w, "Method not supported", http.StatusMethodNotAllowed)
-		latency.setError()
 		return
 	}
 
@@ -138,8 +128,6 @@ func (c *Controller) processAllocationRequest(w http.ResponseWriter, r *http.Req
 		return err
 	}
 
-	latency.setRequest(gsa)
-
 	result, err := c.allocator.Allocate(gsa, stop)
 	if err != nil {
 		return err
@@ -148,25 +136,10 @@ func (c *Controller) processAllocationRequest(w http.ResponseWriter, r *http.Req
 		w.WriteHeader(int(status.Code))
 	}
 
-	latency.setResponse(result)
 	err = c.serialisation(r, w, result, scheme.Codecs)
 	return err
 }
 
-// newMetrics creates a new gsa latency recorder.
-func (c *Controller) newMetrics(ctx context.Context) *metrics {
-	ctx, err := tag.New(ctx, latencyTags...)
-	if err != nil {
-		c.baseLogger.WithError(err).Warn("failed to tag latency recorder.")
-	}
-	return &metrics{
-		ctx:              ctx,
-		gameServerLister: c.allocator.readyGameServerCache.gameServerLister,
-		logger:           c.baseLogger,
-		start:            time.Now(),
-	}
-}
-
 // allocationDeserialization processes the request and namespace, and attempts to deserialise its values
 // into a GameServerAllocation. Returns an error if it fails for whatever reason.
 func (c *Controller) allocationDeserialization(r *http.Request, namespace string) (*allocationv1.GameServerAllocation, error) {