Skip to content

Commit c9150d5

Browse files
[Backport 7.82.x] autodiscovery: skip config discovery when a generic openmetrics/prometheus config already exists (#54270)
Backport 75b1f56 from #54263. ___ ## What does this PR do? Config discovery currently only skips proposing a discovered integration's check when another config *for the same integration name* already covers the service or host. In practice, many customers point the generic `openmetrics`/`prometheus` checks at a service instead of using the specific integration. Discovery doesn't recognize this as a conflict (different `Name`), so it still proposes the specific integration's check alongside the generic one, which can lead to duplicate — and for counters, doubled — metrics. This implements the "quickest fix" option from [Config discovery conflict with generic integrations](https://datadoghq.atlassian.net/wiki/spaces/DSCVR/pages/7031522288/Config+discovery+conflict+with+generic+integrations): if an `openmetrics` or `prometheus` config already matches the service, or is scheduled host-wide, skip config discovery for *every* integration on that service/host, regardless of `Name`. ## Motivation Avoid duplicate/doubled metrics when a generic `openmetrics`/`prometheus` config already covers the same service that config discovery would otherwise also configure via a specific integration. https://datadoghq.atlassian.net/browse/DSCVR-626 ## Describe how to test/verify your changes Added unit tests in `comp/core/autodiscovery/listeners/service_test.go` (`TestServiceFilterTemplatesDiscovery`) covering: - a discovery template is dropped when an `openmetrics` sibling config matches the same service, even though it has a different `Name` - a discovery template is dropped when `prometheus` is configured host-wide (in the static config index) - an unrelated discovery template on the same service/host is also dropped, since a generic integration match is treated as covering every integration ## Possible Drawbacks / Trade-offs This can cause false-positive skips: e.g. if a generic `openmetrics`/`prometheus` config targets one container/host but doesn't actually scrape the same metrics as the discoverable integration, or targets a different service, discovery will still be skipped. See the wiki page for the full discussion of alternatives that reduce this risk at the cost of being slower to ship. Co-authored-by: ali.benabdallah <ali.benabdallah@datadoghq.com>
1 parent 035c4cf commit c9150d5

2 files changed

Lines changed: 68 additions & 2 deletions

File tree

comp/core/autodiscovery/listeners/common_filter.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,24 @@ func filterTemplatesMatched(svc FilterableService, configs map[string]integratio
2929
}
3030
}
3131

32+
// genericIntegrationNames are check names for generic metric-scraping
33+
// integrations that customers commonly point at any service, potentially
34+
// under a different check name than the one discovery would configure. Their
35+
// presence for a service (or host) is treated as covering every integration,
36+
// since we can't tell whether they already scrape the same metrics.
37+
var genericIntegrationNames = map[string]struct{}{
38+
"openmetrics": {},
39+
"prometheus": {},
40+
}
41+
3242
// filterTemplatesDiscovery drops configuration-discovery templates that are
3343
// redundant with another config source for the same integration. Dropped when:
3444
// 1. another check template (Instances > 0) for the same integration Name has
3545
// matched this same service (present in configs), or
3646
// 2. a scheduled non-template (static) config exists for the same Name
37-
// (tracked in staticIdx).
47+
// (tracked in staticIdx), or
48+
// 3. a generic integration (openmetrics/prometheus) config matched this
49+
// service or is scheduled host-wide, regardless of its Name.
3850
//
3951
// Logs-only sibling templates (no Instances) are ignored — discovery covers
4052
// metric-check configuration and shouldn't be suppressed by an integration's
@@ -44,17 +56,28 @@ func filterTemplatesDiscovery(staticIdx *StaticConfigIndex, configs map[string]i
4456
return
4557
}
4658
nonDiscoveryNames := map[string]struct{}{}
59+
hasGenericSibling := false
4760
for _, cfg := range configs {
4861
if !cfg.IsDiscovery() && len(cfg.Instances) > 0 {
4962
nonDiscoveryNames[cfg.Name] = struct{}{}
63+
if _, ok := genericIntegrationNames[cfg.Name]; ok {
64+
hasGenericSibling = true
65+
}
66+
}
67+
}
68+
hasGenericStatic := false
69+
for name := range genericIntegrationNames {
70+
if staticIdx.Has(name) {
71+
hasGenericStatic = true
72+
break
5073
}
5174
}
5275
for digest, cfg := range configs {
5376
if !cfg.IsDiscovery() {
5477
continue
5578
}
5679
_, hasSibling := nonDiscoveryNames[cfg.Name]
57-
if hasSibling || staticIdx.Has(cfg.Name) {
80+
if hasGenericSibling || hasGenericStatic || hasSibling || staticIdx.Has(cfg.Name) {
5881
log.Debugf("Ignoring discovery template %s from %s: another config source already covers this integration",
5982
cfg.Name, cfg.Source)
6083
delete(configs, digest)

comp/core/autodiscovery/listeners/service_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,13 @@ func TestServiceFilterTemplatesDiscovery(t *testing.T) {
194194
Instances: []integration.Data{[]byte("port: 80")},
195195
Source: "file:nginx/auto_conf.yaml",
196196
}
197+
unrelatedDiscoveryTpl := integration.Config{
198+
Name: "nginx",
199+
Provider: names.File,
200+
ADIdentifiers: []string{"nginx"},
201+
Discovery: &integration.DiscoveryConfig{},
202+
Source: "file:nginx/auto_conf.yaml",
203+
}
197204

198205
containsDigests := func(configs map[string]integration.Config, want ...integration.Config) []string {
199206
t.Helper()
@@ -268,6 +275,42 @@ func TestServiceFilterTemplatesDiscovery(t *testing.T) {
268275
"logs-only sibling should be kept")
269276
})
270277

278+
t.Run("discovery dropped when generic integration sibling matches same service", func(t *testing.T) {
279+
openmetricsSibling := integration.Config{
280+
Name: "openmetrics",
281+
Provider: names.File,
282+
ADIdentifiers: []string{"redis"},
283+
Instances: []integration.Data{[]byte("openmetrics_endpoint: http://%%host%%:9121/metrics")},
284+
Source: "file:openmetrics/conf.yaml",
285+
}
286+
configs := map[string]integration.Config{
287+
discoveryTpl.Digest(): discoveryTpl,
288+
openmetricsSibling.Digest(): openmetricsSibling,
289+
unrelatedDiscoveryTpl.Digest(): unrelatedDiscoveryTpl,
290+
}
291+
mkSvc(NewStaticConfigIndex()).FilterTemplates(configs)
292+
assert.NotContains(t, configs, discoveryTpl.Digest(),
293+
"discovery template should be dropped when an openmetrics config matches the same service, even under a different Name")
294+
assert.Contains(t, configs, openmetricsSibling.Digest(), "openmetrics config should be kept")
295+
assert.NotContains(t, configs, unrelatedDiscoveryTpl.Digest(),
296+
"unrelated discovery template should also be dropped: a generic integration match covers every integration")
297+
})
298+
299+
t.Run("discovery dropped when generic integration is configured host-wide", func(t *testing.T) {
300+
idx := NewStaticConfigIndex()
301+
idx.Add("prometheus")
302+
303+
configs := map[string]integration.Config{
304+
discoveryTpl.Digest(): discoveryTpl,
305+
unrelatedDiscoveryTpl.Digest(): unrelatedDiscoveryTpl,
306+
}
307+
mkSvc(idx).FilterTemplates(configs)
308+
assert.NotContains(t, configs, discoveryTpl.Digest(),
309+
"discovery template should be dropped when a prometheus config is scheduled host-wide")
310+
assert.NotContains(t, configs, unrelatedDiscoveryTpl.Digest(),
311+
"unrelated discovery template should also be dropped: a generic integration match covers every integration")
312+
})
313+
271314
t.Run("instrumentation check overrides matched file check", func(t *testing.T) {
272315
fileTpl := integration.Config{
273316
Name: "redis",

0 commit comments

Comments
 (0)