go-kit
metrics/dogstatsd: always reset all metrics before writing them when calling WriteTo
Description
This PR fixes an issue that occurred when the Dogstatsd client could not reach the Datadog agent to send metrics. It would fail to send counters, return and never reset timings and histograms, resulting in an ever-increasing memory consumption while the client cannot reach the Datadog agent.
If local state fails to get sent to Datadog and you reset it anyway, doesn't that lose information and invalidate your metrics?
It does, but it's coherent with the method's comment I suppose: "WriteTo abides best-effort semantics, so observations are lost if there is a problem with the write.". The current implementation already loses some information if you have multiple counters for example but you fail to send the first one, no?
We could chose to buffer these metrics while the connection is down (i.e.: only resetting when all writes are successful) instead but then we expose ourselves to the ever-growing memory issue if it lasts for too long.
Hello, sorry for bumping this again, but we would need this fix on our side, any chance this gets merged? Or do you see another implementation for fixing this issue?
@peterbourgon can you please take a look at this? It, as a dependency, affects an app in our production env for two months and a downgrade is not an option.
I'm happy to merge with a test that fails on current master and passes on the branch.
Hello, thanks for your reply.
I added a test that fails without the fix as you asked. Let me know if it works for you.
If I understand correctly, to notify @peterbourgon about comments, need to mention him
Hello @peterbourgon, did you have time to check if the tests added match what you were asking for?
