feat: Add promethues metrics to govppmux (#1626)

* Fix parsing VPP version info strings

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

* Update integration tests

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

* Fix race condition related to retrieving stats

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

* Update version package

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

* Add prometheus metrics to govppmux

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

* Address linter review

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

* Fix integration test

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

cmd/vpp-agent/app/vpp_agent.go

@@ -162,7 +162,7 @@ func (a *VPPAgent) AfterInit() error {
 }
 
 // Close could close used resources.
-func (VPPAgent) Close() error {
+func (a *VPPAgent) Close() error {
 	return nil
 }
 

cmd/vpp-agent/main.go

@@ -19,6 +19,7 @@ package main
 import (
 	"flag"
 	"fmt"
+	"io/ioutil"
 	"log"
 	"os"
 	"time"
@@ -34,33 +35,40 @@ import (
 )
 
 const logo = `                                       __
-  _  _____  ___ _______ ____ ____ ___ / /_   
+  _  _____  ___ _______ ____ ____ ___ / /_  %s
  | |/ / _ \/ _ /___/ _ '/ _ '/ -_/ _ / __/  %s
  |___/ .__/ .__/   \_'_/\_' /\__/_//_\__/   %s
     /_/  /_/           /___/                %s
 
 `
 
 func parseVersion() {
-	ver, rev, date := version.Data()
-	agent.BuildVersion = ver
-	agent.CommitHash = rev
-	agent.BuildDate = date
 	s := flag.NewFlagSet("version", flag.ContinueOnError)
-	v := s.Bool("version", false, "Print version info and exit.")
 	s.Usage = func() {}
+	s.SetOutput(ioutil.Discard)
+	var (
+		v  = s.Bool("V", false, "Print version and exit.")
+		vv = s.Bool("version", false, "Print version info and exit.")
+	)
 	if err := s.Parse(os.Args[1:]); err == nil {
 		if *v {
+			fmt.Fprintln(os.Stdout, version.Version())
+			os.Exit(0)
+		}
+		if *vv {
 			fmt.Fprintln(os.Stdout, version.Info())
 			os.Exit(0)
 		}
 	}
+	ver, rev, date := version.Data()
+	agent.BuildVersion = ver
+	agent.CommitHash = rev
+	agent.BuildDate = date
 }
 
 func main() {
 	parseVersion()
-
-	fmt.Fprintf(os.Stderr, logo, version.Short(), version.BuiltStamp(), version.BuiltBy())
+	fmt.Fprintf(os.Stderr, logo, version.App(), version.Version(), version.BuiltOn(), version.BuiltBy())
 
 	if debug.IsEnabled() {
 		logging.DefaultLogger.SetLevel(logging.DebugLevel)

pkg/version/version.go

@@ -36,11 +36,10 @@ var buildTime time.Time
 var revision string
 
 func init() {
-	buildstampInt64, _ := strconv.ParseInt(buildDate, 10, 64)
-	if buildstampInt64 == 0 {
-		buildstampInt64 = time.Now().Unix()
+	if buildDate != "" {
+		buildstampInt64, _ := strconv.ParseInt(buildDate, 10, 64)
+		buildTime = time.Unix(buildstampInt64, 0)
 	}
-	buildTime = time.Unix(buildstampInt64, 0)
 	revision = gitCommit
 	if len(revision) > 7 {
 		revision = revision[:7]
@@ -50,8 +49,13 @@ func init() {
 	}
 }
 
-// String returns version string.
-func String() string {
+// App returns app name.
+func App() string {
+	return app
+}
+
+// Version returns version string.
+func Version() string {
 	return version
 }
 
@@ -64,22 +68,24 @@ func Short() string {
 	return fmt.Sprintf(`%s %s`, app, version)
 }
 
-func BuiltStamp() string {
-	return fmt.Sprintf("%s (%s)", buildTime.Format(time.UnixDate), timeAgo(buildTime))
+func BuiltOn() string {
+	stamp := buildTime.Format(time.UnixDate)
+	if !buildTime.IsZero() {
+		stamp += fmt.Sprintf(" (%s)", timeAgo(buildTime))
+	}
+	return stamp
 }
 
 func BuiltBy() string {
 	return fmt.Sprintf("%s@%s (%s %s/%s)",
-		buildUser, buildHost,
-		runtime.Version(), runtime.GOOS, runtime.GOARCH,
+		buildUser, buildHost, runtime.Version(), runtime.GOOS, runtime.GOARCH,
 	)
 }
 
 // Info returns string with complete version info on single line.
 func Info() string {
-	return fmt.Sprintf(`%s %s (%s) built by %s@%s on %v (%s)`,
-		app, version, revision,
-		buildUser, buildHost, buildTime.Format(time.Stamp), timeAgo(buildTime),
+	return fmt.Sprintf(`%s %s (%s) built by %s@%s on %v`,
+		app, version, revision, buildUser, buildHost, BuiltOn(),
 	)
 }
 

plugins/govppmux/client_binapi.go

@@ -17,7 +17,6 @@ package govppmux
 import (
 	"context"
 	"runtime/trace"
-	"sync/atomic"
 	"time"
 
 	govppapi "git.fd.io/govpp.git/api"
@@ -27,10 +26,6 @@ import (
 
 // NewAPIChannel returns a new API channel for communication with VPP via govpp core.
 // It uses default buffer sizes for the request and reply Go channels.
-//
-// Example of binary API call from some plugin using GOVPP:
-//      ch, _ := govpp_mux.NewAPIChannel()
-//      ch.SendRequest(req).ReceiveReply
 func (p *Plugin) NewAPIChannel() (govppapi.Channel, error) {
 	ch, err := p.vppConn.NewAPIChannel()
 	if err != nil {
@@ -45,10 +40,6 @@ func (p *Plugin) NewAPIChannel() (govppapi.Channel, error) {
 
 // NewAPIChannelBuffered returns a new API channel for communication with VPP via govpp core.
 // It allows to specify custom buffer sizes for the request and reply Go channels.
-//
-// Example of binary API call from some plugin using GOVPP:
-//      ch, _ := govpp_mux.NewAPIChannelBuffered(100, 100)
-//      ch.SendRequest(req).ReceiveReply
 func (p *Plugin) NewAPIChannelBuffered(reqChanBufSize, replyChanBufSize int) (govppapi.Channel, error) {
 	ch, err := p.vppConn.NewAPIChannelBuffered(reqChanBufSize, replyChanBufSize)
 	if err != nil {
@@ -75,14 +66,13 @@ func newGovppChan(ch govppapi.Channel, retryCfg retryConfig) *goVppChan {
 		Channel: ch,
 		retry:   retryCfg,
 	}
-	atomic.AddUint64(&stats.ChannelsCreated, 1)
-	atomic.AddUint64(&stats.ChannelsOpen, 1)
+	reportChannelsOpened()
 	return govppChan
 }
 
 func (c *goVppChan) Close() {
 	c.Channel.Close()
-	atomic.AddUint64(&stats.ChannelsOpen, ^uint64(0)) // decrement
+	reportChannelsClosed()
 }
 
 // helper struct holding info about retry configuration
@@ -130,7 +120,7 @@ func (c *goVppChan) SendRequest(request govppapi.Message) govppapi.RequestCtx {
 	// Send request now and wait for context
 	requestCtx := c.Channel.SendRequest(request)
 
-	atomic.AddUint64(&stats.RequestsSent, 1)
+	reportRequestSent(request)
 
 	// Return context with value and function which allows to send request again if needed
 	return &govppRequestCtx{
@@ -149,37 +139,30 @@ func (r *govppRequestCtx) ReceiveReply(reply govppapi.Message) error {
 	defer r.task.End()
 
 	var timeout time.Duration
-	maxRetries := r.retry.attempts
+	attempts := r.retry.attempts
 	if r.retry.timeout > 0 { // Default value is 500ms
 		timeout = r.retry.timeout
 	}
-
 	// Receive reply from original send
 	err := r.requestCtx.ReceiveReply(reply)
-
 	for retry := 1; err == core.ErrNotConnected; retry++ {
-		if retry > maxRetries {
-			// retrying failed
-			break
+		if retry > attempts {
+			break // max attempts exceeded
 		}
-		logging.Warnf("Govppmux: request retry (%d/%d), message %s in %v",
-			retry, maxRetries, r.requestMsg.GetMessageName(), timeout)
+		logging.Warnf("govppmux: request retry (%d/%d), message %s in %v",
+			retry, attempts, r.requestMsg.GetMessageName(), timeout)
 		// Wait before next attempt
 		time.Sleep(timeout)
 		// Retry request
-		trace.Logf(r.ctx, "requestRetry", "%d/%d", retry, maxRetries)
+		trace.Logf(r.ctx, "requestRetry", "%d/%d", retry, attempts)
 		err = r.sendRequest(r.requestMsg).ReceiveReply(reply)
 	}
-
-	atomic.AddUint64(&stats.RequestsDone, 1)
 	if err != nil {
-		trackError(err.Error())
-		atomic.AddUint64(&stats.RequestsFail, 1)
+		reportRequestFailed(reply, err)
+	} else {
+		reportRequestSuccess(r.requestMsg, r.start)
+		reportRepliesReceived(reply)
 	}
-
-	took := time.Since(r.start)
-	trackMsgRequestDur(r.requestMsg.GetMessageName(), took)
-
 	return err
 }
 
@@ -192,7 +175,7 @@ func (c *goVppChan) SendMultiRequest(request govppapi.Message) govppapi.MultiReq
 	// Send request now and wait for context
 	requestCtx := c.Channel.SendMultiRequest(request)
 
-	atomic.AddUint64(&stats.RequestsSent, 1)
+	reportRequestSent(request)
 
 	// Return context with value and function which allows to send request again if needed
 	return &govppMultirequestCtx{
@@ -209,19 +192,14 @@ func (r *govppMultirequestCtx) ReceiveReply(reply govppapi.Message) (bool, error
 	// Receive reply from original send
 	last, err := r.requestCtx.ReceiveReply(reply)
 	if last || err != nil {
-		took := time.Since(r.start)
-		trackMsgRequestDur(r.requestMsg.GetMessageName(), took)
-
-		atomic.AddUint64(&stats.RequestsDone, 1)
+		defer r.task.End()
 		if err != nil {
-			trackError(err.Error())
-			atomic.AddUint64(&stats.RequestsFail, 1)
+			reportRequestFailed(r.requestMsg, err)
+		} else {
+			reportRequestSuccess(r.requestMsg, r.start)
 		}
-
-		defer r.task.End()
 	} else {
-		atomic.AddUint64(&stats.RequestReplies, 1)
-		trackMsgReply(reply.GetMessageName())
+		reportRepliesReceived(reply)
 	}
 	return last, err
 }

plugins/govppmux/client_stats.go

@@ -40,6 +40,8 @@ func (p *Plugin) GetSystemStats(stats *govppapi.SystemStats) error {
 	if p.statsConn == nil {
 		return nil
 	}
+	p.statsMu.Lock()
+	defer p.statsMu.Unlock()
 	return p.statsConn.GetSystemStats(stats)
 }
 
@@ -48,6 +50,8 @@ func (p *Plugin) GetNodeStats(stats *govppapi.NodeStats) error {
 	if p.statsConn == nil {
 		return nil
 	}
+	p.statsMu.Lock()
+	defer p.statsMu.Unlock()
 	return p.statsConn.GetNodeStats(stats)
 }
 
@@ -56,6 +60,8 @@ func (p *Plugin) GetInterfaceStats(stats *govppapi.InterfaceStats) error {
 	if p.statsConn == nil {
 		return nil
 	}
+	p.statsMu.Lock()
+	defer p.statsMu.Unlock()
 	return p.statsConn.GetInterfaceStats(stats)
 }
 
@@ -64,6 +70,8 @@ func (p *Plugin) GetErrorStats(stats *govppapi.ErrorStats) error {
 	if p.statsConn == nil {
 		return nil
 	}
+	p.statsMu.Lock()
+	defer p.statsMu.Unlock()
 	return p.statsConn.GetErrorStats(stats)
 }
 
@@ -72,5 +80,7 @@ func (p *Plugin) GetBufferStats(stats *govppapi.BufferStats) error {
 	if p.statsConn == nil {
 		return nil
 	}
+	p.statsMu.Lock()
+	defer p.statsMu.Unlock()
 	return p.statsConn.GetBufferStats(stats)
 }