autodiscovery: suppress configuration discovery on generic-integration namespace conflicts

comp/core/autodiscovery/impl/configmgr.go

@@ -124,6 +124,10 @@ type reconcilingConfigManager struct {
 
 	// staticConfigIndex is a shared name set published to listeners so they
 	// can deduplicate templates against static configs (see ProcessService).
+	// It also tracks the namespace root (see listeners.NamespaceRoot) of every
+	// scheduled static openmetrics/prometheus config, so a configuration-
+	// discovery template can be suppressed when a host-wide generic-scraper
+	// config already claims its metric namespace (see filterTemplatesDiscovery).
 	// May be nil; callers that don't need cross-listener dedup can omit it.
 	staticConfigIndex *listeners.StaticConfigIndex
 
@@ -283,6 +287,17 @@ func (cm *reconcilingConfigManager) processNewConfig(config integration.Config)
 		// duplicate scheduled until something else perturbs the service.
 		if len(decryptedConfig.Instances) > 0 {
 			cm.staticConfigIndex.Add(config.Name)
+
+			// Also index the namespace root(s) of host-wide static
+			// openmetrics/prometheus configs under the same set, so discovery
+			// templates for a dedicated integration can be suppressed when
+			// such a config is already claiming the same metric namespace
+			// (see filterTemplatesDiscovery).
+			if listeners.IsGenericIntegrationCheckName(config.Name) {
+				for _, root := range listeners.GenericIntegrationNamespaceRoots(decryptedConfig) {
+					cm.staticConfigIndex.Add(root)
+				}
+			}
 		}
 	}
 
@@ -339,6 +354,12 @@ func (cm *reconcilingConfigManager) processDelConfigs(configs []integration.Conf
 			// Update the cross-listener index.
 			if len(config.Instances) > 0 {
 				cm.staticConfigIndex.Remove(config.Name)
+
+				if listeners.IsGenericIntegrationCheckName(config.Name) {
+					for _, root := range listeners.GenericIntegrationNamespaceRoots(config) {
+						cm.staticConfigIndex.Remove(root)
+					}
+				}
 			}
 		}
 

comp/core/autodiscovery/integration/config.go

@@ -130,7 +130,14 @@ type Config struct {
 }
 
 // DiscoveryConfig holds per-template configuration-discovery options.
-type DiscoveryConfig struct{}
+type DiscoveryConfig struct {
+	// MetricsPrefix is the integration's own metric namespace/prefix, as
+	// declared by its auto_conf.yaml's `discovery.metrics_prefix` field.
+	// Empty when the integration doesn't declare one, in which case
+	// consumers may fall back to the check name as the expected namespace
+	// root instead.
+	MetricsPrefix string `yaml:"metrics_prefix,omitempty"`
+}
 
 // MatchingProgram is an interface for matching objects against filter rules.
 type MatchingProgram interface {
@@ -481,7 +488,7 @@ func (c *Config) IntDigest() uint64 {
 	_, _ = h.Write([]byte(c.ServiceID))
 	_, _ = h.Write([]byte(strconv.FormatBool(c.IgnoreAutodiscoveryTags)))
 	if c.Discovery != nil {
-		_, _ = h.Write([]byte("discovery"))
+		_, _ = h.Write([]byte("discovery:" + c.Discovery.MetricsPrefix))
 	}
 
 	return h.Sum64()
@@ -508,7 +515,7 @@ func (c *Config) FastDigest() uint64 {
 	_, _ = h.Write([]byte(c.ServiceID))
 	_, _ = h.Write([]byte(strconv.FormatBool(c.IgnoreAutodiscoveryTags)))
 	if c.Discovery != nil {
-		_, _ = h.Write([]byte("discovery"))
+		_, _ = h.Write([]byte("discovery:" + c.Discovery.MetricsPrefix))
 	}
 
 	return h.Sum64()

comp/core/autodiscovery/integration/config_test.go

@@ -268,6 +268,21 @@ func TestDigestIncludesDiscovery(t *testing.T) {
 		"Discovery field must change the config digest so a discovery template and its non-discovery counterpart are distinct")
 	assert.NotEqual(t, withoutDiscovery.FastDigest(), withDiscovery.FastDigest(),
 		"Discovery field must change FastDigest as well")
+
+	withMetricsPrefixA := &Config{
+		Name:       "foo",
+		InitConfig: Data(""),
+		Discovery:  &DiscoveryConfig{MetricsPrefix: "a"},
+	}
+	withMetricsPrefixB := &Config{
+		Name:       "foo",
+		InitConfig: Data(""),
+		Discovery:  &DiscoveryConfig{MetricsPrefix: "b"},
+	}
+	assert.NotEqual(t, withMetricsPrefixA.Digest(), withMetricsPrefixB.Digest(),
+		"a change to Discovery.MetricsPrefix alone must change the digest, or an auto_conf.yaml update that only adds/changes it would be silently treated as an already-tracked config")
+	assert.NotEqual(t, withMetricsPrefixA.FastDigest(), withMetricsPrefixB.FastDigest(),
+		"a change to Discovery.MetricsPrefix alone must change FastDigest as well")
 }
 
 func TestDigestIncludesCELSelector(t *testing.T) {

comp/core/autodiscovery/listeners/BUILD.bazel

@@ -71,6 +71,7 @@ go_library(
         "//pkg/util/log",
         "//pkg/util/option",
         "@com_github_gosnmp_gosnmp//:gosnmp",
+        "@in_yaml_go_yaml_v2//:yaml",
         "@io_k8s_api//core/v1:core",
         "@io_k8s_api//discovery/v1:discovery",
         "@io_k8s_apimachinery//pkg/api/equality",
@@ -90,6 +91,7 @@ dd_agent_go_test(
     name = "listeners_test",
     srcs = [
         "cloudfoundry_test.go",
+        "common_filter_test.go",
         "common_test.go",
         "container_test.go",
         "dbm_aurora_test.go",

comp/core/autodiscovery/listeners/common_filter.go

@@ -6,6 +6,11 @@
 package listeners
 
 import (
+	"slices"
+	"strings"
+
+	yaml "go.yaml.in/yaml/v2"
+
 	"github.com/DataDog/datadog-agent/comp/core/autodiscovery/integration"
 	workloadfilter "github.com/DataDog/datadog-agent/comp/core/workloadfilter/def"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
@@ -29,24 +34,158 @@ func filterTemplatesMatched(svc FilterableService, configs map[string]integratio
 	}
 }
 
-// genericIntegrationNames are check names for generic metric-scraping
-// integrations that customers commonly point at any service, potentially
-// under a different check name than the one discovery would configure. Their
-// presence for a service (or host) is treated as covering every integration,
-// since we can't tell whether they already scrape the same metrics.
-var genericIntegrationNames = map[string]struct{}{
+// genericIntegrationCheckNames are check names whose entire configuration —
+// including the metric namespace — is supplied directly by the user, rather
+// than being intrinsic to a dedicated integration. These are commonly used as
+// a fallback to collect metrics from services that don't (yet) have a
+// dedicated Datadog integration. A configuration-discovery template is
+// suppressed when one of these already claims the same metric namespace the
+// discovery-driven integration would use, since that's a strong, specific
+// signal the user is already covering it manually.
+var genericIntegrationCheckNames = map[string]struct{}{
 	"openmetrics": {},
 	"prometheus":  {},
 }
 
+// IsGenericIntegrationCheckName reports whether name is a "generic"
+// integration check name (openmetrics, prometheus) — see
+// genericIntegrationCheckNames. Exported so the config manager can use the
+// same check to decide which scheduled static configs to also track by
+// namespace root in StaticConfigIndex.
+func IsGenericIntegrationCheckName(name string) bool {
+	_, ok := genericIntegrationCheckNames[name]
+	return ok
+}
+
+// NamespaceRoot returns the portion of namespace before the first '.', or the
+// whole string if there is none — e.g. "krakend.api" roots to "krakend".
+func NamespaceRoot(namespace string) string {
+	if i := strings.IndexByte(namespace, '.'); i >= 0 {
+		return namespace[:i]
+	}
+	return namespace
+}
+
+// ExpectedNamespaceRoot returns the metric-namespace root a discovery-driven
+// integration's own metrics are expected to be published under: the root of
+// its declared `discovery.metrics_prefix` (see integration.DiscoveryConfig)
+// when set, or its own check name otherwise — true for the vast majority of
+// integrations, with a small set of exceptions (e.g. gearmand's own namespace
+// is "gearman", not "gearmandd") that need metrics_prefix to be detected
+// correctly.
+//
+// Only the root is used even when metrics_prefix is itself multi-segment
+// (e.g. krakend's "krakend.api"): a generic scraper's own `namespace:` or
+// metric rename could independently collide at a shorter prefix (e.g.
+// `namespace: krakend` with no `.api`, or a pass-through metric whose raw
+// Prometheus name already happens to start with "krakend."), and there's no
+// way to tell from the raw instance config alone whether that would actually
+// produce the exact same final metric name — so comparing only the root
+// stays conservative and consistent with how the generic-scraper side of the
+// comparison already only ever compares roots (see
+// GenericIntegrationNamespaceRoots).
+func ExpectedNamespaceRoot(cfg integration.Config) string {
+	if cfg.Discovery != nil && cfg.Discovery.MetricsPrefix != "" {
+		return NamespaceRoot(cfg.Discovery.MetricsPrefix)
+	}
+	return cfg.Name
+}
+
+// GenericIntegrationNamespaceRoots returns, for each instance in cfg, the
+// metric-namespace root (see NamespaceRoot) it would submit metrics under:
+//   - if the instance sets an explicit `namespace:`, that field's root, or
+//   - otherwise, the root of each explicit metric rename target in the
+//     instance's `metrics`/`extra_metrics` field (see
+//     instanceMetricRenameTargets).
+//
+// The metrics-rename fallback only matters when namespace is unset: a
+// generic openmetrics/prometheus check submits `namespace.metric_name`, but
+// when namespace is empty the metric name is submitted completely
+// unprefixed (verified in datadog_checks_base's AgentCheck._format_namespace)
+// — so a rename target that's already a fully-qualified dotted name (e.g.
+// `envoy_cluster_http2_streams_active: envoy.cluster.http2.streams_active`)
+// collides with the native integration's own metric, and there's no
+// `namespace:` value to catch it. When namespace *is* set, it's prepended on
+// top of the rename target regardless, so the rename can't itself collide —
+// hence checking metrics only in the no-namespace case.
+//
+// Instances with neither an explicit namespace nor a qualifying rename
+// contribute nothing: with no signal to compare, assuming a match would risk
+// suppressing discovery unnecessarily. Exported so the config manager can use
+// the same logic to populate StaticConfigIndex with namespace roots from
+// scheduled static (non-template) generic-scraper configs.
+func GenericIntegrationNamespaceRoots(cfg integration.Config) []string {
+	var roots []string
+	for _, inst := range cfg.Instances {
+		var common integration.CommonInstanceConfig
+		if err := yaml.Unmarshal(inst, &common); err != nil {
+			log.Debugf("Error while checking namespace root for %s, skipping instance: %v", cfg.Name, err)
+			continue
+		}
+		if common.Namespace != "" {
+			roots = append(roots, NamespaceRoot(common.Namespace))
+			continue
+		}
+		for _, target := range instanceMetricRenameTargets(inst) {
+			roots = append(roots, NamespaceRoot(target))
+		}
+	}
+	return roots
+}
+
+// instanceMetricRenameTargets returns the explicit rename target of each
+// entry in inst's `metrics`/`extra_metrics` field that renames a raw metric
+// to a different name, mirroring the shapes accepted by
+// MetricTransformer.normalize_metric_config (openmetrics v2) and the legacy
+// metrics_mapper loops (openmetrics v1, prometheus) in datadog_checks_base:
+// each list entry is either
+//   - a plain string: pass-through, not a rename, skipped;
+//   - a single-key map to a string: the string is the rename target; or
+//   - a single-key map to a nested map with a `name` key: that key's value is
+//     the rename target (no `name` key means the raw metric name is kept,
+//     i.e. still not a rename, skipped).
+func instanceMetricRenameTargets(inst integration.Data) []string {
+	var raw struct {
+		Metrics      []interface{} `yaml:"metrics"`
+		ExtraMetrics []interface{} `yaml:"extra_metrics"`
+	}
+	if err := yaml.Unmarshal(inst, &raw); err != nil {
+		return nil
+	}
+	var targets []string
+	for _, entry := range slices.Concat(raw.Metrics, raw.ExtraMetrics) {
+		m, ok := entry.(map[interface{}]interface{})
+		if !ok {
+			continue // plain string (or any other scalar): pass-through, no rename
+		}
+		for _, value := range m {
+			switch v := value.(type) {
+			case string:
+				targets = append(targets, v)
+			case map[interface{}]interface{}:
+				if name, ok := v["name"].(string); ok {
+					targets = append(targets, name)
+				}
+			}
+		}
+	}
+	return targets
+}
+
 // filterTemplatesDiscovery drops configuration-discovery templates that are
-// redundant with another config source for the same integration. Dropped when:
+// redundant with another config source for the same integration, or with a
+// generic scraper (openmetrics/prometheus) config that's already claiming the
+// same metric namespace. Dropped when:
 //  1. another check template (Instances > 0) for the same integration Name has
 //     matched this same service (present in configs), or
-//  2. a scheduled non-template (static) config exists for the same Name
-//     (tracked in staticIdx), or
-//  3. a generic integration (openmetrics/prometheus) config matched this
-//     service or is scheduled host-wide, regardless of its Name.
+//  2. a sibling generic-scraper (openmetrics/prometheus) template matched to
+//     this same service configures a namespace whose root matches this
+//     integration's expected namespace root (see ExpectedNamespaceRoot), or
+//  3. a scheduled non-template (static) config exists for the same Name, or a
+//     scheduled non-template generic-scraper config anywhere on the host
+//     configures a namespace whose root matches this integration's expected
+//     namespace root (both tracked, by check name and by namespace root
+//     respectively, in the same staticIdx — see configmgr.go).
 //
 // Logs-only sibling templates (no Instances) are ignored — discovery covers
 // metric-check configuration and shouldn't be suppressed by an integration's
@@ -56,28 +195,26 @@ func filterTemplatesDiscovery(staticIdx *StaticConfigIndex, configs map[string]i
 		return
 	}
 	nonDiscoveryNames := map[string]struct{}{}
-	hasGenericSibling := false
+	siblingGenericNamespaceRoots := map[string]struct{}{}
 	for _, cfg := range configs {
-		if !cfg.IsDiscovery() && len(cfg.Instances) > 0 {
-			nonDiscoveryNames[cfg.Name] = struct{}{}
-			if _, ok := genericIntegrationNames[cfg.Name]; ok {
-				hasGenericSibling = true
-			}
+		if cfg.IsDiscovery() || len(cfg.Instances) == 0 {
+			continue
 		}
-	}
-	hasGenericStatic := false
-	for name := range genericIntegrationNames {
-		if staticIdx.Has(name) {
-			hasGenericStatic = true
-			break
+		nonDiscoveryNames[cfg.Name] = struct{}{}
+		if IsGenericIntegrationCheckName(cfg.Name) {
+			for _, root := range GenericIntegrationNamespaceRoots(cfg) {
+				siblingGenericNamespaceRoots[root] = struct{}{}
+			}
 		}
 	}
 	for digest, cfg := range configs {
 		if !cfg.IsDiscovery() {
 			continue
 		}
 		_, hasSibling := nonDiscoveryNames[cfg.Name]
-		if hasGenericSibling || hasGenericStatic || hasSibling || staticIdx.Has(cfg.Name) {
+		expectedRoot := ExpectedNamespaceRoot(cfg)
+		_, hasNamespaceConflict := siblingGenericNamespaceRoots[expectedRoot]
+		if hasSibling || hasNamespaceConflict || staticIdx.Has(cfg.Name) || (expectedRoot != cfg.Name && staticIdx.Has(expectedRoot)) {
 			log.Debugf("Ignoring discovery template %s from %s: another config source already covers this integration",
 				cfg.Name, cfg.Source)
 			delete(configs, digest)