Fix synchronization issue with stats

samstarling / finagle-prometheus

Provides a bridge between Finagle and Prometheus metrics

https://samstarling.co.uk/projects/finagle-prometheus/

MIT License

30 stars 18 forks source link

Fix synchronization issue with stats #26

Closed coduinix closed 6 years ago

coduinix commented 6 years ago

Synchronization should be done around the getOrElseUpdate. Otherwise we can still have the side effecting operation (in newSummary), twice for the same metric.

samstarling commented 6 years ago

Hey @coduinix – thanks for the contribution, and sorry that this slipped through. It's been tricky to test this reliably, so thanks for your patience. Looks good! :+1:

coduinix commented 6 years ago

Indeed tricky to test. I tried to create a more reliable test, but did not succeed yet.

samstarling commented 6 years ago

@coduinix I just released v0.0.9 with this change – thanks again. It's now available on bintray, and is in the process of syncing to Maven Central.

coduinix commented 6 years ago

@samstarling That was quick... Thanks!