chore(prometheus): update histogram variable reference (#1167)

Signed-off-by: Miguel <[email protected]>

Author: migmartri

app/controlplane/pkg/metrics/prometheus/manager.go

@@ -20,26 +20,26 @@ import (
 )
 
 type ChainloopRegistryManager struct {
-	Registries map[string]*registry.PrometheusRegistry
+	registries map[string]*registry.PrometheusRegistry
 }
 
 func NewChainloopRegistryManager() *ChainloopRegistryManager {
 	return &ChainloopRegistryManager{
-		Registries: make(map[string]*registry.PrometheusRegistry),
+		registries: make(map[string]*registry.PrometheusRegistry),
 	}
 }
 
 // AddRegistry adds a registry to the manager
 func (rm *ChainloopRegistryManager) AddRegistry(reg *registry.PrometheusRegistry) {
-	rm.Registries[reg.Name] = reg
+	rm.registries[reg.Name] = reg
 }
 
 // GetRegistryByName returns a registry by name
 func (rm *ChainloopRegistryManager) GetRegistryByName(name string) *registry.PrometheusRegistry {
-	return rm.Registries[name]
+	return rm.registries[name]
 }
 
 // DeleteRegistryByName deletes a registry by name
 func (rm *ChainloopRegistryManager) DeleteRegistryByName(name string) {
-	delete(rm.Registries, name)
+	delete(rm.registries, name)
 }

app/controlplane/pkg/metrics/prometheus/manager_test.go

@@ -0,0 +1,56 @@
+//
+// Copyright 2024 The Chainloop Authors.
+//
+// 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 prometheus
+
+import (
+	"testing"
+
+	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/metrics/prometheus/registry"
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
+)
+
+func (s *managerTestSuite) TestAddAndRetrieveRegistries() {
+	r1 := registry.NewPrometheusRegistry("test1", nil, nil)
+	r2 := registry.NewPrometheusRegistry("test2", nil, nil)
+	s.manager.AddRegistry(r1)
+	s.manager.AddRegistry(r2)
+	s.Len(s.manager.registries, 2)
+
+	s.Equal(r1, s.manager.GetRegistryByName("test1"))
+	s.Equal(r2, s.manager.GetRegistryByName("test2"))
+	s.Nil(s.manager.GetRegistryByName("test-not-found"))
+
+	// delete one
+	s.manager.DeleteRegistryByName("test1")
+	s.Len(s.manager.registries, 1)
+	s.Nil(s.manager.GetRegistryByName("test1"))
+}
+
+type managerTestSuite struct {
+	suite.Suite
+	manager *ChainloopRegistryManager
+}
+
+func (s *managerTestSuite) SetupTest() {
+	s.manager = NewChainloopRegistryManager()
+	require.NotNil(s.T(), s.manager)
+	require.NotNil(s.T(), s.manager.registries)
+}
+
+func TestManager(t *testing.T) {
+	suite.Run(t, new(managerTestSuite))
+}

app/controlplane/pkg/metrics/prometheus/registry/registry.go

@@ -33,13 +33,6 @@ type PrometheusRegistry struct {
 	WorkflowRunDurationSeconds *prometheus.HistogramVec
 }
 
-var workflowRunDurationSeconds = prometheus.NewHistogramVec(prometheus.HistogramOpts{
-	Name: "chainloop_workflow_run_duration_seconds",
-	Help: "Duration of a workflow runs in seconds.",
-	// 10 seconds to 20 minutes
-	Buckets: []float64{10, 30, 60, 90, 120, 180, 240, 300, 600, 900, 1200},
-}, []string{"org", "workflow", "status", "runner"})
-
 // NewPrometheusRegistry creates a new Prometheus registry with a given ID and collector
 func NewPrometheusRegistry(name string, gatherer collector.ChainloopMetricsGatherer, logger log.Logger) *PrometheusRegistry {
 	reg := prometheus.NewRegistry()
@@ -49,6 +42,14 @@ func NewPrometheusRegistry(name string, gatherer collector.ChainloopMetricsGathe
 
 	reg.MustRegister(bcc)
 
+	// It's important to register this variable in the factory since it's a different one per registry
+	workflowRunDurationSeconds := prometheus.NewHistogramVec(prometheus.HistogramOpts{
+		Name: "chainloop_workflow_run_duration_seconds",
+		Help: "Duration of a workflow runs in seconds.",
+		// 10 seconds to 20 minutes
+		Buckets: []float64{10, 30, 60, 90, 120, 180, 240, 300, 600, 900, 1200},
+	}, []string{"org", "workflow", "status", "runner"})
+
 	// Custom metrics that come from the business logic
 	reg.MustRegister(workflowRunDurationSeconds)
 

app/controlplane/pkg/metrics/prometheus/registry/registry_test.go

@@ -0,0 +1,57 @@
+//
+// Copyright 2024 The Chainloop Authors.
+//
+// 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 registry
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+)
+
+// This test looks obvious but checks an important property which is that the
+// metrics must be different between two isolated registries, not a shared memory space.
+func (s *registryTestSuite) TestIsolatedRegistries() {
+	s.NotEqual(s.registry1, s.registry2)
+	s.NotEqual(s.registry1.WorkflowRunDurationSeconds, s.registry2.WorkflowRunDurationSeconds)
+}
+
+func (s *registryTestSuite) TestName() {
+	testCases := []struct {
+		registry *PrometheusRegistry
+		expected string
+	}{
+		{s.registry1, "test1"},
+		{s.registry2, "test2"},
+	}
+
+	for _, tc := range testCases {
+		s.Equal(tc.expected, tc.registry.Name)
+	}
+}
+
+type registryTestSuite struct {
+	suite.Suite
+	registry1, registry2 *PrometheusRegistry
+}
+
+func (s *registryTestSuite) SetupTest() {
+	s.registry1 = NewPrometheusRegistry("test1", nil, nil)
+	s.registry2 = NewPrometheusRegistry("test2", nil, nil)
+}
+
+func TestRegistry(t *testing.T) {
+	suite.Run(t, new(registryTestSuite))
+}