Pull request #402: Fix missing custom metrics when using Docker label "glouton.allow_metric"

Merge in PRODUCT/glouton from bugfix/PRODUCT-2833-glouton-prometheus-exporter-no-longer-working to main

* commit '51425ce804158e4c6f823663f54f0c2f20f0c865':
  Fix issue with update of metrics filter

Author: PierreF

agent/agent.go

@@ -145,6 +145,8 @@ type agent struct {
 	lastContainerEventTime time.Time
 	watchdogRunAt          []time.Time
 	metricFilter           *metricFilter
+	promFilter             *metricFilter
+	mergeMetricFilter      *metricFilter
 	monitorManager         *blackbox.RegisterManager
 	rulesManager           *rules.Manager
 	reloadState            ReloadState
@@ -648,7 +650,7 @@ func (a *agent) updateThresholds(ctx context.Context, thresholds map[string]thre
 	if err != nil {
 		logger.V(2).Printf("An error occurred while running discoveries for updateThresholds: %v", err)
 	} else {
-		err = a.metricFilter.RebuildDynamicLists(a.dynamicScrapper, services, a.threshold.GetThresholdMetricNames(), a.rulesManager.MetricNames())
+		err = a.rebuildDynamicMetricAllowDenyList(services)
 		if err != nil {
 			logger.V(2).Printf("An error occurred while rebuilding dynamic list for updateThresholds: %v", err)
 		}
@@ -671,6 +673,24 @@ func (a *agent) updateThresholds(ctx context.Context, thresholds map[string]thre
 	}
 }
 
+func (a *agent) rebuildDynamicMetricAllowDenyList(services []discovery.Service) error {
+	var errs []error
+
+	errs = append(
+		errs,
+		a.metricFilter.RebuildDynamicLists(a.dynamicScrapper, services, a.threshold.GetThresholdMetricNames(), a.rulesManager.MetricNames()),
+	)
+
+	errs = append(
+		errs,
+		a.promFilter.RebuildDynamicLists(a.dynamicScrapper, services, a.threshold.GetThresholdMetricNames(), a.rulesManager.MetricNames()),
+	)
+
+	a.mergeMetricFilter.mergeInPlace(a.metricFilter, a.promFilter)
+
+	return errors.Join(errs...)
+}
+
 // Run will start the agent. It will terminate when sigquit/sigterm/sigint is received.
 func (a *agent) run(ctx context.Context, sighupChan chan os.Signal) { //nolint:maintidx
 	ctx, cancel := context.WithCancel(ctx)
@@ -800,6 +820,9 @@ func (a *agent) run(ctx context.Context, sighupChan chan os.Signal) { //nolint:m
 		logger.Printf("An error occurred while building the Prometheus metric filter, allow/deny list may be partial: %v", err)
 	}
 
+	a.promFilter = promFilter
+	a.mergeMetricFilter = mergeMetricFilters(a.promFilter, bleemeoFilter)
+
 	secretInputsGate := gate.New(inputs.MaxParallelSecrets())
 
 	a.gathererRegistry, err = registry.New(
@@ -809,7 +832,7 @@ func (a *agent) run(ctx context.Context, sighupChan chan os.Signal) { //nolint:m
 			FQDN:                  fqdn,
 			GloutonPort:           strconv.Itoa(a.config.Web.Listener.Port),
 			BlackboxSendScraperID: a.config.Blackbox.ScraperSendUUID,
-			Filter:                mergeMetricFilters(promFilter, bleemeoFilter),
+			Filter:                a.mergeMetricFilter,
 			Queryable:             a.store,
 			SecretInputsGate:      secretInputsGate,
 			ShutdownDeadline:      15 * time.Second,
@@ -1943,7 +1966,7 @@ func (a *agent) handleTrigger(ctx context.Context) {
 				}
 			}
 
-			err := a.metricFilter.RebuildDynamicLists(a.dynamicScrapper, services, a.threshold.GetThresholdMetricNames(), a.rulesManager.MetricNames())
+			err := a.rebuildDynamicMetricAllowDenyList(services)
 			if err != nil {
 				logger.V(2).Printf("Error during dynamic Filter rebuild: %v", err)
 			}

agent/metric.go

@@ -974,6 +974,18 @@ func newMetricFilter(metricCfg config.Metric, hasSNMP, hasSwap, forBleemeo bool)
 // that represents the union of both the given filters.
 // This only works if the deny-list uses equal comparison (no regexp), or is identical.
 func mergeMetricFilters(f1, f2 *metricFilter) *metricFilter {
+	result := &metricFilter{}
+	result.mergeInPlace(f1, f2)
+
+	return result
+}
+
+// mergeInPlace merge the two input filter (f1 & f2) and update the filter m.
+// All previous content of m is overwrite, the result only depend on f1 & f2.
+// The merge result is the union of both result the given filters (metrics that are allowed by
+// one of the filter).
+// This only works if the deny-list uses equal comparison (no regexp), or is identical.
+func (m *metricFilter) mergeInPlace(f1, f2 *metricFilter) {
 	// Merging logic:
 	// - to be allowed, a metric only needs to be present in the allowlist of one of the filters
 	// - to be denied, a metric needs to be present in the deny-list of both the filters
@@ -997,7 +1009,7 @@ func mergeMetricFilters(f1, f2 *metricFilter) *metricFilter {
 		}
 	}
 
-	return &metricFilter{
+	*m = metricFilter{
 		includeDefaultMetrics: f1.includeDefaultMetrics || f2.includeDefaultMetrics,
 		staticAllowList:       slices.Concat(f1.staticAllowList, f2.staticAllowList),
 		staticDenyList:        staticDenyList,

agent/metric_test.go

@@ -1562,6 +1562,62 @@ func Test_MergeMetricFilters(t *testing.T) {
 	}
 }
 
+// Test_MergeMetricFiltersMutation ensure that it's possible to modify in place
+// the merge filter. This is required because filter get updated by RebuildDynamicLists.
+func Test_MergeMetricFiltersMutation(t *testing.T) {
+	t.Parallel()
+
+	mustMatchers := func(s string) matcher.Matchers {
+		matchers, err := matcher.NormalizeMetric(s)
+		if err != nil {
+			t.Fatalf("Can't normalize metric %q: %v", s, err)
+		}
+
+		return matchers
+	}
+
+	metricCfg := config.Metric{
+		IncludeDefaultMetrics: false,
+		AllowMetrics:          []string{"m1", "m2"},
+		DenyMetrics:           []string{"no1"},
+	}
+	mf1, _ := newMetricFilter(metricCfg, false, false, false)
+
+	metricCfg.AllowMetrics = []string{"m3", "m4"}
+	metricCfg.DenyMetrics = []string{"no1", "no2"}
+	mf2, _ := newMetricFilter(metricCfg, false, false, false)
+
+	mergedFilter := mergeMetricFilters(mf1, mf2)
+	shallowCopy := mergedFilter
+
+	metricCfg2 := config.Metric{
+		IncludeDefaultMetrics: false,
+		AllowMetrics:          []string{"m1", "m2"},
+		DenyMetrics:           []string{"m4"},
+	}
+	mf1b, _ := newMetricFilter(metricCfg2, false, false, false)
+
+	mergedFilter.mergeInPlace(mf1b, mf2)
+
+	expectedFilter := &metricFilter{
+		includeDefaultMetrics: false,
+		staticAllowList:       []matcher.Matchers{mustMatchers("m1"), mustMatchers("m2"), mustMatchers("m3"), mustMatchers("m4")},
+		staticDenyList:        nil,
+		allowList: map[labels.Matcher][]matcher.Matchers{
+			*labels.MustNewMatcher(labels.MatchEqual, types.LabelName, "m1"): {mustMatchers("m1")},
+			*labels.MustNewMatcher(labels.MatchEqual, types.LabelName, "m2"): {mustMatchers("m2")},
+			*labels.MustNewMatcher(labels.MatchEqual, types.LabelName, "m3"): {mustMatchers("m3")},
+			*labels.MustNewMatcher(labels.MatchEqual, types.LabelName, "m4"): {mustMatchers("m4")},
+		},
+		denyList: map[labels.Matcher][]matcher.Matchers{},
+	}
+
+	cmpOpts := cmp.Options{cmp.AllowUnexported(metricFilter{}), cmpopts.IgnoreTypes(sync.Mutex{}), cmpopts.IgnoreFields(labels.Matcher{}, "re")}
+	if diff := cmp.Diff(expectedFilter, shallowCopy, cmpOpts); diff != "" {
+		t.Fatalf("Unexpected filter merge result: (-want +got):\n%s", diff)
+	}
+}
+
 func Benchmark_MultipleFilters(b *testing.B) {
 	rawData, err := os.ReadFile("../prometheus/scrapper/testdata/large.txt")
 	if err != nil {