-
Notifications
You must be signed in to change notification settings - Fork 104
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. few minor changes. I wonder if we could refactor this util and mt-replicator to give it output plugins (kafka and tsdb) but we don't have to spend time on that now.
return | ||
} | ||
|
||
func (r *MetricsReplicator) Start() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would reorder the code. after the constructor put Start, then Stop, then consume and flush (make the latter two non-exported), the order seems pretty arbitrary now, I think it's generally a good idea to put the main calleable logic first and helper functions below them
|
||
var ( | ||
producerBatchSize = flag.Int("batch-size", 10000, "number of metrics to send in each batch.") | ||
destinationUrl = flag.String("destination-url", "http://localhost/metrics", "tsdb-gw address to send metrics to") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when i run gometalinter I see:
metrics.go:23:2:warning: var destinationUrl should be destinationURL (golint)
metrics.go:27:2:warning: var clientId should be clientID (golint)
see https://github.com/golang/go/wiki/CodeReviewComments#initialisms
When replicating between clusters in different networks it is not always easy to push from one kafka to another due to Kafka requiring that publishers send to the configured "advertised_addr" which probably isnt internet facing. This new tool consumes from a local kafka cluster and publishes data to a remote tsdb-gw server.
8509fd1
to
151c262
Compare
@Dieterbe can we get this merged now |
When replicating between clusters in different networks it is not
always easy to push from one kafka to another due to Kafka
requiring that publishers send to the configured "advertised_addr"
which probably isnt internet facing. This new tool consumes from
a local kafka cluster and publishes data to a remote tsdb-gw server.