diff --git a/app/controlplane/pkg/metrics/prometheus/manager.go b/app/controlplane/pkg/metrics/prometheus/manager.go index ada09105b..bb576f23b 100644 --- a/app/controlplane/pkg/metrics/prometheus/manager.go +++ b/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) } diff --git a/app/controlplane/pkg/metrics/prometheus/manager_test.go b/app/controlplane/pkg/metrics/prometheus/manager_test.go new file mode 100644 index 000000000..cbc18d9ea --- /dev/null +++ b/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)) +} diff --git a/app/controlplane/pkg/metrics/prometheus/registry/registry.go b/app/controlplane/pkg/metrics/prometheus/registry/registry.go index 888d33741..1707c6d25 100644 --- a/app/controlplane/pkg/metrics/prometheus/registry/registry.go +++ b/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) diff --git a/app/controlplane/pkg/metrics/prometheus/registry/registry_test.go b/app/controlplane/pkg/metrics/prometheus/registry/registry_test.go new file mode 100644 index 000000000..77a0c26e2 --- /dev/null +++ b/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)) +}