Support IPv6 DogStatsD addresses - #399
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves UDP address resolution to correctly handle IPv6 literals (including bracketed forms and zone identifiers) by centralizing “add default port if missing” logic and expanding test coverage.
Changes:
- Introduced a
withPorthelper usingnet.SplitHostPort/net.JoinHostPortto normalize host/port handling (including IPv6). - Updated UDP URL parsing to reuse the same port-normalization logic.
- Added unit tests covering IPv6 address scenarios for env-based and URL-based configuration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| statsd/statsdex.go | Adds IPv6-safe port normalization via withPort and uses it for UDP URL parsing. |
| statsd/statsd_test.go | Expands test cases to validate IPv6 behavior across env vars and DD_DOGSTATSD_URL. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| func withPort(addr, port string) string { | ||
| if _, _, err := net.SplitHostPort(addr); err == nil { | ||
| return addr | ||
| } | ||
| if envPort != "" { | ||
| addr = fmt.Sprintf("%s:%s", addr, envPort) | ||
| } else { | ||
| addr = fmt.Sprintf("%s:%s", addr, defaultUDPPort) | ||
|
|
||
| // Preserve the existing handling of host:port-like addresses while treating | ||
| // addresses with multiple colons as unbracketed IPv6 literals. | ||
| if strings.Count(addr, ":") == 1 { | ||
| return addr | ||
| } | ||
| return addr | ||
|
|
||
| host := strings.TrimPrefix(addr, "[") | ||
| host = strings.TrimSuffix(host, "]") | ||
| return net.JoinHostPort(host, port) | ||
| } |
|
The PR description claims zone identifier support, and it works for direct addresses and |
|
|
||
| // Preserve the existing handling of host:port-like addresses while treating | ||
| // addresses with multiple colons as unbracketed IPv6 literals. | ||
| if strings.Count(addr, ":") == 1 { |
There was a problem hiding this comment.
AFAIR SplitHostPort already handles things like localhost:, :1234, foo:bar (basically any input with a single :) so this check might probably be unneeded.
Address resolution previously treated any colon as evidence that a port was already present. As a result, raw and bracketed IPv6 hosts could reach the UDP writer without a port and fail client initialization. Normalize IPv6 hosts with net.JoinHostPort while preserving explicitly provided ports. Apply the same behavior to direct addresses, DD_AGENT_HOST, and DD_DOGSTATSD_URL, and cover default ports, custom ports, bracketed hosts, and zone identifiers. Fixes DataDog#275.
eebf792 to
e577690
Compare
|
Thanks for the review! I removed the redundant single-colon check since I also updated the PR description to clarify that zone identifiers are supported for direct addresses and |
Hey, big fan of the Go migration here! I saw #275 and wanted to give a PR a shot.
Problem
The address resolver assumed that any address containing a colon already included a port. Raw IPv6 addresses such as
::1therefore reached the UDP writer without brackets or a port.Solution
Use Go’s standard address helpers to normalize IPv6 addresses while preserving explicit ports. This supports IPv6 addresses passed directly, through
DD_AGENT_HOST, or throughDD_DOGSTATSD_URL. Zone identifiers are supported for direct addresses andDD_AGENT_HOST.Testing
go test -race ./statsd -run ^TestResolveAddressFromEnvironment$ -count=1go vet ./...Fixes #275.