8000 Added support for 'min' and 'max' on both Histogram and Summary. by scottweaver · Pull Request #6510 · zio/zio · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Added support for 'min' and 'max' on both Histogram and Summary. #6510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Apr 1, 2022
Merged

Added support for 'min' and 'max' on both Histogram and Summary. #6510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8a5d096
Added support for 'min' and 'max' on both Histogram and Summary.
scottweaver Mar 31, 2022
cf33a3d
Fixed compilation issue for ConcurrentMetricHooksPlatformSpecific due…
scottweaver Mar 31, 2022
c05dff0
Fix JS compilation issues due to missing parens.
scottweaver Apr 1, 2022
b5d1382
Implemeted recommended performance improvements.
scottweaver Apr 1, 2022
53caecf
Merge pull request #1 from zio/series/2.x
scottweaver Apr 1, 2022
3ec097d
Merge branch 'metrics/min+max_state' of https://github.com/scottweave…
scottweaver Apr 1, 2022
7761c7f
Worked with Adam Fraser to come up with a safer and more performant a…
scottweaver Apr 1, 2022
28f43aa
Completed documentation of 'AtomicDouble'.
scottweaver Apr 1, 2022
5e6fdac
Formatting
scottweaver Apr 1, 2022
f86c170
Fix native compile issue.
scottweaver Apr 1, 2022
4bcb802
Attempt to address test flakiness.
scottweaver Apr 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions core-tests/shared/src/test/scala/zio/metrics/MetricSpec.scala
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ object MetricSpec extends ZIOBaseSpec {
_ <- ZIO.succeed(1.0) @@ h
_ <- ZIO.succeed(3.0) @@ h
state <- h.value
} yield assertTrue(state.count == 2L, state.sum == 4.0)
} yield assertTrue(state.count == 2L, state.sum == 4.0, state.min == 1.0, state.max == 3.0)
},
test("direct observe") {
val h = Metric.histogram("h2", Histogram.Boundaries.linear(0, 1.0, 10)).tagged(labels1)
Expand All @@ -167,7 +167,7 @@ object MetricSpec extends ZIOBaseSpec {
_ <- h.update(1.0)
_ <- h.update(3.0)
state <- h.value
} yield assertTrue(state.count == 2L, state.sum == 4.0)
} yield assertTrue(state.count == 2L, state.sum == 4.0, state.min == 1.0, state.max == 3.0)
},
test("observeDurations") {
val h =
Expand All @@ -187,9 +187,13 @@ object MetricSpec extends ZIOBaseSpec {
} yield assertTrue(
state.count == 2L,
state.sum > 3.9,
state.sum <= elapsed
state.sum <= elapsed,
state.min >= 1.0,
state.min < state.max,
state.max >= 3.0,
state.max < elapsed
)
} @@ withLiveClock,
} @@ withLiveClock @@ flaky,
test("observeHistogram") {
val h = Metric
.histogram("h4", Histogram.Boundaries.linear(0, 1.0, 10))
Expand All @@ -201,7 +205,9 @@ object MetricSpec extends ZIOBaseSpec {
state <- h.value
} yield assertTrue(
state.count == 2L,
state.sum == 4.0
state.sum == 4.0,
state.min == 1.0,
state.max == 3.0
)
},
test("observeHistogramWith") {
Expand All @@ -214,7 +220,7 @@ object MetricSpec extends ZIOBaseSpec {
_ <- ZIO.succeed("x") @@ h
_ <- ZIO.succeed("xyz") @@ h
state <- h.value
} yield assertTrue(state.count == 2L, state.sum == 4.0)
} yield assertTrue(state.count == 2L, state.sum == 4.0, state.min == 1.0, state.max == 3.0)
},
test("observeHistogramWith + taggedWith") {
val boundaries = Histogram.Boundaries.linear(0, 1.0, 10)
Expand Down Expand Up @@ -245,7 +251,7 @@ object MetricSpec extends ZIOBaseSpec {
_ <- ZIO.succeed(1.0) @@ s
_ <- ZIO.succeed(3.0) @@ s
state <- s.value
} yield assertTrue(state.count == 2L, state.sum == 4.0)
} yield assertTrue(state.count == 2L, state.sum == 4.0, state.min == 1.0, state.max == 3.0)
},
test("direct observe") {
val s = Metric
Expand All @@ -256,7 +262,7 @@ object MetricSpec extends ZIOBaseSpec {
_ <- s.update(1.0)
_ <- s.update(3.0)
state <- s.value
} yield assertTrue(state.count == 2L, state.sum == 4.0)
} yield assertTrue(state.count == 2L, state.sum == 4.0, state.min == 1.0, state.max == 3.0)
},
test("observeSummary") {
val s = Metric
Expand All @@ -267,7 +273,7 @@ object MetricSpec extends ZIOBaseSpec {
_ <- ZIO.succeed(1.0) @@ s
_ <- ZIO.succeed(3.0) @@ s
state <- s.value
} yield assertTrue(state.count == 2L, state.sum == 4.0)
} yield assertTrue(state.count == 2L, state.sum == 4.0, state.min == 1.0, state.max == 3.0)
},
test("observeSummaryWith") {
val s = Metric
Expand All @@ -279,7 +285,7 @@ object MetricSpec extends ZIOBaseSpec {
_ <- ZIO.succeed("x") @@ s
_ <- ZIO.succeed("xyz") @@ s
state <- s.value
} yield assertTrue(state.count == 2L, state.sum == 4.0)
} yield assertTrue(state.count == 2L, state.sum == 4.0, state.min == 1.0, state.max == 3.0)
},
test("observeSummaryWith + taggedWith") {
val s0 = Metric
Expand Down
21 changes: 19 additions & 2 deletions core/js/src/main/scala/zio/internal/metrics/ConcurrentMetricHooksPlatformSpecific.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
val boundaries = Array.ofDim[Double](bounds.length)
var count = 0L
var sum = 0.0
val size = bounds.length
var size = bounds.length
var min = Double.MaxValue
var max = Double.MinValue

bounds.sorted.zipWithIndex.foreach { case (n, i) => boundaries(i) = n }

Expand All @@ -59,6 +61,8 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
values(from) = values(from) + 1
count += 1
sum += value
if (value < min) min = value
if (value > max) max = value
()
}

Expand All @@ -76,7 +80,10 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
builder.result()
}

MetricHook(update, () => MetricState.Histogram(getBuckets(), count, sum))
MetricHook(
update,
() => MetricState.Histogram(getBuckets(), count, min, max, sum)
)
}

def summary(key: MetricKey.Summary): MetricHook.Summary = {
Expand All @@ -86,11 +93,17 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
var head = 0
var count = 0L
var sum = 0.0
var min = Double.MaxValue
var max = Double.MinValue

val sortedQuantiles: Chunk[Double] = quantiles.sorted(DoubleOrdering)

def getCount(): Long = count

def getMin(): Double = min

def getMax(): Double = max

def getSum(): Double = sum

// Just before the Snapshot we filter out all values older than maxAge
Expand Down Expand Up @@ -129,6 +142,8 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {

count += 1
sum += value
if (value < min) min = value
if (value > max) max = value
()
}

Expand All @@ -139,6 +154,8 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
error,
snapshot(java.time.Instant.now()),
getCount(),
getMin(),
getMax(),
getSum()
)
)
Expand Down
76 changes: 74 additions & 2 deletions core/jvm/src/main/scala/zio/internal/metrics/ConcurrentMetricHooksPlatformSpecific.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import zio.metrics._

import java.util.concurrent.atomic._
import java.util.concurrent.ConcurrentHashMap
import java.lang.{Double => JDouble}

class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
def counter(key: MetricKey.Counter): MetricHook.Counter = {
Expand All @@ -34,13 +35,37 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
MetricHook(v => ref.set(v), () => MetricState.Gauge(ref.get()))
}

private def updateMin(atomic: AtomicDouble, value: Double): Unit = {
var loop = true

while (loop) {
val current = atomic.get()
if (value < current) {
loop = !atomic.compareAndSet(current, value)
} else loop = false
}
}

private def updateMax(atomic: AtomicDouble, value: Double): Unit = {
var loop = true

while (loop) {
val current = atomic.get()
if (value > current) {
loop = !atomic.compareAndSet(current, value)
} else loop = false
}

}
def histogram(key: MetricKey.Histogram): MetricHook.Histogram = {
val bounds = key.keyType.boundaries.values
val values = new AtomicLongArray(bounds.length + 1)
val boundaries = Array.ofDim[Double](bounds.length)
val count = new LongAdder
val sum = new DoubleAdder
val size = bounds.length
val min = AtomicDouble.make(Double.MaxValue)
val max = AtomicDouble.make(Double.MinValue)

bounds.sorted.zipWithIndex.foreach { case (n, i) => boundaries(i) = n }

Expand All @@ -61,6 +86,8 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
values.getAndIncrement(from)
count.increment()
sum.add(value)
updateMin(min, value)
updateMax(max, value)
()
}

Expand All @@ -78,7 +105,10 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
builder.result()
}

MetricHook(update, () => MetricState.Histogram(getBuckets(), count.longValue(), sum.doubleValue()))
MetricHook(
update,
() => MetricState.Histogram(getBuckets(), count.longValue(), min.get(), max.get(), sum.doubleValue())
)
}

def summary(key: MetricKey.Summary): MetricHook.Summary = {
Expand All @@ -88,12 +118,20 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
val head = new AtomicInteger(0)
val count = new LongAdder
val sum = new DoubleAdder
val min = AtomicDouble.make(Double.MaxValue)
val max = AtomicDouble.make(Double.MinValue)

val sortedQuantiles: Chunk[Double] = quantiles.sorted(DoubleOrdering)

def getCount(): Long =
count.longValue

def getMin(): Double =
min.get()

def getMax(): Double =
max.get()

def getSum(): Double =
sum.doubleValue

Expand Down Expand Up @@ -130,8 +168,11 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
values.set(target, tuple)
}

val value = tuple._1
count.increment()
sum.add(tuple._1)
sum.add(value)
updateMin(min, value)
updateMax(max, value)
()
}

Expand All @@ -142,6 +183,8 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
error,
snapshot(java.time.Instant.now()),
getCount(),
getMin(),
getMax(),
getSum()
)
)
Expand Down Expand Up @@ -175,4 +218,33 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {

MetricHook(update, () => MetricState.Frequency(snapshot()))
}

/**
* Scala's `Double` implementation does not play nicely with Java's
* `AtomicReference.compareAndSwap` as `compareAndSwap` uses Java's `==`
* reference equality when it performs an equality check. This means that even
* if two Scala `Double`s have the same value, they will still fail
* `compareAndSwap` as they will most likely be two, distinct object
* references. Thus, `compareAndSwap` will fail.
*
* This `AtomicDouble` implementation is a workaround for this issue that is
* backed by an `AtomicLong` instead of an `AtomicReference` in which the
* Double's bits are stored as a Long value. This approach also reduces boxing
* and unboxing overhead that can be incurred with `AtomicReference`.
*/
private final class AtomicDouble private (private val ref: AtomicLong) {
def get(): Double =
JDouble.longBitsToDouble(ref.get())

def compareAndSet(expected: Double, newValue: Double): Boolean =
ref.compareAndSet(JDouble.doubleToLongBits(expected), JDouble.doubleToLongBits(newValue))

}

private object AtomicDouble {

def make(value: Double): AtomicDouble =
new AtomicDouble(new AtomicLong(JDouble.doubleToLongBits(value)))
}

}
19 changes: 18 additions & 1 deletion core/native/src/main/scala/zio/internal/metrics/ConcurrentMetricHooksPlatformSpecific.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
var count = 0L
var sum = 0.0
val size = bounds.length
var min = Double.MaxValue
var max = Double.MinValue

bounds.sorted.zipWithIndex.foreach { case (n, i) => boundaries(i) = n }

Expand All @@ -59,6 +61,8 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
values(from) = values(from) + 1
count += 1
sum += value
if (value < min) min = value
if (value > max) max = value
()
}

Expand All @@ -76,7 +80,10 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
builder.result()
}

MetricHook(update, () => MetricState.Histogram(getBuckets(), count, sum))
MetricHook(
update,
() => MetricState.Histogram(getBuckets(), count, min, max, sum)
)
}

def summary(key: MetricKey.Summary): MetricHook.Summary = {
Expand All @@ -86,11 +93,17 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
var head = 0
var count = 0L
var sum = 0.0
var min = Double.MaxValue
var max = Double.MinValue

val sortedQuantiles: Chunk[Double] = quantiles.sorted(DoubleOrdering)

def getCount(): Long = count

def getMin(): Double = min

def getMax(): Double = max

def getSum(): Double = sum

// Just before the Snapshot we filter out all values older than maxAge
Expand Down Expand Up @@ -129,6 +142,8 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {

count += 1
sum += value
if (value < min) min = value
if (value > max) max = value
()
}

Expand All @@ -139,6 +154,8 @@ class ConcurrentMetricHooksPlatformSpecific extends ConcurrentMetricHooks {
error,
snapshot(java.time.Instant.now()),
getCount(),
getMin(),
getMax(),
getSum()
)
)
Expand Down
4 changes: 4 additions & 0 deletions core/shared/src/main/scala/zio/metrics/MetricState.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,17 @@ object MetricState {
final case class Histogram(
buckets: Chunk[(Double, Long)],
count: Long,
min: Double,
max: Double,
sum: Double
) extends MetricState[MetricKeyType.Histogram]

final case class Summary(
error: Double,
quantiles: Chunk[(Double, Option[Double])],
count: Long,
min: Double,
max: Double,
sum: Double
) extends MetricState[MetricKeyType.Summary]
}
0