diff --git a/comp/core/autodiscovery/common/utils/annotations.go b/comp/core/autodiscovery/common/utils/annotations.go index ed7f516d73b7..1096116065dc 100644 --- a/comp/core/autodiscovery/common/utils/annotations.go +++ b/comp/core/autodiscovery/common/utils/annotations.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "strconv" + "strings" "github.com/DataDog/datadog-agent/comp/core/autodiscovery/integration" ) @@ -25,6 +26,23 @@ const ( checkTagCardinality = "check_tag_cardinality" ) +// v1CheckKeys holds the suffixes of the keys defining a v1 check +// configuration. ignore_autodiscovery_tags and logs are left out on purpose: +// they can be read next to a v2 check configuration, so setting them alongside +// one does not mean they are ignored. For instance, ignore_autodiscovery_tags is +// applied to v2 configurations when +// cluster_checks.support_hybrid_ignore_ad_tags is enabled. +// +// check_tag_cardinality is also left out, for a different reason: a standalone +// one is dropped next to a v2 configuration, but that happens whether or not v1 +// keys are set, so it is a separate gap rather than a consequence of mixing +// formats. +var v1CheckKeys = []string{ + checkNamePath, + initConfigPath, + instancePath, +} + // ExtractTemplatesFromMap looks for autodiscovery configurations in a given // map and returns them if found. func ExtractTemplatesFromMap(key string, input map[string]string, prefix string) ([]integration.Config, []error) { @@ -275,6 +293,13 @@ func extractTemplatesFromMapWithV2(entityName string, annotations map[string]str } else { configs = append(configs, c...) } + + // v1 and legacy check configurations are not applied when a v2 one + // is set on the same entity. Report them, as they would otherwise + // be dropped without any feedback. + if ignored := findIgnoredCheckKeys(annotations, prefixCandidates); len(ignored) > 0 { + errors = append(errors, ignoredCheckKeysError(prefix+checksPath, ignored)) + } } else { // AD annotations v1: "ad.datadoghq.com/redis.check_names" // AD annotations legacy: "service-discovery.datadoghq.com/redis.check_names" @@ -287,6 +312,12 @@ func extractTemplatesFromMapWithV2(entityName string, annotations map[string]str } else { configs = append(configs, c...) } + + // Same as above for the formats with a lower priority than the one that was found. + lowerPriority := lowerPriorityPrefixes(prefixCandidates, actualPrefix) + if ignored := findIgnoredCheckKeys(annotations, lowerPriority); len(ignored) > 0 { + errors = append(errors, ignoredCheckKeysError(actualPrefix+checkNamePath, ignored)) + } } } @@ -321,3 +352,40 @@ func findPrefix(annotations map[string]string, prefixes []string, suffix string) return "" } + +// lowerPriorityPrefixes returns the prefixes that come after usedPrefix in +// prefixes, which is ordered by decreasing priority. +func lowerPriorityPrefixes(prefixes []string, usedPrefix string) []string { + for idx, prefix := range prefixes { + if prefix == usedPrefix { + return prefixes[idx+1:] + } + } + + return nil +} + +// findIgnoredCheckKeys returns the keys holding a check configuration that is +// not applied because a configuration with a higher priority was found on the +// same entity. Autodiscovery only applies one check configuration format per +// entity, so mixing them drops part of the configuration. +func findIgnoredCheckKeys(input map[string]string, ignoredPrefixes []string) []string { + var ignored []string + + for _, prefix := range ignoredPrefixes { + for _, suffix := range v1CheckKeys { + key := prefix + suffix + if _, found := input[key]; found { + ignored = append(ignored, key) + } + } + } + + return ignored +} + +// ignoredCheckKeysError builds the error reported when several check +// configuration formats are set on the same entity. +func ignoredCheckKeysError(usedKey string, ignoredKeys []string) error { + return fmt.Errorf("%s takes precedence, ignoring %s: Autodiscovery only applies the check configuration with the highest priority (v2, then v1, then legacy)", usedKey, strings.Join(ignoredKeys, ", ")) +} diff --git a/comp/core/autodiscovery/common/utils/container_labels_test.go b/comp/core/autodiscovery/common/utils/container_labels_test.go index 243306d7a635..eb29a68da784 100644 --- a/comp/core/autodiscovery/common/utils/container_labels_test.go +++ b/comp/core/autodiscovery/common/utils/container_labels_test.go @@ -195,6 +195,9 @@ func TestExtractTemplatesFromContainerLabels(t *testing.T) { ADIdentifiers: []string{adID}, }, }, + errs: []error{ + errors.New("com.datadoghq.ad.checks takes precedence, ignoring com.datadoghq.ad.check_names: Autodiscovery only applies the check configuration with the highest priority (v2, then v1, then legacy)"), + }, }, } diff --git a/comp/core/autodiscovery/common/utils/pod_annotations_test.go b/comp/core/autodiscovery/common/utils/pod_annotations_test.go index ad59cf3238f0..a07eef0dbc1a 100644 --- a/comp/core/autodiscovery/common/utils/pod_annotations_test.go +++ b/comp/core/autodiscovery/common/utils/pod_annotations_test.go @@ -356,6 +356,9 @@ func TestExtractTemplatesFromAnnotations(t *testing.T) { ADIdentifiers: []string{adID}, }, }, + errs: []error{ + errors.New("ad.datadoghq.com/foobar.checks takes precedence, ignoring ad.datadoghq.com/foobar.check_names, service-discovery.datadoghq.com/foobar.check_names: Autodiscovery only applies the check configuration with the highest priority (v2, then v1, then legacy)"), + }, }, { name: "v2 annotations with ignore_ad_tags", @@ -381,6 +384,9 @@ func TestExtractTemplatesFromAnnotations(t *testing.T) { IgnoreAutodiscoveryTags: true, }, }, + errs: []error{ + errors.New("ad.datadoghq.com/foobar.checks takes precedence, ignoring ad.datadoghq.com/foobar.check_names, service-discovery.datadoghq.com/foobar.check_names: Autodiscovery only applies the check configuration with the highest priority (v2, then v1, then legacy)"), + }, }, { name: "v2 annotations with adv1 ignore_ad_tags", @@ -406,6 +412,9 @@ func TestExtractTemplatesFromAnnotations(t *testing.T) { IgnoreAutodiscoveryTags: false, }, }, + errs: []error{ + errors.New("ad.datadoghq.com/foobar.checks takes precedence, ignoring ad.datadoghq.com/foobar.check_names, service-discovery.datadoghq.com/foobar.check_names: Autodiscovery only applies the check configuration with the highest priority (v2, then v1, then legacy)"), + }, }, { name: "v2 annotations with init_config", @@ -473,6 +482,56 @@ func TestExtractTemplatesFromAnnotations(t *testing.T) { }, }, }, + { + name: "complete v1 and v2 check configurations on the same identifier", + annotations: map[string]string{ + "ad.datadoghq.com/foobar.checks": `{ + "apache": { + "instances": [ + {"apache_status_url":"http://%%host%%/server-status?auto"} + ] + } + }`, + "ad.datadoghq.com/foobar.check_names": `["http_check"]`, + "ad.datadoghq.com/foobar.init_configs": `[{}]`, + "ad.datadoghq.com/foobar.instances": `[{"name":"My service","url":"http://%%host%%"}]`, + }, + adIdentifier: "foobar", + output: []integration.Config{ + { + Name: "apache", + Instances: []integration.Data{integration.Data(`{"apache_status_url":"http://%%host%%/server-status?auto"}`)}, + InitConfig: integration.Data("{}"), + ADIdentifiers: []string{adID}, + }, + }, + errs: []error{ + errors.New("ad.datadoghq.com/foobar.checks takes precedence, ignoring ad.datadoghq.com/foobar.check_names, ad.datadoghq.com/foobar.init_configs, ad.datadoghq.com/foobar.instances: Autodiscovery only applies the check configuration with the highest priority (v2, then v1, then legacy)"), + }, + }, + { + name: "complete v1 and legacy check configurations on the same identifier", + annotations: map[string]string{ + "ad.datadoghq.com/foobar.check_names": `["apache"]`, + "ad.datadoghq.com/foobar.init_configs": `[{}]`, + "ad.datadoghq.com/foobar.instances": `[{"apache_status_url":"http://%%host%%/server-status?auto"}]`, + "service-discovery.datadoghq.com/foobar.check_names": `["http_check"]`, + "service-discovery.datadoghq.com/foobar.init_configs": `[{}]`, + "service-discovery.datadoghq.com/foobar.instances": `[{"name":"My service","url":"http://%%host%%"}]`, + }, + adIdentifier: "foobar", + output: []integration.Config{ + { + Name: "apache", + Instances: []integration.Data{integration.Data(`{"apache_status_url":"http://%%host%%/server-status?auto"}`)}, + InitConfig: integration.Data("{}"), + ADIdentifiers: []string{adID}, + }, + }, + errs: []error{ + errors.New("ad.datadoghq.com/foobar.check_names takes precedence, ignoring service-discovery.datadoghq.com/foobar.check_names, service-discovery.datadoghq.com/foobar.init_configs, service-discovery.datadoghq.com/foobar.instances: Autodiscovery only applies the check configuration with the highest priority (v2, then v1, then legacy)"), + }, + }, } for _, tt := range tests { diff --git a/comp/core/autodiscovery/providers/container_test.go b/comp/core/autodiscovery/providers/container_test.go index 9ae76b6449a8..c40cc8c6b8c4 100644 --- a/comp/core/autodiscovery/providers/container_test.go +++ b/comp/core/autodiscovery/providers/container_test.go @@ -164,7 +164,9 @@ func TestGenerateConfig(t *testing.T) { Source: "container:docker://3b8efe0c50e8", }, }, - expectedErr: nil, + expectedErr: types.ErrorMsgSet{ + "ad.datadoghq.com/apache.checks takes precedence, ignoring ad.datadoghq.com/apache.check_names, ad.datadoghq.com/apache.init_configs, ad.datadoghq.com/apache.instances: Autodiscovery only applies the check configuration with the highest priority (v2, then v1, then legacy)": {}, + }, }, { name: "New + old, new takes over", @@ -195,7 +197,9 @@ func TestGenerateConfig(t *testing.T) { Source: "container:docker://3b8efe0c50e8", }, }, - expectedErr: nil, + expectedErr: types.ErrorMsgSet{ + "ad.datadoghq.com/apache.check_names takes precedence, ignoring service-discovery.datadoghq.com/apache.check_names, service-discovery.datadoghq.com/apache.init_configs, service-discovery.datadoghq.com/apache.instances: Autodiscovery only applies the check configuration with the highest priority (v2, then v1, then legacy)": {}, + }, }, { name: "New annotation prefix, two templates", diff --git a/releasenotes/notes/report-ignored-ad-check-configs-34903b7a393295f0.yaml b/releasenotes/notes/report-ignored-ad-check-configs-34903b7a393295f0.yaml new file mode 100644 index 000000000000..81252baef7f1 --- /dev/null +++ b/releasenotes/notes/report-ignored-ad-check-configs-34903b7a393295f0.yaml @@ -0,0 +1,17 @@ +# Each section from every release note are combined when the +# CHANGELOG.rst is rendered. So the text needs to be worded so that +# it does not depend on any information only available in another +# section. This may mean repeating some details, but each section +# must be readable independently of the other. +# +# Each section note must be formatted as reStructuredText. +--- +enhancements: + - | + Autodiscovery now reports the check configuration keys it ignores when + several annotation or label formats are set on the same entity. Only the + format with the highest priority is applied (``checks``, then + ``check_names`` with ``init_configs`` and ``instances``, then the legacy + ``service-discovery.datadoghq.com`` prefix), and the others used to be + discarded silently. The ignored keys are now listed in the Autodiscovery + section of the ``agent status`` output.