feat: Add metrics registry (#1522)

* feat: Add metrics registry

Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

* Protect access with RWMutex

Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

* Add missing RUnlock for mutex

Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

pkg/metrics/registry.go

@@ -14,15 +14,53 @@
 
 package metrics
 
-/*
-import "github.com/golang/protobuf/proto"
+import (
+	"fmt"
+	"reflect"
+	"sync"
 
-var registeredMetrics = make(map[string]func() proto.Message)
+	"github.com/ligato/vpp-agent/pkg/models"
+)
 
-func Register(pb proto.Message, fn func() proto.Message) {
-    models.
-    if _, ok := registeredMetrics; ok {
-        panic("duplicate registration for metrics %s", )
-    }
+var (
+	mu                sync.RWMutex
+	registeredMetrics = make(map[string]Retriever)
+)
+
+// Retriever defines function that returns metrics data
+type Retriever func() interface{}
+
+// Register registers given type with retriever to metrics.
+func Register(metricType interface{}, retrieverFunc Retriever) {
+	model, err := models.DefaultRegistry.GetModelFor(metricType)
+	if err != nil {
+		panic(fmt.Sprintf("type %T not registered as model", metricType))
+	}
+	if model.Spec().Class != "metrics" {
+		panic(fmt.Sprintf("model %v not registered with class metrics", model.Name()))
+	}
+	mu.Lock()
+	defer mu.Unlock()
+	if _, ok := registeredMetrics[model.Name()]; ok {
+		panic(fmt.Sprintf("duplicate registration for metrics model %s", model.Name()))
+	}
+	registeredMetrics[model.Name()] = retrieverFunc
+}
+
+// Retrieve calls registered retriever for given metric and sets returned data to metric.
+func Retrieve(metric interface{}) error {
+	model, err := models.DefaultRegistry.GetModelFor(metric)
+	if err != nil {
+		return fmt.Errorf("type %T not registered as model", metric)
+	}
+	mu.RLock()
+	retriever, ok := registeredMetrics[model.Name()]
+	if !ok {
+		mu.RUnlock()
+		return fmt.Errorf("metric %v does not have registered retriever", model.Name())
+	}
+	mu.RUnlock()
+	data := retriever()
+	reflect.ValueOf(metric).Elem().Set(reflect.ValueOf(data).Elem())
+	return nil
 }
-*/

pkg/metrics/registry_test.go

@@ -0,0 +1,59 @@
+//  Copyright (c) 2019 Cisco and/or its affiliates.
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at:
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+
+package metrics
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+
+	"github.com/ligato/vpp-agent/pkg/models"
+)
+
+type TestMetrics struct {
+	TestValue int
+}
+
+var n = 0
+
+func GetTestMetrics() *TestMetrics {
+	return &TestMetrics{
+		TestValue: n,
+	}
+}
+
+func TestRetrieve(t *testing.T) {
+	RegisterTestingT(t)
+
+	models.DefaultRegistry = models.NewRegistry()
+
+	_, err := models.DefaultRegistry.Register(&TestMetrics{}, models.Spec{
+		Module: "testmodule",
+		Type:   "testtype",
+		Class:  "metrics",
+	})
+	Expect(err).ToNot(HaveOccurred())
+
+	Register(&TestMetrics{}, func() interface{} {
+		return GetTestMetrics()
+	})
+
+	n = 1
+
+	var metricData TestMetrics
+	err = Retrieve(&metricData)
+	Expect(err).ToNot(HaveOccurred())
+	Expect(metricData.TestValue).To(Equal(1))
+}

pkg/models/models.go

@@ -34,9 +34,9 @@ func RegisteredModels() []RegisteredModel {
 	return DefaultRegistry.RegisteredModels()
 }
 
-// GetModel returns registered model for given model path.
-func GetModel(path string) (RegisteredModel, error) {
-	return DefaultRegistry.GetModel(path)
+// GetModel returns registered model for given model name.
+func GetModel(name string) (RegisteredModel, error) {
+	return DefaultRegistry.GetModel(name)
 }
 
 // GetModelFor returns model registered in DefaultRegistry for given proto message.

pkg/models/registry.go

@@ -44,18 +44,18 @@ func NewRegistry() *Registry {
 	}
 }
 
-// GetModel returns registered model for the given model path
+// GetModel returns registered model for the given model name
 // or error if model is not found.
-func (r *Registry) GetModel(path string) (RegisteredModel, error) {
-	model, found := r.modelNames[path]
+func (r *Registry) GetModel(name string) (RegisteredModel, error) {
+	model, found := r.modelNames[name]
 	if !found {
-		return RegisteredModel{}, fmt.Errorf("no model registered for path %v", path)
+		return RegisteredModel{}, fmt.Errorf("no model registered for name %v", name)
 	}
 	return *model, nil
 }
 
 // GetModelFor returns registered model for the given proto message.
-func (r *Registry) GetModelFor(x proto.Message) (RegisteredModel, error) {
+func (r *Registry) GetModelFor(x interface{}) (RegisteredModel, error) {
 	t := reflect.TypeOf(x)
 	model, found := r.registeredTypes[t]
 	if !found {
@@ -85,8 +85,8 @@ func (r *Registry) RegisteredModels() []RegisteredModel {
 
 // Register registers a protobuf message with given model specification.
 // If spec.Class is unset empty it defaults to 'config'.
-func (r *Registry) Register(pb proto.Message, spec Spec, opts ...ModelOption) (*RegisteredModel, error) {
-	goType := reflect.TypeOf(pb)
+func (r *Registry) Register(x interface{}, spec Spec, opts ...ModelOption) (*RegisteredModel, error) {
+	goType := reflect.TypeOf(x)
 
 	// Check go type duplicate registration
 	if m, ok := r.registeredTypes[goType]; ok {
@@ -112,13 +112,16 @@ func (r *Registry) Register(pb proto.Message, spec Spec, opts ...ModelOption) (*
 	}
 
 	model := &RegisteredModel{
-		spec:      spec,
-		goType:    goType,
-		protoName: proto.MessageName(pb),
+		spec:   spec,
+		goType: goType,
+	}
+
+	if pb, ok := x.(proto.Message); ok {
+		model.protoName = proto.MessageName(pb)
 	}
 
 	// Use GetName as fallback for generating name
-	if _, ok := pb.(named); ok {
+	if _, ok := x.(named); ok {
 		model.nameFunc = func(obj interface{}) (s string, e error) {
 			return obj.(named).GetName(), nil
 		}

plugins/govppmux/model/metrics.proto

@@ -9,4 +9,6 @@ message Metrics {
     uint64 requests_sent = 3;
     uint64 requests_done = 4;
     uint64 requests_fail = 5;
+
+    uint64 request_replies = 6;
 }

plugins/govppmux/stats.go

@@ -24,7 +24,6 @@ import (
 )
 
 var (
-	//metricz vpp_client.Metrics
 	stats   Stats
 	statsMu sync.RWMutex
 )
@@ -46,14 +45,6 @@ func GetStats() *Stats {
 // Stats defines various statistics for govppmux plugin.
 type Stats struct {
 	vpp_client.Metrics
-	/*ChannelsCreated uint64
-	ChannelsOpen    uint64
-
-	RequestsSent   uint64
-	RequestsDone   uint64
-	RequestsErrors uint64*/
-
-	RequestReplies uint64
 
 	Errors metrics.Calls
 
@@ -125,6 +116,9 @@ func trackError(m string) {
 }
 
 func init() {
+	metrics.Register(&vpp_client.Metrics{}, func() interface{} {
+		return &GetStats().Metrics
+	})
 	expvar.Publish(vpp_client.MetricsModel.Name(), expvar.Func(func() interface{} {
 		return &GetStats().Metrics
 	}))

plugins/telemetry/telemetry.go

@@ -16,14 +16,12 @@ package telemetry
 
 import (
 	"context"
-	"expvar"
 	"fmt"
 	"net/http"
 	"os"
 	"sync"
 	"time"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/gorilla/mux"
 	"github.com/ligato/cn-infra/infra"
 	"github.com/ligato/cn-infra/logging"
@@ -34,6 +32,8 @@ import (
 	"github.com/unrolled/render"
 
 	"github.com/ligato/vpp-agent/api/configurator"
+	"github.com/ligato/vpp-agent/pkg/metrics"
+	"github.com/ligato/vpp-agent/pkg/models"
 	"github.com/ligato/vpp-agent/plugins/govppmux"
 	"github.com/ligato/vpp-agent/plugins/telemetry/vppcalls"
 
@@ -223,20 +223,14 @@ func metricsHandler(formatter *render.Render) http.HandlerFunc {
 			return
 		}
 		metric := vars["metric"]
-		var data proto.Message
-		expvar.Do(func(kv expvar.KeyValue) {
-			if data != nil {
-				return
-			}
-			if kv.Key == metric {
-				if fn, ok := kv.Value.(expvar.Func); ok {
-					data = fn.Value().(proto.Message)
-					return
-				}
-			}
-		})
-		if data == nil {
-			_ = formatter.JSON(w, http.StatusNotFound, struct{}{})
+		model, err := models.DefaultRegistry.GetModel(metric)
+		if err != nil {
+			_ = formatter.JSON(w, http.StatusNotFound, struct{ Error string }{err.Error()})
+			return
+		}
+		data := model.NewInstance()
+		if err := metrics.Retrieve(data); err != nil {
+			_ = formatter.JSON(w, http.StatusInternalServerError, struct{ Error string }{err.Error()})
 			return
 		}
 		_ = formatter.JSON(w, 200, data)