coursera / metrics-datadog

metrics-datadog
Other
188 stars 105 forks source link

Retrying for volatile host #75

Closed goulashify closed 7 years ago

goulashify commented 7 years ago

Hi Guys n Gals!

First of all: thanks for putting work into this project, we found it supa-dupa-handy! :)

We found a little "bug" in it which caused some of the services to miss reporting, it was triggered by some DNS trouble on our side, which caused the metrics to fail, this PR is a proposal for fixing these scenarios, please see below.

Problem

By default NonBlockingStatsDClient's constructor uses staticStatsDAddressResolution which resolves the host only once at the beginning.

This is a good choice! This surely spares a lot of extra work thorough the lifecycle of a service, as a side effect, if the host is not resolvable at the time of construction it'll cause reporting to fail.

How one may avoid it?

I've added an extra retrying_lookup option to the configuration for UdpTransport, which defaults to false, in that setting everything works as before, when set to true it'll use volatileAddressResolution for resolving addresses.

volatileAddressResolution will do a lookup each time it's called (cache is provided by InetAddress), allowing the host to be resolved later, when it's possible to do so.


Let me know what you think!

Thanks! Reg, -Dan

nickdella commented 7 years ago

@danigulyas Thanks for the really thoughtful PR! I'll be sure to take a look at this before the weekend.

goulashify commented 7 years ago

Hi Nick!

Added the nits!

Is there any plan when this feature will be released?

Reg, -Dan

nickdella commented 7 years ago

@danigulyas I should be able to get a new build pushed out tomorrow. Thanks again!

goulashify commented 7 years ago

Cool, thanks!

nickdella commented 7 years ago

I just pushed version 1.1.11 to maven central. Should be available within an hour or so

goulashify commented 7 years ago

Cool! Thanks Nick!