[DBMON-6669] Fix resolve_db_host for IPv6 loopback addresses#23849
[DBMON-6669] Fix resolve_db_host for IPv6 loopback addresses#23849eric-weaver wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80c7783c9f
ℹ️ 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".
|
Codecov Report❌ Patch coverage is Additional details and impacted files🚀 New features to boost your workflow:
|
AAraKKe
left a comment
There was a problem hiding this comment.
Hi @eric-weaver, this review was generated with Claude Code and has been looked over and approved before posting. I believe the comments below are valid — feel free to discuss any you disagree with.
Comment legend
praise: no action needed, just celebrate!
note: informational, no action required.
question: seeking clarification or understanding your approach.
nit: minor, non-blocking (style, typo). Feel free to ignore.
suggestion: optional improvement, recommended but not required.
request: a change I believe is necessary before merging.
| host = db_host.strip() | ||
| if host.startswith('[') and host.endswith(']'): | ||
| host = host[1:-1] | ||
| if '%' in host: |
There was a problem hiding this comment.
Question: Is there a reason to route zone-scoped addresses through IPv6Interface(host).ip rather than ip_address(host) directly? In Python 3.9+, ip_address("fe80::1%eth0") returns an IPv6Address with scope_id set, and is_loopback works correctly on the result. IPv6Interface is semantically for CIDR network addresses and defaults to /128 when no prefix is given, so the detour works but is unusual. If this is a holdover from earlier Python compatibility, the pyproject.toml for datadog-checks-base targets 3.13 — ip_address for both branches would be a small simplification.
There was a problem hiding this comment.
Agree this simplifies it. Made the change
Review from sethsamuel is dismissed. Related teams and files:
- database-monitoring-agent
- datadog_checks_base/datadog_checks/base/utils/db/utils.py
- datadog_checks_base/tests/base/utils/db/test_util.py
Review from Kyle-Neale is dismissed. Related teams and files:
- agent-integrations
- datadog_checks_base/datadog_checks/base/utils/db/utils.py
- datadog_checks_base/tests/base/utils/db/test_util.py
Review from AAraKKe is dismissed. Related teams and files:
- agent-integrations
- datadog_checks_base/datadog_checks/base/utils/db/utils.py
- datadog_checks_base/tests/base/utils/db/test_util.py
Validation ReportAll 21 validations passed. Show details
|
What does this PR do?
Fixes
resolve_db_hostindatadog-checks-baseso loopback database endpoints use the agent hostname for metrics instead of the raw IP literal._try_parse_db_host_ipto parse IPv4/IPv6 address literals (bracketed IPv6, zone-scoped IPv6 viaIPv6Interface)._is_local_db_hostto detect localhost, unix sockets, and loopback IPs (127.0.0.1,::1,[::1], etc.).datadog_agent.get_hostname()beforegethostbyname, restoring correcthosttags anddatadog.yamlhost-level tags on MySQL/Postgres/SQL Server metrics.test_resolve_db_hostcases.Motivation
Fixes DataDog/datadog-agent#51280: on Agent 7.79.0, MySQL metrics lost
datadog.yamlhost tags and usedhost:::1whenserverwas set to::1. Agent 7.78.4 was unaffected.Agent 7.79.0 bundles
datadog-checks-base37.35.x, which includes #23181: whengethostbynamefails,resolve_db_hostreturns the configureddb_hostrather than the agent hostname. That is correct for remote hostnames that fail DNS lookup, but loopback IP literals (e.g.::1) also failgethostbynamebecause it is IPv4-only—they must not be treated as DNS failures. This PR classifies loopback addresses as local before any name resolution, so we keep #23181’s semantics for genuinely unresolvable remote hosts while restoring the agent hostname for on-host database connections.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged