cs: refactor metrics approach

go/cs/BUILD.bazel

@@ -15,7 +15,6 @@ go_library(
         "//go/cs/beaconing/grpc:go_default_library",
         "//go/cs/config:go_default_library",
         "//go/cs/ifstate:go_default_library",
-        "//go/cs/metrics:go_default_library",
         "//go/cs/onehop:go_default_library",
         "//go/cs/segreg/grpc:go_default_library",
         "//go/cs/segreq:go_default_library",

go/cs/beaconing/BUILD.bazel

@@ -18,7 +18,6 @@ go_library(
     deps = [
         "//go/cs/beacon:go_default_library",
         "//go/cs/ifstate:go_default_library",
-        "//go/cs/metrics:go_default_library",
         "//go/lib/addr:go_default_library",
         "//go/lib/common:go_default_library",
         "//go/lib/ctrl/seg:go_default_library",
@@ -45,7 +44,6 @@ go_test(
     srcs = [
         "extender_test.go",
         "handler_test.go",
-        "main_test.go",
         "originator_test.go",
         "propagator_test.go",
         "registrar_test.go",
@@ -57,7 +55,6 @@ go_test(
         "//go/cs/beacon:go_default_library",
         "//go/cs/beaconing/mock_beaconing:go_default_library",
         "//go/cs/ifstate:go_default_library",
-        "//go/cs/metrics:go_default_library",
         "//go/lib/addr:go_default_library",
         "//go/lib/common:go_default_library",
         "//go/lib/ctrl/seg:go_default_library",

go/cs/beaconing/extender.go

@@ -75,7 +75,7 @@ func (s *DefaultExtender) Extend(ctx context.Context, pseg *seg.PathSegment,
 		return serrors.New("ingress must only be zero in first hop")
 	}
 	if ingress != 0 && firstHop {
-		return serrors.New("ingress must be zero in first hop", "ingress_id", ingress)
+		return serrors.New("ingress must be zero in first hop", "ingress_interface", ingress)
 	}
 	if ingress == 0 && egress == 0 {
 		return serrors.New("ingress and egress must not be both 0")
@@ -121,7 +121,8 @@ func (s *DefaultExtender) createPeerEntries(egress common.IFIDType, peers []comm
 	for _, peer := range peers {
 		peerEntry, err := s.createPeerEntry(peer, egress, ts, beta)
 		if err != nil {
-			log.Debug("Ignoring peer link upon error", "task", s.Task, "ifid", peer, "err", err)
+			log.Debug("Ignoring peer link upon error",
+				"task", s.Task, "peer_interface", peer, "err", err)
 			continue
 		}
 		peerEntries = append(peerEntries, peerEntry)
@@ -135,7 +136,7 @@ func (s *DefaultExtender) createHopEntry(ingress, egress common.IFIDType, ts tim
 	remoteInMTU, err := s.remoteMTU(ingress)
 	if err != nil {
 		return seg.HopEntry{}, serrors.WrapStr("checking remote ingress interface (mtu)", err,
-			"ifid", ingress)
+			"interfaces", ingress)
 	}
 	hopF := s.createHopF(uint16(ingress), uint16(egress), ts, beta)
 	return seg.HopEntry{
@@ -155,7 +156,7 @@ func (s *DefaultExtender) createPeerEntry(ingress, egress common.IFIDType, ts ti
 	remoteInIA, remoteInIfID, remoteInMTU, err := s.remoteInfo(ingress)
 	if err != nil {
 		return seg.PeerEntry{}, serrors.WrapStr("checking remote ingress interface", err,
-			"ifid", ingress)
+			"ingress_interface", ingress)
 	}
 	hopF := s.createHopF(uint16(ingress), uint16(egress), ts, beta)
 	return seg.PeerEntry{
@@ -210,10 +211,10 @@ func (s *DefaultExtender) remoteInfo(ifid common.IFIDType) (
 	}
 	topoInfo := intf.TopoInfo()
 	if topoInfo.RemoteIFID == 0 {
-		return 0, 0, 0, serrors.New("remote ifid is not set")
+		return 0, 0, 0, serrors.New("remote interface ID is not set")
 	}
 	if topoInfo.IA.IsWildcard() {
-		return 0, 0, 0, serrors.New("remote is wildcard", "isd_as", topoInfo.IA)
+		return 0, 0, 0, serrors.New("remote ISD-AS is wildcard", "isd_as", topoInfo.IA)
 	}
 	return topoInfo.IA.IAInt(), topoInfo.RemoteIFID, uint16(topoInfo.MTU), nil
 }

go/cs/beaconing/handler.go

@@ -21,7 +21,6 @@ import (
 
 	"github.com/scionproto/scion/go/cs/beacon"
 	"github.com/scionproto/scion/go/cs/ifstate"
-	csmetrics "github.com/scionproto/scion/go/cs/metrics"
 	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/ctrl/seg"
@@ -59,43 +58,45 @@ func (h Handler) HandleBeacon(ctx context.Context, b beacon.Beacon, peer *snet.U
 
 	intf := h.Interfaces.Get(b.InIfId)
 	if intf == nil {
-		err := serrors.New("received beacon on non-existent interface", "if_id", b.InIfId)
-		h.updateMetric(span, labels.WithResult(csmetrics.ErrNotClassified), err)
+		err := serrors.New("received beacon on non-existent interface",
+			"ingress_interface", b.InIfId)
+		h.updateMetric(span, labels.WithResult(prom.ErrNotClassified), err)
 		return err
 	}
 
 	upstream := intf.TopoInfo().IA
 	if span != nil {
-		span.SetTag("in_if_id", b.InIfId)
+		span.SetTag("ingress_interface", b.InIfId)
 		span.SetTag("upstream", upstream)
 	}
+	labels.Neighbor = upstream
 	logger := log.FromCtx(ctx).New("beacon", b, "upstream", upstream)
 	ctx = log.CtxWith(ctx, logger)
 
 	logger.Debug("Received beacon")
 	if err := h.Inserter.PreFilter(b); err != nil {
 		logger.Debug("Beacon pre-filtered", "err", err)
-		h.updateMetric(span, labels.WithResult(csmetrics.ErrPrefilter), err)
+		h.updateMetric(span, labels.WithResult("err_prefilter"), err)
 		return err
 	}
 	if err := h.validateASEntry(b, intf); err != nil {
 		logger.Info("Beacon validation failed", "err", err)
-		h.updateMetric(span, labels.WithResult(csmetrics.ErrVerify), err)
+		h.updateMetric(span, labels.WithResult(prom.ErrVerify), err)
 		return err
 	}
 	if err := h.verifySegment(ctx, b.Segment, peer); err != nil {
 		logger.Info("Beacon verification failed", "err", err)
-		h.updateMetric(span, labels.WithResult(csmetrics.ErrVerify), err)
+		h.updateMetric(span, labels.WithResult(prom.ErrVerify), err)
 		return serrors.WrapStr("verifying beacon", err)
 	}
 	stat, err := h.Inserter.InsertBeacon(ctx, b)
 	if err != nil {
 		logger.Debug("Failed to insert beacon", "err", err)
-		h.updateMetric(span, labels.WithResult(csmetrics.ErrDB), err)
+		h.updateMetric(span, labels.WithResult(prom.ErrDB), err)
 		return serrors.WrapStr("inserting beacon", err)
 
 	}
-	labels = labels.WithResult(csmetrics.GetResultValue(stat.Inserted, stat.Updated, stat.Filtered))
+	labels = labels.WithResult(resultValue(stat.Inserted, stat.Updated, stat.Filtered))
 	h.updateMetric(span, labels, err)
 	logger.Debug("Inserted beacon")
 	return nil
@@ -105,7 +106,7 @@ func (h Handler) validateASEntry(b beacon.Beacon, intf *ifstate.Interface) error
 	topoInfo := intf.TopoInfo()
 	if topoInfo.LinkType != topology.Parent && topoInfo.LinkType != topology.Core {
 		return serrors.New("beacon received on invalid link",
-			"ifid", b.InIfId, "linkType", topoInfo.LinkType)
+			"ingress_interface", b.InIfId, "link_type", topoInfo.LinkType)
 	}
 	asEntry := b.Segment.ASEntries[b.Segment.MaxIdx()]
 	if !asEntry.Local.Equal(topoInfo.IA) {
@@ -153,7 +154,7 @@ type handlerLabels struct {
 
 func (l handlerLabels) Expand() []string {
 	return []string{
-		"in_if_id", l.Ingress.String(),
+		"ingress_interface", l.Ingress.String(),
 		prom.LabelNeighIA, l.Neighbor.String(),
 		prom.LabelResult, l.Result,
 	}
@@ -163,3 +164,16 @@ func (l handlerLabels) WithResult(result string) handlerLabels {
 	l.Result = result
 	return l
 }
+
+func resultValue(ins, upd, flt int) string {
+	switch {
+	case flt > 0:
+		return "ok_filtered"
+	case upd > 0:
+		return "ok_updated"
+	case ins > 0:
+		return "ok_new"
+	default:
+		return "ok_old"
+	}
+}

go/cs/beaconing/originator.go

@@ -19,16 +19,18 @@ import (
 	"crypto/rand"
 	"math/big"
 	"net"
+	"strconv"
 	"sync"
 	"time"
 
 	"github.com/scionproto/scion/go/cs/ifstate"
-	"github.com/scionproto/scion/go/cs/metrics"
 	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/ctrl/seg"
 	"github.com/scionproto/scion/go/lib/log"
+	"github.com/scionproto/scion/go/lib/metrics"
 	"github.com/scionproto/scion/go/lib/periodic"
+	"github.com/scionproto/scion/go/lib/prom"
 	"github.com/scionproto/scion/go/lib/serrors"
 	"github.com/scionproto/scion/go/lib/topology"
 )
@@ -49,13 +51,15 @@ type Originator struct {
 	Signer       seg.Signer
 	Intfs        *ifstate.Interfaces
 
-	// tick is mutable.
+	Originated metrics.Counter
+
+	// Tick is mutable.
 	Tick Tick
 }
 
 // Name returns the tasks name.
 func (o *Originator) Name() string {
-	return "bs_beaconing_originator"
+	return "control_beaconing_originator"
 }
 
 // Run originates core and downstream beacons.
@@ -74,7 +78,6 @@ func (o *Originator) Run(ctx context.Context) {
 		o.originateBeacons(ctx, topology.Child)
 	}()
 	wg.Wait()
-	metrics.Originator.Runtime().Add(time.Since(o.Tick.now).Seconds())
 	o.Tick.updateLast()
 }
 
@@ -84,7 +87,7 @@ func (o *Originator) originateBeacons(ctx context.Context, linkType topology.Lin
 	logger := log.FromCtx(ctx)
 	active, nonActive := sortedIntfs(o.Intfs, linkType)
 	if len(nonActive) > 0 && o.Tick.passed() {
-		logger.Debug("Ignore non-active interfaces", "ifids", nonActive)
+		logger.Debug("Ignore non-active interfaces", "egress_interfaces", nonActive)
 	}
 	intfs := o.needBeacon(active)
 	if len(intfs) == 0 {
@@ -104,7 +107,8 @@ func (o *Originator) originateBeacons(ctx context.Context, linkType topology.Lin
 			defer log.HandlePanic()
 			defer wg.Done()
 			if err := b.originateBeacon(ctx); err != nil {
-				logger.Info("Unable to originate on interface", "ifid", b.ifID, "err", err)
+				logger.Info("Unable to originate on interface",
+					"egress_interface", b.ifID, "err", err)
 			}
 		}()
 	}
@@ -132,12 +136,13 @@ func (o *Originator) needBeacon(active []common.IFIDType) []common.IFIDType {
 
 func (o *Originator) logSummary(logger log.Logger, s *summary, linkType topology.LinkType) {
 	if o.Tick.passed() {
-		logger.Debug("Originated beacons", "type", linkType.String(), "egIfIds", s.IfIds())
+		logger.Debug("Originated beacons",
+			"type", linkType.String(), "egress_interfaces", s.IfIds())
 		return
 	}
 	if s.count > 0 {
 		logger.Debug("Originated beacons on stale interfaces",
-			"type", linkType.String(), "egIfIds", s.IfIds())
+			"type", linkType.String(), "egress_interfaces", s.IfIds())
 	}
 }
 
@@ -151,17 +156,17 @@ type beaconOriginator struct {
 
 // originateBeacon originates a beacon on the given ifid.
 func (o *beaconOriginator) originateBeacon(ctx context.Context) error {
-	labels := metrics.OriginatorLabels{EgIfID: o.ifID, Result: metrics.Success}
+	labels := originatorLabels{Egress: o.ifID}
 	intf := o.Intfs.Get(o.ifID)
 	if intf == nil {
-		metrics.Originator.Beacons(labels.WithResult(metrics.ErrVerify)).Inc()
-		return serrors.New("Interface does not exist")
+		o.incrementMetrics(labels.WithResult(prom.ErrValidate))
+		return serrors.New("interface does not exist")
 	}
 	topoInfo := intf.TopoInfo()
 	bseg, err := o.createBeacon(ctx)
 	if err != nil {
-		metrics.Originator.Beacons(labels.WithResult(metrics.ErrCreate)).Inc()
-		return common.NewBasicError("Unable to create beacon", err, "ifid", o.ifID)
+		o.incrementMetrics(labels.WithResult("err_create"))
+		return serrors.WrapStr("creating beacon", err, "egress_interface", o.ifID)
 	}
 
 	err = o.BeaconSender.Send(
@@ -172,11 +177,11 @@ func (o *beaconOriginator) originateBeacon(ctx context.Context) error {
 		topoInfo.InternalAddr,
 	)
 	if err != nil {
-		metrics.Originator.Beacons(labels.WithResult(metrics.ErrSend)).Inc()
-		return common.NewBasicError("Unable to send packet", err)
+		o.incrementMetrics(labels.WithResult(prom.ErrNetwork))
+		return serrors.WrapStr("sending beacon", err)
 	}
 	o.onSuccess(intf)
-	metrics.Originator.Beacons(labels).Inc()
+	o.incrementMetrics(labels.WithResult(prom.Success))
 	return nil
 }
 
@@ -201,3 +206,24 @@ func (o *beaconOriginator) onSuccess(intf *ifstate.Interface) {
 	o.summary.AddIfid(o.ifID)
 	o.summary.Inc()
 }
+
+func (o *beaconOriginator) incrementMetrics(labels originatorLabels) {
+	if o.Originator.Originated == nil {
+		return
+	}
+	o.Originator.Originated.With(labels.Expand()...).Add(1)
+}
+
+type originatorLabels struct {
+	Egress common.IFIDType
+	Result string
+}
+
+func (l originatorLabels) Expand() []string {
+	return []string{"egress_interface", strconv.Itoa(int(l.Egress)), prom.LabelResult, l.Result}
+}
+
+func (l originatorLabels) WithResult(result string) originatorLabels {
+	l.Result = result
+	return l
+}