address feedback (#13)

motivation: address feedback from community, prepare for moving api to official repo

changes:
* add reset method to counter
* do not force the user facing metric object to implement the mteric handler protocol. this was done for convinience and confuses matters
* adjust tests
* fix a few typos
This commit is contained in:
tomer doron 2019-04-05 07:28:52 -07:00 committed by GitHub
parent 20b998220e
commit 187653d466
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 59 additions and 19 deletions

View File

@ -62,7 +62,7 @@ How would you use `counter`, `recorder`, `gauge` and `timer` in you application
## Detailed design
### Implementing a metrics backend (eg prometheus client library)
### Implementing a metrics backend (e.g. prometheus client library)
As seen above, the constructors `Counter`, `Timer`, `Recorder` and `Gauge` provides a metric object. This raises the question of what metrics backend I will actually get when calling these constructors? The answer is that it's configurable _per application_. The application sets up the metrics backend it wishes the whole application to use. Libraries should never change the metrics implementation as that is something owned by the application. Configuring the metrics backend is straightforward:

View File

@ -15,10 +15,11 @@
/// This is the Counter protocol a metrics library implements. It must have reference semantics
public protocol CounterHandler: AnyObject {
func increment<DataType: BinaryInteger>(_ value: DataType)
func reset()
}
// This is the user facing Counter API. Its behavior depends on the `CounterHandler` implementation
public class Counter: CounterHandler {
public class Counter {
@usableFromInline
var handler: CounterHandler
public let label: String
@ -41,10 +42,15 @@ public class Counter: CounterHandler {
public func increment() {
self.increment(1)
}
@inlinable
public func reset() {
self.handler.reset()
}
}
public extension Counter {
public convenience init(label: String, dimensions: [(String, String)] = []) {
convenience init(label: String, dimensions: [(String, String)] = []) {
let handler = MetricsSystem.factory.makeCounter(label: label, dimensions: dimensions)
self.init(label: label, dimensions: dimensions, handler: handler)
}
@ -57,7 +63,7 @@ public protocol RecorderHandler: AnyObject {
}
// This is the user facing Recorder API. Its behavior depends on the `RecorderHandler` implementation
public class Recorder: RecorderHandler {
public class Recorder {
@usableFromInline
var handler: RecorderHandler
public let label: String
@ -85,7 +91,7 @@ public class Recorder: RecorderHandler {
}
public extension Recorder {
public convenience init(label: String, dimensions: [(String, String)] = [], aggregate: Bool = true) {
convenience init(label: String, dimensions: [(String, String)] = [], aggregate: Bool = true) {
let handler = MetricsSystem.factory.makeRecorder(label: label, dimensions: dimensions, aggregate: aggregate)
self.init(label: label, dimensions: dimensions, aggregate: aggregate, handler: handler)
}
@ -103,8 +109,8 @@ public protocol TimerHandler: AnyObject {
func recordNanoseconds(_ duration: Int64)
}
// This is the user facing Timer API. Its behavior depends on the `RecorderHandler` implementation
public class Timer: TimerHandler {
// This is the user facing Timer API. Its behavior depends on the `TimerHandler` implementation
public class Timer {
@usableFromInline
var handler: TimerHandler
public let label: String
@ -155,7 +161,7 @@ public class Timer: TimerHandler {
}
public extension Timer {
public convenience init(label: String, dimensions: [(String, String)] = []) {
convenience init(label: String, dimensions: [(String, String)] = []) {
let handler = MetricsSystem.factory.makeTimer(label: label, dimensions: dimensions)
self.init(label: label, dimensions: dimensions, handler: handler)
}
@ -222,6 +228,10 @@ public final class MultiplexMetricsHandler: MetricsFactory {
func increment<DataType: BinaryInteger>(_ value: DataType) {
self.counters.forEach { $0.increment(value) }
}
func reset() {
self.counters.forEach { $0.reset() }
}
}
private class MuxRecorder: RecorderHandler {
@ -269,6 +279,7 @@ public final class NOOPMetricsHandler: MetricsFactory, CounterHandler, RecorderH
}
public func increment<DataType: BinaryInteger>(_: DataType) {}
public func reset() {}
public func record<DataType: BinaryInteger>(_: DataType) {}
public func record<DataType: BinaryFloatingPoint>(_: DataType) {}
public func recordNanoseconds(_: Int64) {}

View File

@ -115,6 +115,12 @@ class ExampleCounter: CounterHandler, CustomStringConvertible {
}
}
func reset() {
self.lock.withLock {
self.value = 0
}
}
var description: String {
return "counter [label: \(self.label) dimensions:\(self.dimensions) values:\(self.value)]"
}
@ -137,7 +143,7 @@ class ExampleRecorder: RecorderHandler, CustomStringConvertible {
}
func record<DataType: BinaryFloatingPoint>(_ value: DataType) {
// this may loose percision, but good enough as an example
// this may loose precision, but good enough as an example
let v = Double(value)
// TODO: sliding window
lock.withLock {
@ -241,7 +247,7 @@ class ExampleGauge: RecorderHandler, CustomStringConvertible {
}
func record<DataType: BinaryFloatingPoint>(_ value: DataType) {
// this may loose percision but good enough as an example
// this may loose precision but good enough as an example
self.lock.withLock { _value = Double(value) }
}

View File

@ -43,6 +43,12 @@ class SimpleMetricsLibrary: MetricsFactory {
self.value += Int64(value)
}
}
func reset() {
self.lock.withLock {
self.value = 0
}
}
}
private class ExampleRecorder: RecorderHandler {
@ -55,15 +61,15 @@ class SimpleMetricsLibrary: MetricsFactory {
}
func record<DataType: BinaryFloatingPoint>(_ value: DataType) {
// this may loose percision, but good enough as an example
// this may loose precision, but good enough as an example
let v = Double(value)
// TODO: sliding window
lock.withLock {
values.append((Date().nanoSince1970, v))
self._count += 1
self._sum += v
if 0 == self._min || v < self._min { self._min = v }
if 0 == self._max || v > self._max { self._max = v }
self._min = Swift.min(self._min, v)
self._max = Swift.max(self._max, v)
}
}
@ -98,7 +104,7 @@ class SimpleMetricsLibrary: MetricsFactory {
}
func record<DataType: BinaryFloatingPoint>(_ value: DataType) {
// this may loose percision but good enough as an example
// this may loose precision but good enough as an example
self.lock.withLock { _value = Double(value) }
}
}

View File

@ -5,7 +5,7 @@
// Convenience for measuring duration of a closure
public extension Timer {
@inlinable
public static func measure<T>(label: String, dimensions: [(String, String)] = [], body: @escaping () throws -> T) rethrows -> T {
static func measure<T>(label: String, dimensions: [(String, String)] = [], body: @escaping () throws -> T) rethrows -> T {
let timer = Timer(label: label, dimensions: dimensions)
let start = Date()
defer {
@ -18,12 +18,12 @@ public extension Timer {
// Convenience for using Foundation and Dispatch
public extension Timer {
@inlinable
public func record(_ duration: TimeInterval) {
func record(_ duration: TimeInterval) {
self.recordSeconds(duration)
}
@inlinable
public func record(_ duration: DispatchTimeInterval) {
func record(_ duration: DispatchTimeInterval) {
switch duration {
case .nanoseconds(let value):
self.recordNanoseconds(Int64(value))

View File

@ -34,6 +34,8 @@ class MetricsTests: XCTestCase {
}
group.wait()
XCTAssertEqual(testCounter.values.count - 1, total, "expected number of entries to match")
testCounter.reset()
XCTAssertEqual(testCounter.values.count, 0, "expected number of entries to match")
}
func testCounterBlock() throws {
@ -47,6 +49,8 @@ class MetricsTests: XCTestCase {
let counter = metrics.counters[name] as! TestCounter
XCTAssertEqual(counter.values.count, 1, "expected number of entries to match")
XCTAssertEqual(counter.values[0].1, Int64(value), "expected value to match")
counter.reset()
XCTAssertEqual(counter.values.count, 0, "expected number of entries to match")
}
func testRecorders() throws {
@ -210,18 +214,25 @@ class MetricsTests: XCTestCase {
// run the test
let name = NSUUID().uuidString
let value = Int.random(in: Int.min ... Int.max)
Counter(label: name).increment(value)
let mux = Counter(label: name)
mux.increment(value)
factories.forEach { factory in
let counter = factory.counters.first?.1 as! TestCounter
XCTAssertEqual(counter.label, name, "expected label to match")
XCTAssertEqual(counter.values.count, 1, "expected number of entries to match")
XCTAssertEqual(counter.values[0].1, Int64(value), "expected value to match")
}
mux.reset()
factories.forEach { factory in
let counter = factory.counters.first?.1 as! TestCounter
XCTAssertEqual(counter.values.count, 0, "expected number of entries to match")
}
}
func testCustomFactory() {
class CustomHandler: CounterHandler {
func increment<DataType>(_: DataType) where DataType: BinaryInteger {}
func reset() {}
}
let counter1 = Counter(label: "foo")

View File

@ -67,6 +67,12 @@ internal class TestCounter: CounterHandler, Equatable {
print("adding \(value) to \(self.label)")
}
func reset() {
self.lock.withLock {
self.values = []
}
}
public static func == (lhs: TestCounter, rhs: TestCounter) -> Bool {
return lhs.id == rhs.id
}
@ -94,7 +100,7 @@ internal class TestRecorder: RecorderHandler, Equatable {
func record<DataType: BinaryFloatingPoint>(_ value: DataType) {
self.lock.withLock {
// this may loose percision but good enough as an example
// this may loose precision but good enough as an example
values.append((Date(), Double(value)))
}
print("recoding \(value) in \(self.label)")