InfluxDbBaseSender not being thread safe

[dimas]

Hello.
We spent a lot of time investigating some issue so I would like to raise a feature request and explain the problem we ran into. Up to you guys if you decide it is "by design" and decide not to do anything :)

We did not realise that InfluxDbBaseSender class and its subclasses (like InfluxDbUdpSender) cannot be reused by multiple reporters. Given the reporter code looks like this

            influxDb.flush();
...
            for (...) {
                influxDb.appendPoints(...);
            }
...
            if (influxDb.hasSeriesData()) {
                influxDb.writeData();
            }

( see https://github.com/iZettle/dropwizard-metrics-influxdb/blob/master/metrics-influxdb/src/main/java/com/izettle/metrics/influxdb/InfluxDbReporter.java#L223 )

and it runs every N seconds, so two of such reporters sharing the same instance of InfluxDbTcpSender may flush() each other's data.
We ran into the issue because in addition to JVM metrics, we are collecting Hystrix metrics too so we implemented subclass of com.netflix.servo.publish.BaseMetricObserver in a way very similar to InfluxDbReporter.java above.

The InfluxDbTcpSender was declared as a Spring bean so Spring created single instance and used it twice. Nothing warned us it is a bad idea. I understand that an easy workaround is to create two instances of sender instead and this is what we are going to do. However, maintaining two TCP connections where a single one is enough is not perfect too.

It kinda feels that InfluxDbBaseSender does two completely unrelated jobs - it is collecting (accumulating) metrics for sending and does the actual sending. It probably would be much safer (and thread safer) if InfluxDbTcpSender did not reset the sender but accumulated all its data into some data-only object that it owns (and does not share with other classes) and then passed that object to the sender for the actual transmission.

Anyways, just a thought.