Repository: scionproto/scion
Pull request #3611 (2020-01-15 13:27:01)

Fix metric names
This reverts the metric changes accidentally introduced by
https://github.com/scionproto/scion/pull/3590.

Fixes https://github.com/scionproto/scion/issues/3610.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/scionproto/scion/3611)
<!-- Reviewable:end -->


scrye: 



___
*[go/cs/main.go, line 112 at r2](https://reviewable.io/reviews/scionproto/scion/3611#-Lyd_cSeEEe-fCwBR3jf:-LydcLCU5WiQtEdZnewU:bnpc8o0) ([raw file](https://github.com/scionproto/scion/blob/f0ec0a10f700e10c2aba675d3f92b8b40ff9b9c3/go/cs/main.go#L112)):*
<details><summary><i>Previously, lukedirtwalker (Lukas Vogel) wrote…</i></summary><blockquote>

Maybe it would make sense to have a `InitMetrics` function
</blockquote></details>

I think it's a bit more transparent this way; I expect the code around application initialization to get simplified in the near future, so we can revisit this soon.


<!-- Sent from Reviewable.io -->


Patch:
diff --git a/go/beacon_srv/BUILD.bazel b/go/beacon_srv/BUILD.bazel
index 26c77cca4..766ee78a6 100644
--- a/go/beacon_srv/BUILD.bazel
+++ b/go/beacon_srv/BUILD.bazel
@@ -14,6 +14,7 @@ go_library(
         "//go/cs/config:go_default_library",
         "//go/cs/ifstate:go_default_library",
         "//go/cs/keepalive:go_default_library",
+        "//go/cs/metrics:go_default_library",
         "//go/cs/onehop:go_default_library",
         "//go/cs/revocation:go_default_library",
         "//go/lib/addr:go_default_library",
diff --git a/go/beacon_srv/main.go b/go/beacon_srv/main.go
index e7f0c5e01..68cfc1cba 100644
--- a/go/beacon_srv/main.go
+++ b/go/beacon_srv/main.go
@@ -40,6 +40,7 @@ import (
 	controlconfig "github.com/scionproto/scion/go/cs/config"
 	"github.com/scionproto/scion/go/cs/ifstate"
 	"github.com/scionproto/scion/go/cs/keepalive"
+	"github.com/scionproto/scion/go/cs/metrics"
 	"github.com/scionproto/scion/go/cs/onehop"
 	"github.com/scionproto/scion/go/cs/revocation"
 	"github.com/scionproto/scion/go/lib/addr"
@@ -103,6 +104,7 @@ func realMain() int {
 		log.Crit("Setup failed", "err", err)
 		return 1
 	}
+	metrics.InitBSMetrics()
 
 	topo := itopo.Get()
 	if !topo.Exists(addr.SvcBS, cfg.General.ID) {
diff --git a/go/cs/BUILD.bazel b/go/cs/BUILD.bazel
index 29b0b518d..18f221c6f 100644
--- a/go/cs/BUILD.bazel
+++ b/go/cs/BUILD.bazel
@@ -13,6 +13,7 @@ go_library(
         "//go/cs/handlers:go_default_library",
         "//go/cs/ifstate:go_default_library",
         "//go/cs/keepalive:go_default_library",
+        "//go/cs/metrics:go_default_library",
         "//go/cs/onehop:go_default_library",
         "//go/cs/revocation:go_default_library",
         "//go/cs/segreq:go_default_library",
diff --git a/go/cs/beaconing/BUILD.bazel b/go/cs/beaconing/BUILD.bazel
index 3e2e9022e..93a76a77b 100644
--- a/go/cs/beaconing/BUILD.bazel
+++ b/go/cs/beaconing/BUILD.bazel
@@ -55,6 +55,7 @@ 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/cs/onehop:go_default_library",
         "//go/lib/addr:go_default_library",
         "//go/lib/common:go_default_library",
diff --git a/go/cs/ifstate/BUILD.bazel b/go/cs/ifstate/BUILD.bazel
index 3a0a334b2..948270af2 100644
--- a/go/cs/ifstate/BUILD.bazel
+++ b/go/cs/ifstate/BUILD.bazel
@@ -43,6 +43,7 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//go/cs/ifstate/mock_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:go_default_library",
diff --git a/go/cs/keepalive/BUILD.bazel b/go/cs/keepalive/BUILD.bazel
index 50d8c8ac6..d76211550 100644
--- a/go/cs/keepalive/BUILD.bazel
+++ b/go/cs/keepalive/BUILD.bazel
@@ -36,6 +36,7 @@ go_test(
     deps = [
         "//go/cs/ifstate:go_default_library",
         "//go/cs/keepalive/mock_keepalive:go_default_library",
+        "//go/cs/metrics:go_default_library",
         "//go/cs/onehop:go_default_library",
         "//go/lib/common:go_default_library",
         "//go/lib/ctrl:go_default_library",
diff --git a/go/cs/main.go b/go/cs/main.go
index 2406c392b..78030e374 100644
--- a/go/cs/main.go
+++ b/go/cs/main.go
@@ -40,6 +40,7 @@ import (
 	"github.com/scionproto/scion/go/cs/handlers"
 	"github.com/scionproto/scion/go/cs/ifstate"
 	"github.com/scionproto/scion/go/cs/keepalive"
+	"github.com/scionproto/scion/go/cs/metrics"
 	"github.com/scionproto/scion/go/cs/onehop"
 	"github.com/scionproto/scion/go/cs/revocation"
 	"github.com/scionproto/scion/go/cs/segreq"
@@ -108,6 +109,8 @@ func realMain() int {
 		log.Crit("Setup failed", "err", err)
 		return 1
 	}
+	metrics.InitBSMetrics()
+	metrics.InitPSMetrics()
 	pathDB, revCache, err := pathstorage.NewPathStorage(cfg.PS.PathDB, cfg.PS.RevCache)
 	if err != nil {
 		log.Crit("Unable to initialize path storage", "err", err)
diff --git a/go/cs/metrics/beaconing.go b/go/cs/metrics/beaconing.go
index e49030606..6e2f496b4 100644
--- a/go/cs/metrics/beaconing.go
+++ b/go/cs/metrics/beaconing.go
@@ -51,7 +51,7 @@ type beaconing struct {
 }
 
 func newBeaconing() beaconing {
-	ns, sub := Namespace, "beaconing"
+	ns, sub := BSNamespace, "beaconing"
 	return beaconing{
 		receivedBeacons: prom.NewCounterVecWithLabels(ns, sub, "received_beacons_total",
 			"Total number of received beacons.", BeaconingLabels{}),
diff --git a/go/cs/metrics/ifstate.go b/go/cs/metrics/ifstate.go
index 4ae6b7e8f..967833276 100644
--- a/go/cs/metrics/ifstate.go
+++ b/go/cs/metrics/ifstate.go
@@ -30,18 +30,18 @@ func newIfstate() ifstate {
 	sub := "ifstate"
 	return ifstate{
 		Desc: prometheus.NewDesc(
-			prometheus.BuildFQName(Namespace, sub, "state"),
+			prometheus.BuildFQName(BSNamespace, sub, "state"),
 			"Interface state, 0==inactive/expired/revoked, 1==active",
 			IfstateLabels{}.Labels(),
 			prometheus.Labels{},
 		),
-		issued: *prom.NewCounterVecWithLabels(Namespace, sub,
+		issued: *prom.NewCounterVecWithLabels(BSNamespace, sub,
 			"issued_revocations_total",
 			"Total number of issued revocations.", IssuedLabels{}),
-		duration: *prom.NewCounterVecWithLabels(Namespace, sub,
+		duration: *prom.NewCounterVecWithLabels(BSNamespace, sub,
 			"revocations_duration_seconds_total",
 			"Duration in seconds of issued revocations.", DurationLabels{}),
-		out: *prom.NewCounterVecWithLabels(Namespace, sub,
+		out: *prom.NewCounterVecWithLabels(BSNamespace, sub,
 			"sent_revocations_total",
 			"Total number of sent revocations.", SentLabels{}),
 	}
diff --git a/go/cs/metrics/keepalive.go b/go/cs/metrics/keepalive.go
index cd5cac930..966ff605c 100644
--- a/go/cs/metrics/keepalive.go
+++ b/go/cs/metrics/keepalive.go
@@ -47,9 +47,9 @@ func newKeepalive() exporter {
 	labels := KeepaliveLabels{}
 
 	return exporter{
-		out: prom.NewCounterVecWithLabels(Namespace, sub, "sent_msgs_total",
+		out: prom.NewCounterVecWithLabels(BSNamespace, sub, "sent_msgs_total",
 			"Total number of sent keepalive msgs.", labels),
-		in: prom.NewCounterVecWithLabels(Namespace, sub, "received_msgs_total",
+		in: prom.NewCounterVecWithLabels(BSNamespace, sub, "received_msgs_total",
 			"Total number of received keepalive msgs.", labels),
 	}
 
diff --git a/go/cs/metrics/metrics.go b/go/cs/metrics/metrics.go
index d25902d18..1cf0fd28f 100644
--- a/go/cs/metrics/metrics.go
+++ b/go/cs/metrics/metrics.go
@@ -18,8 +18,12 @@ import (
 	"github.com/scionproto/scion/go/lib/prom"
 )
 
-// Namespace is the metrics namespace for the control service.
-const Namespace = "cs"
+// Metrics namespaces for control service subcomponents.
+const (
+	CSNamespace = "cs"
+	BSNamespace = "bs"
+	PSNamespace = "ps"
+)
 
 // Labels values.
 const (
@@ -90,29 +94,48 @@ const (
 
 var (
 	// Beaconing is the single-instance struct to get prometheus metrics or counters.
-	Beaconing = newBeaconing()
+	Beaconing beaconing
 	// Ifstate is the single-instance struct to get prometheus metrics or counters.
-	Ifstate = newIfstate()
+	Ifstate ifstate
 	// Keepalive is the single-instance struct to get keepalive prometheus counters.
-	Keepalive = newKeepalive()
+	Keepalive exporter
 	// Originator is the single-instance struct to get prometheus counters.
-	Originator = newOriginator()
+	Originator originator
 	// Propagator is the single-instance struct to get prometheus metrics or counters.
-	Propagator = newPropagator()
+	Propagator propagator
 	// Revocation is the single-instance struct to get prometheus counters.
-	Revocation = newRevocation()
+	Revocation exporterR
 	// Registrar is the single-instance struct to get prometheus metrics or counters.
-	Registrar = newRegistrar()
+	Registrar registrar
 	// Registrations contains metrics for segments registrations.
-	Registrations = newRegistration()
+	Registrations Registration
 	// Requests contains metrics for segments requests.
-	Requests = newRequests()
+	Requests Request
 	// Sync contains metrics for segment synchronization.
-	Sync = newSync()
+	Sync sync
 	// PSRevocation contains metrics for revocations.
-	PSRevocation = psNewRevocation()
+	PSRevocation revocation
 )
 
+// InitBSMetrics initializes the metrics used by BS modules.
+func InitBSMetrics() {
+	Beaconing = newBeaconing()
+	Ifstate = newIfstate()
+	Keepalive = newKeepalive()
+	Originator = newOriginator()
+	Propagator = newPropagator()
+	Revocation = newRevocation()
+	Registrar = newRegistrar()
+}
+
+// InitPSMetrics initializes the metrics used by PS modules.
+func InitPSMetrics() {
+	Registrations = newRegistration()
+	Requests = newRequests()
+	Sync = newSync()
+	PSRevocation = psNewRevocation()
+}
+
 // Revocation sources
 const (
 	RevSrcNotification = "notification"
diff --git a/go/cs/metrics/originator.go b/go/cs/metrics/originator.go
index d2d25f5e5..92c068a42 100644
--- a/go/cs/metrics/originator.go
+++ b/go/cs/metrics/originator.go
@@ -49,7 +49,7 @@ type originator struct {
 }
 
 func newOriginator() originator {
-	ns, sub := Namespace, "beaconing"
+	ns, sub := BSNamespace, "beaconing"
 	return originator{
 		originatedBeacons: prom.NewCounterVecWithLabels(ns, sub, "originated_beacons_total",
 			"Number of beacons originated", OriginatorLabels{}),
diff --git a/go/cs/metrics/propagator.go b/go/cs/metrics/propagator.go
index 34e0845de..5ee64e82e 100644
--- a/go/cs/metrics/propagator.go
+++ b/go/cs/metrics/propagator.go
@@ -45,7 +45,7 @@ type propagator struct {
 }
 
 func newPropagator() propagator {
-	ns, sub := Namespace, "beaconing"
+	ns, sub := BSNamespace, "beaconing"
 	return propagator{
 		propagatedBeacons: prom.NewCounterVecWithLabels(ns, sub,
 			"propagated_beacons_total",
diff --git a/go/cs/metrics/registrar.go b/go/cs/metrics/registrar.go
index 37c2b745a..e59081afd 100644
--- a/go/cs/metrics/registrar.go
+++ b/go/cs/metrics/registrar.go
@@ -60,7 +60,7 @@ type registrar struct {
 }
 
 func newRegistrar() registrar {
-	ns, sub := Namespace, "beaconing"
+	ns, sub := BSNamespace, "beaconing"
 	return registrar{
 		registeredBeacons: prom.NewCounterVecWithLabels(ns, sub, "registered_beacons_total",
 			"Number of beacons registered",
diff --git a/go/cs/metrics/registration.go b/go/cs/metrics/registration.go
index 2ffbf3b5f..50f60ad92 100644
--- a/go/cs/metrics/registration.go
+++ b/go/cs/metrics/registration.go
@@ -55,7 +55,7 @@ type Registration struct {
 
 func newRegistration() Registration {
 	return Registration{
-		regsTotal: prom.NewCounterVecWithLabels(Namespace, "", "registrations_total",
+		regsTotal: prom.NewCounterVecWithLabels(PSNamespace, "", "registrations_total",
 			fmt.Sprintf("Number of path registrations. \"result\" can be one of: [%s]",
 				strings.Join(regResults, ",")),
 			RegistrationLabels{}),
diff --git a/go/cs/metrics/requests.go b/go/cs/metrics/requests.go
index 8e0e4d9d8..017dc167e 100644
--- a/go/cs/metrics/requests.go
+++ b/go/cs/metrics/requests.go
@@ -74,13 +74,18 @@ type Request struct {
 func newRequests() Request {
 	subsystem := "requests"
 	return Request{
-		count: prom.NewCounterVecWithLabels(Namespace, subsystem, "total",
+		count: prom.NewCounterVecWithLabels(PSNamespace, subsystem, "total",
 			"Number of segment requests total. \"result\" indicates the outcome.",
 			RequestLabels{}),
-		repliedSegs: prom.NewCounterVecWithLabels(Namespace, subsystem, "replied_segments_total",
+		repliedSegs: prom.NewCounterVecWithLabels(PSNamespace, subsystem, "replied_segments_total",
 			"Number of segments in reply to segment requests.", RequestOkLabels{}),
-		repliedRevs: prom.NewCounterVecWithLabels(Namespace, subsystem, "replied_revocations_total",
-			"Number of revocations in reply to segments requests.", RequestOkLabels{}),
+		repliedRevs: prom.NewCounterVecWithLabels(
+			PSNamespace,
+			subsystem,
+			"replied_revocations_total",
+			"Number of revocations in reply to segments requests.",
+			RequestOkLabels{},
+		),
 	}
 }
 
diff --git a/go/cs/metrics/revocation.go b/go/cs/metrics/revocation.go
index 34d84a758..917b5be5a 100644
--- a/go/cs/metrics/revocation.go
+++ b/go/cs/metrics/revocation.go
@@ -39,7 +39,7 @@ type exporterR struct {
 }
 
 func newRevocation() exporterR {
-	ns, sub := Namespace, "revocation"
+	ns, sub := BSNamespace, "revocation"
 	l := RevocationLabels{Result: RevNew, Method: RevFromCtrl}
 	return exporterR{
 		received: prom.NewCounterVecWithLabels(ns, sub, "received_revocations_total",
@@ -80,7 +80,7 @@ type revocation struct {
 
 func psNewRevocation() revocation {
 	return revocation{
-		count: prom.NewCounterVecWithLabels(Namespace, "", "received_revocations_total",
+		count: prom.NewCounterVecWithLabels(PSNamespace, "", "received_revocations_total",
 			"The amount of revocations received.",
 			PSRevocationLabels{Result: OkSuccess, Src: RevSrcNotification}),
 	}
diff --git a/go/cs/metrics/sync.go b/go/cs/metrics/sync.go
index 5a770d0de..63da64058 100644
--- a/go/cs/metrics/sync.go
+++ b/go/cs/metrics/sync.go
@@ -73,10 +73,10 @@ type sync struct {
 func newSync() sync {
 	subsystem := "segment_sync"
 	return sync{
-		registrations: prom.NewCounterVecWithLabels(Namespace, subsystem, "registrations_total",
+		registrations: prom.NewCounterVecWithLabels(PSNamespace, subsystem, "registrations_total",
 			"Number of segments registered in down segment synchronizations",
 			SyncRegLabels{}),
-		pushes: prom.NewCounterVecWithLabels(Namespace, subsystem, "pushes_total",
+		pushes: prom.NewCounterVecWithLabels(PSNamespace, subsystem, "pushes_total",
 			"Number of pushes towards a destination", SyncPushLabels{}),
 	}
 }
diff --git a/go/cs/revocation/BUILD.bazel b/go/cs/revocation/BUILD.bazel
index 0ad0e0c90..9388f9db0 100644
--- a/go/cs/revocation/BUILD.bazel
+++ b/go/cs/revocation/BUILD.bazel
@@ -25,6 +25,7 @@ go_test(
     srcs = ["handler_test.go"],
     embed = [":go_default_library"],
     deps = [
+        "//go/cs/metrics:go_default_library",
         "//go/cs/revocation/mock_revocation:go_default_library",
         "//go/lib/common:go_default_library",
         "//go/lib/ctrl:go_default_library",
diff --git a/go/cs/segreq/handler.go b/go/cs/segreq/handler.go
index 573ca5e81..ed905c593 100644
--- a/go/cs/segreq/handler.go
+++ b/go/cs/segreq/handler.go
@@ -50,7 +50,7 @@ func NewHandler(args handlers.HandlerArgs) infra.Handler {
 			RequestAPI:          args.SegRequestAPI,
 			DstProvider:         createDstProvider(args, core),
 			Splitter:            &Splitter{ASInspector: args.ASInspector},
-			MetricsNamespace:    metrics.Namespace,
+			MetricsNamespace:    metrics.PSNamespace,
 			LocalInfo:           localInfo,
 		}.New(),
 		revCache: args.RevCache,
diff --git a/go/path_srv/BUILD.bazel b/go/path_srv/BUILD.bazel
index 0ea0b472f..5fd773d23 100644
--- a/go/path_srv/BUILD.bazel
+++ b/go/path_srv/BUILD.bazel
@@ -8,6 +8,7 @@ go_library(
     visibility = ["//visibility:private"],
     deps = [
         "//go/cs/handlers:go_default_library",
+        "//go/cs/metrics:go_default_library",
         "//go/cs/segreq:go_default_library",
         "//go/cs/segsyncer:go_default_library",
         "//go/lib/addr:go_default_library",
diff --git a/go/path_srv/main.go b/go/path_srv/main.go
index 3f9e11363..c5555fefc 100644
--- a/go/path_srv/main.go
+++ b/go/path_srv/main.go
@@ -30,6 +30,7 @@ import (
 	"github.com/opentracing/opentracing-go"
 
 	"github.com/scionproto/scion/go/cs/handlers"
+	"github.com/scionproto/scion/go/cs/metrics"
 	"github.com/scionproto/scion/go/cs/segreq"
 	"github.com/scionproto/scion/go/cs/segsyncer"
 	"github.com/scionproto/scion/go/lib/addr"
@@ -87,6 +88,7 @@ func realMain() int {
 		log.Crit("Setup failed", "err", err)
 		return 1
 	}
+	metrics.InitPSMetrics()
 	pathDB, revCache, err := pathstorage.NewPathStorage(cfg.PS.PathDB, cfg.PS.RevCache)
 	if err != nil {
 		log.Crit("Unable to initialize path storage", "err", err)


Test patch:
diff --git a/go/cs/beaconing/handler_test.go b/go/cs/beaconing/handler_test.go
index c046c9682..6544a805b 100644
--- a/go/cs/beaconing/handler_test.go
+++ b/go/cs/beaconing/handler_test.go
@@ -26,6 +26,7 @@ import (
 	"github.com/scionproto/scion/go/cs/beacon"
 	"github.com/scionproto/scion/go/cs/beaconing/mock_beaconing"
 	"github.com/scionproto/scion/go/cs/ifstate"
+	"github.com/scionproto/scion/go/cs/metrics"
 	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/ctrl"
 	"github.com/scionproto/scion/go/lib/ctrl/seg"
@@ -48,6 +49,7 @@ var (
 
 // Disable logging in all tests
 func TestMain(m *testing.M) {
+	metrics.InitBSMetrics()
 	log.Root().SetHandler(log.DiscardHandler())
 	os.Exit(m.Run())
 }
diff --git a/go/cs/ifstate/revoker_test.go b/go/cs/ifstate/revoker_test.go
index 3b9029e68..9e9da060c 100644
--- a/go/cs/ifstate/revoker_test.go
+++ b/go/cs/ifstate/revoker_test.go
@@ -27,6 +27,7 @@ import (
 	. "github.com/smartystreets/goconvey/convey"
 
 	"github.com/scionproto/scion/go/cs/ifstate/mock_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"
@@ -59,6 +60,7 @@ type brMsg struct {
 }
 
 func TestMain(m *testing.M) {
+	metrics.InitBSMetrics()
 	log.Root().SetHandler(log.DiscardHandler())
 	os.Exit(m.Run())
 }
diff --git a/go/cs/keepalive/handler_test.go b/go/cs/keepalive/handler_test.go
index 3810fe17b..45e4b0150 100644
--- a/go/cs/keepalive/handler_test.go
+++ b/go/cs/keepalive/handler_test.go
@@ -28,6 +28,7 @@ import (
 
 	"github.com/scionproto/scion/go/cs/ifstate"
 	"github.com/scionproto/scion/go/cs/keepalive/mock_keepalive"
+	"github.com/scionproto/scion/go/cs/metrics"
 	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/ctrl"
 	"github.com/scionproto/scion/go/lib/ctrl/ifid"
@@ -48,6 +49,7 @@ var (
 
 // Disable logging in all tests
 func TestMain(m *testing.M) {
+	metrics.InitBSMetrics()
 	log.Root().SetHandler(log.DiscardHandler())
 	os.Exit(m.Run())
 }
diff --git a/go/cs/revocation/handler_test.go b/go/cs/revocation/handler_test.go
index 87a99762c..0820ecd3e 100644
--- a/go/cs/revocation/handler_test.go
+++ b/go/cs/revocation/handler_test.go
@@ -22,6 +22,7 @@ import (
 
 	"github.com/golang/mock/gomock"
 
+	"github.com/scionproto/scion/go/cs/metrics"
 	"github.com/scionproto/scion/go/cs/revocation/mock_revocation"
 	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/ctrl"
@@ -44,6 +45,7 @@ var (
 )
 
 func TestMain(m *testing.M) {
+	metrics.InitBSMetrics()
 	log.Root().SetHandler(log.DiscardHandler())
 	os.Exit(m.Run())
 }