Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ipv6 support for dogstatsd #791

Merged
merged 4 commits into from
Sep 5, 2023
Merged

Add ipv6 support for dogstatsd #791

merged 4 commits into from
Sep 5, 2023

Conversation

vickenty
Copy link
Contributor

@vickenty vickenty commented Sep 4, 2023

What does this PR do?

Add IPv6 support for hostnames and literal addresses.

Description of the Change

Call getaddrinfo to parse and resolve an internet address before creating the socket. Try in a loop until one address succeeds.

For backwards compatibility, prefer ipv4 addresses: ipv4-only agent should still work even if hostname (e.g. localhost) resolves to both ipv4 and ipv6 address and ipv6 is preferred by the OS. This also matches behavior of Golang.

Alternate Designs

Possible Drawbacks

Address order specified in gai.conf is ignored, which may lead to inconsistencies with other applications.

Verification Process

Additional Notes

Release Notes

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

@vickenty vickenty marked this pull request as ready for review September 4, 2023 16:09
@vickenty vickenty requested review from a team as code owners September 4, 2023 16:09
@vickenty vickenty added the changelog/Added Added features results into a minor version bump label Sep 5, 2023
Keyword arguments not supported on py2.7
remeh
remeh previously approved these changes Sep 5, 2023
Copy link
Contributor

@remeh remeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a small remark, approved for a future merge as the rest looks good to me!

sock = None
try:
sock = socket.socket(af, ty, proto)
sock.connect(addr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since your change, are you sure it's OK to call the settimeout after the call to connect? Could be for UDP but might be worth preserving the old behavior: https://docs.python.org/3/library/socket.html#timeouts-and-the-connect-method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure socket timeout doesn't make any difference for udp sockets, but just for symmetry I updated the code to have the order same between udp and uds.

This makes no difference with udp sockets, but do it anyway for
symmetry and ease of maintenace.
@vickenty vickenty merged commit cb538b7 into master Sep 5, 2023
@vickenty vickenty deleted the vickenty/ipv6 branch September 5, 2023 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/Added Added features results into a minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants