feat(autodiscovery): report ignored keys when AD formats are mixed - #54941
feat(autodiscovery): report ignored keys when AD formats are mixed#54941jumpe1 wants to merge 2 commits into
Conversation
Autodiscovery only applies one check configuration format per entity (v2, then v1, then legacy), and until now the formats that lost the precedence race were dropped without any feedback. Report the ignored keys through the errors extractTemplatesFromMapWithV2 already collects, so they surface in `agent status` and as an AD annotation health issue.
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd3cf22474
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // v1CheckKeys holds the suffixes of the keys defining a v1 check | ||
| // configuration. The optional keys (ignore_autodiscovery_tags, | ||
| // check_tag_cardinality) 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. |
There was a problem hiding this comment.
Report ignored standalone cardinality keys
When a user migrates the main fields to ad.datadoghq.com/<id>.checks but leaves the supported standalone check_tag_cardinality annotation in place, parseChecksJSON only reads cardinality from inside each v2 check object, so the standalone value is silently discarded. Excluding this key from v1CheckKeys therefore suppresses the warning in a migration scenario where check tagging changes; include it among the ignored keys rather than claiming it is read alongside v2.
Useful? React with 👍 / 👎.
What does this PR do?
Autodiscovery applies one check configuration format per entity — v2 (
checks), then v1 (check_names+init_configs+instances), then legacy — and until now the ones that lost were dropped silently. This reports them through the errorsextractTemplatesFromMapWithV2already collects, so they surface inagent statusunderConfiguration Errorsand as an AD annotation health issue. That function is the common entry point for pod annotations, container labels and Kubernetes service/endpoints annotations.Detection is limited to the keys that define a check:
ignore_autodiscovery_tagsandlogsare excluded because they are read next to a v2 configuration — the former is applied on top of one whencluster_checks.support_hybrid_ignore_ad_tagsis enabled. Following #41372, this uses the stored errors rather than a log line.Motivation
A team on a cluster we operate followed the docs and added a v2 annotation to a workload that already had v1 ones, and their existing checks silently stopped — no warning, no log line, nothing in
agent status. Both forms are valid Autodiscovery syntax, so nothing looked wrong at a glance, and finding the cause took a while. The precedence is intentional, but it is documented only in a code comment.Describe how you validated your changes
Added cases for v1 next to v2 and v1 next to legacy; six existing mixed-format cases were updated. The single-format cases still assert no error, and the hybrid-mode case in
kube_services_test.goguards the exclusion above.Additional Notes
Users who mix formats today will see a new error in
agent statusand a new AD annotation health issue (MEDIUM); the applied configuration does not change.Follow-ups not included: the health issue's remediation steps, documenting the precedence rule, and reporting a standalone
check_tag_cardinalitythat is dropped next to a v2 configuration.