common/reporter: simplify interface for collecting metrics

Remove unused methods and always collect scoped metrics. As a
side-effect, BioRIS gRPC metrics are now correctly scoped.
This commit is contained in:
Vincent Bernat
2025-08-27 07:32:24 +02:00
parent 3affecc309
commit fa11e7de6d
9 changed files with 19 additions and 51 deletions

View File

@@ -111,22 +111,7 @@ func (r *Reporter) MetricsHTTPHandler() http.Handler {
return r.metrics.HTTPHandler() return r.metrics.HTTPHandler()
} }
// MetricCollector register a custom collector. // RegisterMetricCollector register a custom collector prefixed by the current module name.
func (r *Reporter) MetricCollector(c prometheus.Collector) { func (r *Reporter) RegisterMetricCollector(c prometheus.Collector) {
r.metrics.Collector(c) r.metrics.RegisterCollector(1, c)
}
// MetricCollectorForCurrentModule register a custom collector prefixed by the current module name.
func (r *Reporter) MetricCollectorForCurrentModule(c prometheus.Collector) {
r.metrics.CollectorForCurrentModule(1, c)
}
// MetricDesc defines a new metric description.
func (r *Reporter) MetricDesc(name, help string, variableLabels []string) *MetricDesc {
return r.metrics.Desc(1, name, help, variableLabels)
}
// MetricDesc2 defines a new metric description. It skips one callstack.
func (r *Reporter) MetricDesc2(name, help string, variableLabels []string) *MetricDesc {
return r.metrics.Desc(2, name, help, variableLabels)
} }

View File

@@ -100,25 +100,9 @@ func (m *Metrics) Factory(skipCallstack int) *Factory {
return &factory return &factory
} }
// Desc allocates and initializes and new metric description. Like for // RegisterCollector register a custom collector and prefix
// factory, names are prefixed with the module name. Unlike factory,
// there is no cache.
func (m *Metrics) Desc(skipCallstack int, name, help string, variableLabels []string) *prometheus.Desc {
callStack := stack.Callers()
call := callStack[1+skipCallstack] // Trial and error, there is a test to check it works
prefix := getPrefix(call.FunctionName())
name = fmt.Sprintf("%s%s", prefix, name)
return prometheus.NewDesc(name, help, variableLabels, nil)
}
// Collector register a custom collector.
func (m *Metrics) Collector(c prometheus.Collector) {
m.registry.MustRegister(c)
}
// CollectorForCurrentModule register a custom collector and prefix
// everything with the module name. // everything with the module name.
func (m *Metrics) CollectorForCurrentModule(skipCallStack int, c prometheus.Collector) { func (m *Metrics) RegisterCollector(skipCallStack int, c prometheus.Collector) {
callStack := stack.Callers() callStack := stack.Callers()
call := callStack[1+skipCallStack] // Should be the same as above ! call := callStack[1+skipCallStack] // Should be the same as above !
prefix := getPrefix(call.FunctionName()) prefix := getPrefix(call.FunctionName())

View File

@@ -7,6 +7,7 @@ import (
"fmt" "fmt"
"net/http/httptest" "net/http/httptest"
"runtime" "runtime"
"slices"
"strings" "strings"
"testing" "testing"
@@ -51,14 +52,7 @@ func TestNew(t *testing.T) {
expecteds = append(expecteds, "process_open_fds") expecteds = append(expecteds, "process_open_fds")
} }
for _, expected := range expecteds { for _, expected := range expecteds {
found := false if !slices.Contains(got, fmt.Sprintf("# TYPE %s gauge", expected)) {
for _, line := range got {
if line == fmt.Sprintf("# TYPE %s gauge", expected) {
found = true
break
}
}
if !found {
t.Errorf("GET /api/v0/metrics missing: %s", expected) t.Errorf("GET /api/v0/metrics missing: %s", expected)
} }
} }

View File

@@ -180,9 +180,9 @@ func TestMetricCollector(t *testing.T) {
r := reporter.NewMock(t) r := reporter.NewMock(t)
m := customMetrics{} m := customMetrics{}
m.metric1 = r.MetricDesc("metric1", "Custom metric 1", nil) m.metric1 = prometheus.NewDesc("metric1", "Custom metric 1", nil, nil)
m.metric2 = r.MetricDesc("metric2", "Custom metric 2", nil) m.metric2 = prometheus.NewDesc("metric2", "Custom metric 2", nil, nil)
r.MetricCollector(m) r.RegisterMetricCollector(m)
got := r.GetMetrics("akvorado_common_reporter_test_") got := r.GetMetrics("akvorado_common_reporter_test_")
expected := map[string]string{ expected := map[string]string{

View File

@@ -25,6 +25,7 @@ the ownership of the Prometheus volume:
#1900](https://github.com/akvorado/akvorado/pull/1900) for the consequences if #1900](https://github.com/akvorado/akvorado/pull/1900) for the consequences if
you upgrade from a previous beta) you upgrade from a previous beta)
- 💥 *docker*: switch from Prometheus to Grafana Alloy for scraping metrics - 💥 *docker*: switch from Prometheus to Grafana Alloy for scraping metrics
- 🩹 *outlet*: move gRPC metrics for BioRIS provider in the routing namespace
- 🌱 *docker*: enforce bridge name - 🌱 *docker*: enforce bridge name
- 🌱 *docker*: add cAdvisor to the monitoring stack - 🌱 *docker*: add cAdvisor to the monitoring stack
- 🌱 *docker*: update Prometheus to 3.5.0 - 🌱 *docker*: update Prometheus to 3.5.0

View File

@@ -87,7 +87,7 @@ func (c *Component) Start() error {
Msg("unable to create Kafka client") Msg("unable to create Kafka client")
return fmt.Errorf("unable to create Kafka client: %w", err) return fmt.Errorf("unable to create Kafka client: %w", err)
} }
c.r.MetricCollectorForCurrentModule(kafkaMetrics) c.r.RegisterMetricCollector(kafkaMetrics)
c.kafkaClient = kafkaClient c.kafkaClient = kafkaClient
// When dying, close the client // When dying, close the client

View File

@@ -35,7 +35,7 @@ func (c *realComponent) newClient(i int) (*kgo.Client, error) {
Msg("unable to create new client") Msg("unable to create new client")
return nil, fmt.Errorf("unable to create Kafka client: %w", err) return nil, fmt.Errorf("unable to create Kafka client: %w", err)
} }
c.r.MetricCollectorForCurrentModule(kafkaMetrics) c.r.RegisterMetricCollector(kafkaMetrics)
return client, nil return client, nil
} }

View File

@@ -18,7 +18,7 @@ type metrics struct {
// initMetrics initialize the metrics for the BMP component. // initMetrics initialize the metrics for the BMP component.
func (p *Provider) initMetrics() { func (p *Provider) initMetrics() {
p.r.MetricCollector(p.clientMetrics) p.r.RegisterMetricCollector(p.clientMetrics)
p.metrics.risUp = p.r.GaugeVec( p.metrics.risUp = p.r.GaugeVec(
reporter.GaugeOpts{ reporter.GaugeOpts{
Name: "connection_up", Name: "connection_up",

View File

@@ -462,7 +462,7 @@ func TestBioRIS(t *testing.T) {
} }
for try := 2; try >= 0; try-- { for try := 2; try >= 0; try-- {
gotMetrics := r.GetMetrics("akvorado_outlet_routing_provider_bioris_") gotMetrics := r.GetMetrics("akvorado_outlet_routing_provider_bioris_", "-grpc_client")
expectedMetrics := map[string]string{ expectedMetrics := map[string]string{
// connection_up may take a bit of time // connection_up may take a bit of time
fmt.Sprintf(`connection_up{ris="%s"}`, addr): "1", fmt.Sprintf(`connection_up{ris="%s"}`, addr): "1",
@@ -506,6 +506,10 @@ func TestNonWorkingBioRIS(t *testing.T) {
expectedMetrics := map[string]string{ expectedMetrics := map[string]string{
`connection_up{ris="ris.invalid:1000"}`: "0", `connection_up{ris="ris.invalid:1000"}`: "0",
`connection_up{ris="192.0.2.10:1000"}`: "0", `connection_up{ris="192.0.2.10:1000"}`: "0",
`grpc_client_handled_total{grpc_code="Unavailable",grpc_method="GetRouters",grpc_service="bio.ris.RoutingInformationService",grpc_type="unary"}`: "2",
`grpc_client_msg_received_total{grpc_method="GetRouters",grpc_service="bio.ris.RoutingInformationService",grpc_type="unary"}`: "2",
`grpc_client_msg_sent_total{grpc_method="GetRouters",grpc_service="bio.ris.RoutingInformationService",grpc_type="unary"}`: "2",
`grpc_client_started_total{grpc_method="GetRouters",grpc_service="bio.ris.RoutingInformationService",grpc_type="unary"}`: "2",
} }
if diff := helpers.Diff(gotMetrics, expectedMetrics); diff != "" { if diff := helpers.Diff(gotMetrics, expectedMetrics); diff != "" {
t.Errorf("Metrics (-got, +want):\n%s", diff) t.Errorf("Metrics (-got, +want):\n%s", diff)