Resolve address before writing UDP packets

[bessb]

The current UDP connection logic will resolve an address only when the writer is created. This is undesirable in cases where the address may change at a later point.

In our case, we have agents deployed in Kubernetes clusters using Cilium CNI. Cilium has an eBPF hook for the connect() syscall that will resolve to the pod IP the of the agent. When an agent pod is terminated, a new agent pod is spun up with a different IP address. In order to pick up the new IP address it needs to be resolved again when Write() is called.

This should also fix #263.

[christian-10101]

@carlosroman

[guidao]

Hi @carlosroman, can you help review this pr? This is really a useful feature.

[carlosroman]

@guidao I've had a look and very curious if this will resolve #263 or not. Just reading the docs on net.ResolveUDPAddr and if you give the function a "literal IP address" (such as 127.0.0.1) then it will return that address back.

By any chance were you able to test this out to see if it did indeed fix the issue with Cilium? I don't think this will try and resolve 127.0.0.1 to the pod IP as Cilium is doing the routing at a lower level. I've gone through some of their open issues on Cilium and nothing jumps out at us. There are a few about UDP and packets not making it to the correct destination though. What you're seeing does seem similar to this issue here.

Another concern is the overhead of doing that resolve each time we wish to write data. I'll need to run some benchmarks to have a look and see if it does indeed come with a penalty.

[guidao]

The example in #263 is a bit incorrect, in fact in our environment we are using a dns name, not a literal IP address. In #263 I was trying to determine if we could use the returned error to re-establish a connection, but maybe there is some case I don't know about. So I think bessb's pr might work better.

On the performance issue, it might be possible to have the user configure the option to resolve every time or not to resolve or to resolve periodically.

[carlosroman]

The example in https://github.com/DataDog/datadog-go/issues/263 is a bit incorrect, in fact in our environment we are using a dns name, not a literal IP address

@guidao yeah, this will probably fix your issue if using a DNS name (assuming you're not using the CGO DNS resolver as that has some quirks). I'll look into seeing how we can release this with a nice config flag for people to use + benchmark it to check if we even need to use a flag.

[bessb]

I've had a look and very curious if this will resolve #263 or not. Just reading the docs on net.ResolveUDPAddr and if you give the function a "literal IP address" (such as 127.0.0.1) then it will return that address back.

Here's a simple example where I'm using the current client code:

package main

import (
	"github.com/DataDog/datadog-go/v5/statsd"
	"log"
	"time"
)

func main() {
	statsd, err := statsd.New("127.0.0.1:8125")
	if err != nil {
		log.Fatal(err)
	}
	log.Println("client initialized")
	defer statsd.Close()
	for range time.Tick(30 * time.Second) {
		statsd.Flush()
	}
}

If I use strace to print the system calls inside a Pod, here's what I end up with:

connect(3, {sa_family=AF_INET, sin_port=htons(8125), sin_addr=inet_addr("127.0.0.1")}, 16) = 0
getsockname(3, {sa_family=AF_INET, sin_port=htons(38169), sin_addr=inet_addr("100.xxx.xxx.24")}, [112->16]) = 0
getpeername(3, {sa_family=AF_INET, sin_port=htons(8125), sin_addr=inet_addr("100.xxx.xxx.42")}, [112->16]) = 0
write(2, "2022/10/17 18:31:41 client initi"..., 392022/10/17 18:31:41 client initialized) = 39
write(3, "datadog.dogstatsd.client.aggrega"..., 168) = 168
write(3, "datadog.dogstatsd.client.aggrega"..., 1180) = 1180
write(3, "datadog.dogstatsd.client.metrics"..., 332) = 332
write(3, "datadog.dogstatsd.client.packets"..., 165) = 165
write(3, "datadog.dogstatsd.client.bytes_d"..., 163) = 163
write(3, "datadog.dogstatsd.client.events:"..., 156) = 156
write(3, "datadog.dogstatsd.client.packets"..., 331) = 331
write(3, "datadog.dogstatsd.client.packets"..., 171) = 171
write(3, "datadog.dogstatsd.client.metrics"..., 1274) = 1274
write(3, "datadog.dogstatsd.client.bytes_d"..., 170) = 170
write(3, "datadog.dogstatsd.client.packets"..., 172) = 172
write(3, "datadog.dogstatsd.client.service"..., 164) = 164

From what I understand, Cilium is hooking into the connect() syscall to translate the 127.0.0.1 local IP into the 100.xxx.xxx.42 Pod IP based on the Pod exposing port 8125. All future writes to the socket maintain this resolved IP address, since Cilium does not hook into the socket write() calls.

If I use the changes from the PR, this is what the system calls look like:

getsockname(3, {sa_family=AF_INET6, sin6_port=htons(41417), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::", &sin6_addr), sin6_scope_id=0}, [112->28]) = 0
write(2, "2022/10/17 18:47:30 client initi"..., 392022/10/17 18:47:30 client initialized) = 39
sendto(3, "datadog.dogstatsd.client.aggrega"..., 168, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 168
sendto(3, "datadog.dogstatsd.client.aggrega"..., 1180, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 1180
sendto(3, "datadog.dogstatsd.client.metrics"..., 332, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 332
sendto(3, "datadog.dogstatsd.client.packets"..., 165, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 165
sendto(3, "datadog.dogstatsd.client.bytes_d"..., 163, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 163
sendto(3, "datadog.dogstatsd.client.events:"..., 156, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 156
sendto(3, "datadog.dogstatsd.client.packets"..., 332, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 332
sendto(3, "datadog.dogstatsd.client.packets"..., 171, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 171
sendto(3, "datadog.dogstatsd.client.metrics"..., 1277, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 1277
sendto(3, "datadog.dogstatsd.client.bytes_d"..., 170, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 170
sendto(3, "datadog.dogstatsd.client.packets"..., 172, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 172
sendto(3, "datadog.dogstatsd.client.service"..., 164, 0, {sa_family=AF_INET6, sin6_port=htons(8125), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, 28) = 164

In the second case, the original IP address is set on all the sendto() calls sending metrics.

I also tested using a hostname (e.g. datadog-agent.datadog-agents within K8s), and each call to send metrics seems to include a DNS lookup.

connect(8, {sa_family=AF_INET, sin_port=htons(53), sin_addr=inet_addr("172.xx.xx.10")}, 16) = 0
getsockname(8, {sa_family=AF_INET, sin_port=htons(50813), sin_addr=inet_addr("100.xx.xx.45")}, [112->16]) = 0
getpeername(8, {sa_family=AF_INET, sin_port=htons(53), sin_addr=inet_addr("100.xx.xx.54")}, [112->16]) = 0
write(8, "\246\245\1\0\0\1\0\0\0\0\0\1\rdatadog-agent\16datad"..., 75) = 75

So, I agree this may incur some additional overhead, but should account for Pod or DNS changes happening after the client is initialized.

[carlosroman]

@bessb Will be checking out if this does help with your issue. I don't believe it will because net.ResolveUDPAddr will just return us 127.0.0.1 rather than a pod IP. As 127.0.0.1 is a literal IP then no DNS request is made. Cilium is doing the routing at a lower level but I wonder if there is something we could do to detect we are now sending data to an IP that no longer exists.

[carlosroman]

@bessb Sorry it has been awhile. I've just verified your PR and it does indeed help when running Cilium with a CiliumLocalRedirectPolicy pointing to 127.0.0.1. It still need to benchmark this to see what type of performance hit it might cause. If the performance change is significant we can work this into a config option or look at other ways to reduce the look ups (every X send create new connection).

[bessb]

@carlosroman Thank you for your continued updates.

[carlosroman]

@bessb Got around to performance testing this and looks like both memory and CPU increased significantly (almost double) with this change. I'm gonna double check everything again and get someone to verify what I'm seeing. If it does indeed cause this performance change then I'll look at refactoring your PR so that it becomes a config option. That way it has to be enabled by a user with the knowledge that it will cause a performance issue.