search

alexcesaro / statsd

An efficient Statsd Go client.
MIT License
301 stars 79 forks source link

Don't mute/discard connection when "quick listener check" fails #6

Open zstyblik opened 8 years ago

zstyblik commented 8 years ago

Hello,

I'm wondering whether it would be possible to try to either connect(un-mute) or from time to time.

Currently, when:

Failed to init StatsD: write udp [::1]:51628->[::1]:8125: write: connection refusedSlept: 0

Then no attempt is made to connect again. I don't want to fail application just because connection cannot be established, although may be I should. Also, it seems to me a bit cumbersome to introduce some check-whether-StatsD-is-initialized logic.

Would it be possible to try to connect from time to time eg. on flush() or just from time to time and un-mute?

If I kill listener, then connection is re-established once listener comes back. However, if listener isn't there when StatsD client is initialized, we never try again.

Thanks, Z.

zstyblik commented 8 years ago

The "problem" is in https://github.com/alexcesaro/statsd/blob/master/conn.go#L50. But after reading the code, the only thing I came up is to remove this code block. :| Because then it works as ... I need it to. :)

It seems https://github.com/gdiazlo/statsd/commit/d12d68c9bd9adeab98f3f002402774e862640e13 did the same.

wbbradley commented 7 years ago

@zstyblik's fork seems good. @alexcesaro is there a great reason to keep that check in there? Would you be willing to accept a PR where that check is optional?

joeycumines commented 2 years ago

Very old issue, but I've been doing some work on my fork.

As I understand it, muting the client returned by New on error is intentional and desirable behavior, that makes the UDP and TCP/streaming implementations consistent. For the UDP case, it is beneficial to be able to fail fast (like TCP), if possible, to detect potential misconfigurations.

Ignoring the initial check is still a valid use case, though, so I've made two changes:

  1. Documented that New always returns a non-nil client, but it (and all clients cloned from it) will be muted, if it was returned with an error
  2. Added an option that allows the initial UDP conn checking to be disabled (does what y'all seem to want, without breaking the existing behavior)