overflow (#19)

motivation: highlight the need to address overflow in metric objects that are integer based

changes:
* add section in the readme about addressing woverflow
* fix timer's unit conversion to address potential overflow
This commit is contained in:
tomer doron 2019-04-22 09:47:46 -07:00 committed by GitHub
parent d9f63df00c
commit 6db51d5f32
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 80 additions and 7 deletions

View File

@ -154,7 +154,27 @@ public protocol RecorderHandler: AnyObject {
}
```
Here is a full example of an in-memory implementation:
#### Dealing with Overflows
Implementaton of metric objects that deal with integers, like `Counter` and `Timer` should be careful with overflow. The expected behavior is to cap at `.max`, and never crash the program due to overflow . For example:
```swift
class ExampleCounter: CounterHandler {
var value: Int64 = 0
func increment(by amount: Int64) {
let result = self.value.addingReportingOverflow(amount)
if result.overflow {
self.value = Int64.max
} else {
self.value = result.partialValue
}
}
}
```
#### Full example
Here is a full, but contrived, example of an in-memory implementation:
```swift
class SimpleMetricsLibrary: MetricsFactory {

View File

@ -259,7 +259,12 @@ public class Timer {
/// - value: Duration to record.
@inlinable
public func recordMicroseconds<DataType: BinaryInteger>(_ duration: DataType) {
self.recordNanoseconds(Int64(duration * 1000))
let result = Int64(duration).multipliedReportingOverflow(by: 1000)
if result.overflow {
self.recordNanoseconds(Int64.max)
} else {
self.recordNanoseconds(result.partialValue)
}
}
/// Record a duration in microseconds.
@ -268,7 +273,7 @@ public class Timer {
/// - value: Duration to record.
@inlinable
public func recordMicroseconds<DataType: BinaryFloatingPoint>(_ duration: DataType) {
self.recordNanoseconds(Int64(duration * 1000))
self.recordNanoseconds(Double(duration * 1000) < Double(Int64.max) ? Int64(duration * 1000) : Int64.max)
}
/// Record a duration in milliseconds.
@ -277,7 +282,12 @@ public class Timer {
/// - value: Duration to record.
@inlinable
public func recordMilliseconds<DataType: BinaryInteger>(_ duration: DataType) {
self.recordNanoseconds(Int64(duration * 1_000_000))
let result = Int64(duration).multipliedReportingOverflow(by: 1_000_000)
if result.overflow {
self.recordNanoseconds(Int64.max)
} else {
self.recordNanoseconds(result.partialValue)
}
}
/// Record a duration in milliseconds.
@ -286,7 +296,7 @@ public class Timer {
/// - value: Duration to record.
@inlinable
public func recordMilliseconds<DataType: BinaryFloatingPoint>(_ duration: DataType) {
self.recordNanoseconds(Int64(duration * 1_000_000))
self.recordNanoseconds(Double(duration * 1_000_000) < Double(Int64.max) ? Int64(duration * 1_000_000) : Int64.max)
}
/// Record a duration in seconds.
@ -295,7 +305,12 @@ public class Timer {
/// - value: Duration to record.
@inlinable
public func recordSeconds<DataType: BinaryInteger>(_ duration: DataType) {
self.recordNanoseconds(Int64(duration * 1_000_000_000))
let result = Int64(duration).multipliedReportingOverflow(by: 1_000_000_000)
if result.overflow {
self.recordNanoseconds(Int64.max)
} else {
self.recordNanoseconds(result.partialValue)
}
}
/// Record a duration in seconds.
@ -304,7 +319,7 @@ public class Timer {
/// - value: Duration to record.
@inlinable
public func recordSeconds<DataType: BinaryFloatingPoint>(_ duration: DataType) {
self.recordNanoseconds(Int64(duration * 1_000_000_000))
self.recordNanoseconds(Double(duration * 1_000_000_000) < Double(Int64.max) ? Int64(duration * 1_000_000_000) : Int64.max)
}
}

View File

@ -34,6 +34,7 @@ extension MetricsTests {
("testTimers", testTimers),
("testTimerBlock", testTimerBlock),
("testTimerVariants", testTimerVariants),
("testTimerOverflow", testTimerOverflow),
("testGauge", testGauge),
("testGaugeBlock", testGaugeBlock),
("testMUX", testMUX),

View File

@ -180,6 +180,43 @@ class MetricsTests: XCTestCase {
XCTAssertEqual(testTimer.values[3].1, sec * 1_000_000_000, "expected value to match")
}
func testTimerOverflow() throws {
// bootstrap with our test metrics
let metrics = TestMetrics()
MetricsSystem.bootstrapInternal(metrics)
// run the test
let timer = Timer(label: "test-timer")
let testTimer = timer.handler as! TestTimer
// nano (integer)
timer.recordNanoseconds(Int64.max)
XCTAssertEqual(testTimer.values.count, 1, "expected number of entries to match")
XCTAssertEqual(testTimer.values[0].1, Int64.max, "expected value to match")
// micro (integer)
timer.recordMicroseconds(Int64.max)
XCTAssertEqual(testTimer.values.count, 2, "expected number of entries to match")
XCTAssertEqual(testTimer.values[1].1, Int64.max, "expected value to match")
// micro (double)
timer.recordMicroseconds(Double(Int64.max) + 1)
XCTAssertEqual(testTimer.values.count, 3, "expected number of entries to match")
XCTAssertEqual(testTimer.values[1].1, Int64.max, "expected value to match")
// milli (integer)
timer.recordMilliseconds(Int64.max)
XCTAssertEqual(testTimer.values.count, 4, "expected number of entries to match")
XCTAssertEqual(testTimer.values[2].1, Int64.max, "expected value to match")
// milli (double)
timer.recordMilliseconds(Double(Int64.max) + 1)
XCTAssertEqual(testTimer.values.count, 5, "expected number of entries to match")
XCTAssertEqual(testTimer.values[2].1, Int64.max, "expected value to match")
// seconds (integer)
timer.recordSeconds(Int64.max)
XCTAssertEqual(testTimer.values.count, 6, "expected number of entries to match")
XCTAssertEqual(testTimer.values[3].1, Int64.max, "expected value to match")
// seconds (double)
timer.recordSeconds(Double(Int64.max) * 1)
XCTAssertEqual(testTimer.values.count, 7, "expected number of entries to match")
XCTAssertEqual(testTimer.values[3].1, Int64.max, "expected value to match")
}
func testGauge() throws {
// bootstrap with our test metrics
let metrics = TestMetrics()