diff --git a/internal/component/loki/process/process_test.go b/internal/component/loki/process/process_test.go index c0cecd928f7a..5256bbcbca0f 100644 --- a/internal/component/loki/process/process_test.go +++ b/internal/component/loki/process/process_test.go @@ -515,6 +515,7 @@ func TestMetricsStageRefresh(t *testing.T) { loki_process_custom_paulin_test{filename="/var/log/pods/agent/agent/1.log",foo="bar"} %d ` + // The component will be reconfigured so that it has a metric. t.Run("config with a metric", func(t *testing.T) { tester.updateAndTest(numLogsToSend, cfgWithMetric, "", @@ -522,7 +523,7 @@ func TestMetricsStageRefresh(t *testing.T) { }) // The component will be "updated" with the same config. - // We expect the metric to stay the same, because the component should be smart enough to + // We expect the metric to stay the same before logs are sent - the component should be smart enough to // know that the new config is the same as the old one and it should just keep running as it is. // If it resets the metric, this could cause issues with some users who have a sidecar "autoreloader" // which reloads the collector config every X seconds. @@ -536,8 +537,9 @@ func TestMetricsStageRefresh(t *testing.T) { // Use a config which has no metrics stage. // This should cause the metric to disappear. cfgWithNoStages := forwardArgs - - tester.updateAndTest(numLogsToSend, cfgWithNoStages, "", "") + t.Run("config with no metrics stage", func(t *testing.T) { + tester.updateAndTest(numLogsToSend, cfgWithNoStages, "", "") + }) // Use a config which has a metric with a different name, // as well as a metric with the same name as the one in the previous config. @@ -566,9 +568,11 @@ func TestMetricsStageRefresh(t *testing.T) { loki_process_custom_paulin_test{filename="/var/log/pods/agent/agent/1.log",foo="bar"} %d ` - tester.updateAndTest(numLogsToSend, cfgWithTwoMetrics, - "", - fmt.Sprintf(expectedMetrics3, numLogsToSend, numLogsToSend)) + t.Run("config with a new and old metric", func(t *testing.T) { + tester.updateAndTest(numLogsToSend, cfgWithTwoMetrics, + "", + fmt.Sprintf(expectedMetrics3, numLogsToSend, numLogsToSend)) + }) } type tester struct { @@ -645,7 +649,7 @@ func (t *tester) updateAndTest(numLogsToSend int, cfg, expectedMetricsBeforeSend // Check the component metrics. if err := testutil.GatherAndCompare(t.registry, strings.NewReader(expectedMetricsBeforeSendingLogs)); err != nil { - t.t.Fatalf("mismatch metrics: %v", err) + require.NoError(t.t, err) } // Send logs. @@ -668,6 +672,6 @@ func (t *tester) updateAndTest(numLogsToSend int, cfg, expectedMetricsBeforeSend // Check the component metrics. if err := testutil.GatherAndCompare(t.registry, strings.NewReader(expectedMetricsAfterSendingLogs)); err != nil { - t.t.Fatalf("mismatch metrics: %v", err) + require.NoError(t.t, err) } } diff --git a/internal/util/unregisterer.go b/internal/util/unregisterer.go index 6c119563b515..eedcabc2441a 100644 --- a/internal/util/unregisterer.go +++ b/internal/util/unregisterer.go @@ -18,6 +18,22 @@ func WrapWithUnregisterer(reg prometheus.Registerer) *Unregisterer { } } +// An "unchecked collector" is a collector which returns an empty description. +// It is described in the Prometheus documentation, here: +// https://pkg.go.dev/github.com/prometheus/client_golang/prometheus#hdr-Custom_Collectors_and_constant_Metrics +// +// > Alternatively, you could return no Desc at all, which will mark the Collector “unchecked”. +// > No checks are performed at registration time, but metric consistency will still be ensured at scrape time, +// > i.e. any inconsistencies will lead to scrape errors. Thus, with unchecked Collectors, +// > the responsibility to not collect metrics that lead to inconsistencies in the total scrape result +// > lies with the implementer of the Collector. While this is not a desirable state, it is sometimes necessary. +// > The typical use case is a situation where the exact metrics to be returned by a Collector cannot be predicted +// > at registration time, but the implementer has sufficient knowledge of the whole system to guarantee metric consistency. +// +// Unchecked collectors are used in the Loki "metrics" stage of the Loki "process" component. +// +// The isUncheckedCollector function is similar to how Prometheus' Go client extracts the metric description: +// https://github.com/prometheus/client_golang/blob/45f1e72421d9d11af6be784ad60b7389f7543e70/prometheus/registry.go#L372-L381 func isUncheckedCollector(c prometheus.Collector) bool { descChan := make(chan *prometheus.Desc, 10)