core: catch any exception being thrown when recording values on histograms #335

Merged
merged 1 commit into from Apr 22, 2016

Projects

None yet

3 participants

@ivantopo
Contributor
ivantopo commented Apr 1, 2016

fixes #232, fixes #332

@ivantopo ivantopo added the In Progress label Apr 1, 2016
@ivantopo
Contributor
ivantopo commented Apr 1, 2016

maybe we can record the max allowed value in a histogram when we hit the out of range exception? I think it might be better to have a huge value than not to have the value at all. What do @dpsoft and @mladens say?

@dpsoft dpsoft commented on the diff Apr 1, 2016
...rc/main/scala/kamon/metric/instrument/Histogram.scala
@@ -135,6 +136,10 @@ object Histogram {
}
}
+object HdrHistogram {
+ private val log = LoggerFactory.getLogger(classOf[HdrHistogram])
@dpsoft
dpsoft Apr 1, 2016 Contributor

I think that we can use our LazyLogger instead of LoggerFactory ;):

private val log = kamon.util.logger.LazyLogger(classOf[HdrHistogram])

@dpsoft
Contributor
dpsoft commented Apr 1, 2016

Maybe could be a good idea if before record the max allowed value, we can show a message in the log with this behavior.

@dpsoft dpsoft merged commit aca5fa2 into kamon-io:master Apr 22, 2016
@dpsoft dpsoft removed the In Progress label Apr 22, 2016
@eiennohito eiennohito commented on the diff May 11, 2016
...rc/main/scala/kamon/metric/instrument/Histogram.scala
- def record(value: Long, count: Long): Unit = recordValueWithCount(value, count)
+ private def tryRecord(value: Long, count: Long): Unit = {
+ try {
+ recordValueWithCount(value, count)
+ } catch {
+ case anyException: Throwable
+ log.warn("Failed to store value {} in HdrHistogram, please review your range configuration.")
@eiennohito
eiennohito May 11, 2016 edited

value is not passed to the logger

@dpsoft
Contributor
dpsoft commented May 15, 2016

@eiennohito thanks for reporting this. I've fixed this with 243658c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment