DataDog/datadog-go

Unix socket path is not extracted correctly

Closed this issue · 6 comments

According to the documentation, the way to pass a UDS path to statsd.New is by prefixing the string with unix://. However, an extra slash is passed down because of

w, err = newUDSWriter(addr[len(UnixAddressPrefix)-1:])
, e.g unix:///var/run/datadog/dsd.socket -> //var/run/datadog/dsd.socket

arbll commented

Hey @avlazarov, indeed that does not look right.

It was part of the original implementation so it's probably not resulting in a bug with most setups:

w, err := newUdsWriter(addr[len(UnixAddressPrefix)-1:])

IIRC any number of consecutive / get collapsed into one on *nix, did this lead to an issue in your case?

Feel free to open a PR to fix it. I can also do it if you prefer, it should be a non-breaking one-line change.

Arthur

arbll commented

@avlazarov bump on this, was it leading to any issues ?

@arbll Not certain, I could not get the client work properly with a Unix socket at all. No issues with the Python or Ruby versions of the client. E.g this does not work

func main() {
  c, err := statsd.New("unix:///var/run/datadog/dsd.socket")
  if err != nil {
      log.Fatal(err)
  }

  err = c.Count("tests.metric", 1, nil, 1)
  ....
}

But this is fine

require 'datadog/statsd'

statsd = Datadog::Statsd.new socket_path: "/var/run/datadog/dsd.socket"
statsd.increment 'test.metric'

The Go version is fine when sending over UDP instead.

arbll commented

@avlazarov Interesting. What does your env look like (OS, linux distro, containerized, ...) ? Trying to see if I can reproduce.

I any case I'll probably write a PR this week

@arbll The setup behind #104 (comment) is a vanilla AWS EC2 Ubuntu server 18.04, running directly on the instance without docker or similar.

arbll commented

@avlazarov The extraction logic should now be fixed! It will get included in the next release. |
If you still have issues setting up UDS after the new version is out, feel free to send an email at support@datadoghq.com referencing this issue.