Add microseconds to TimeUnit (#64)

Motivation: I have a metrics backend where I need to store timer values in microseconds. The "preferred display unit" models this exactly, but it seems to have (inadvertently?) omitted the possibility to express microseconds. Note that among the timer "report" methods there is already a microseconds option. Changes: * change TimeUnit from enum to struct with static members to support non-api breaking future evolution * add scaleFromNanoseconds to TimeUnit to make it easy to compute the values without switching over * add tests Co-authored-by: Chris Burrows <[email protected]>
apple · Mar 4, 2020 · 708b960 · 708b960
1 parent ab3f3ff
commit 708b960
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 13 deletions.
diff --git a/Sources/CoreMetrics/Metrics.swift b/Sources/CoreMetrics/Metrics.swift
@@ -172,8 +172,34 @@ public class Gauge: Recorder {
     }
 }
 
-public enum TimeUnit {
-    case nanoseconds, milliseconds, seconds, minutes, hours, days
+public struct TimeUnit: Equatable {
+    private enum Code: Equatable {
+        case nanoseconds
+        case microseconds
+        case milliseconds
+        case seconds
+        case minutes
+        case hours
+        case days
+    }
+
+    private let code: Code
+    public let scaleFromNanoseconds: UInt64
+
+    private init(code: Code, scaleFromNanoseconds: UInt64) {
+        assert(scaleFromNanoseconds > 0, "invalid scale from nanoseconds")
+
+        self.code = code
+        self.scaleFromNanoseconds = scaleFromNanoseconds
+    }
+
+    public static let nanoseconds = TimeUnit(code: .nanoseconds, scaleFromNanoseconds: 1)
+    public static let microseconds = TimeUnit(code: .microseconds, scaleFromNanoseconds: 1000)
+    public static let milliseconds = TimeUnit(code: .milliseconds, scaleFromNanoseconds: 1000 * TimeUnit.microseconds.scaleFromNanoseconds)
+    public static let seconds = TimeUnit(code: .seconds, scaleFromNanoseconds: 1000 * TimeUnit.milliseconds.scaleFromNanoseconds)
+    public static let minutes = TimeUnit(code: .minutes, scaleFromNanoseconds: 60 * TimeUnit.seconds.scaleFromNanoseconds)
+    public static let hours = TimeUnit(code: .hours, scaleFromNanoseconds: 60 * TimeUnit.minutes.scaleFromNanoseconds)
+    public static let days = TimeUnit(code: .days, scaleFromNanoseconds: 24 * TimeUnit.hours.scaleFromNanoseconds)
 }
 
 public extension Timer {

diff --git a/Tests/MetricsTests/MetricsTests+XCTest.swift b/Tests/MetricsTests/MetricsTests+XCTest.swift
@@ -29,6 +29,7 @@ extension MetricsExtensionsTests {
             ("testTimerWithTimeInterval", testTimerWithTimeInterval),
             ("testTimerWithDispatchTime", testTimerWithDispatchTime),
             ("testTimerUnits", testTimerUnits),
+            ("testPreferDisplayUnit", testPreferDisplayUnit),
         ]
     }
 }
diff --git a/Tests/MetricsTests/MetricsTests.swift b/Tests/MetricsTests/MetricsTests.swift
@@ -100,9 +100,40 @@ class MetricsExtensionsTests: XCTestCase {
 
         let testSecondsTimer = secondsTimer.handler as! TestTimer
         XCTAssertEqual(testSecondsTimer.values.count, 1, "expected number of entries to match")
-        XCTAssertEqual(testSecondsTimer.retriveValueInPreferredUnit(atIndex: 0), secondsValue, "expected value to match")
         XCTAssertEqual(metrics.timers.count, 2, "timer should have been stored")
     }
+
+    func testPreferDisplayUnit() {
+        let metrics = TestMetrics()
+        MetricsSystem.bootstrapInternal(metrics)
+
+        let value = Double.random(in: 0 ... 1000)
+        let timer = Timer(label: "test", preferredDisplayUnit: .seconds)
+        timer.recordSeconds(value)
+
+        let testTimer = timer.handler as! TestTimer
+
+        testTimer.preferDisplayUnit(.nanoseconds)
+        XCTAssertEqual(testTimer.retriveValueInPreferredUnit(atIndex: 0), value * 1000 * 1000 * 1000, accuracy: 1.0, "expected value to match")
+
+        testTimer.preferDisplayUnit(.microseconds)
+        XCTAssertEqual(testTimer.retriveValueInPreferredUnit(atIndex: 0), value * 1000 * 1000, accuracy: 0.001, "expected value to match")
+
+        testTimer.preferDisplayUnit(.milliseconds)
+        XCTAssertEqual(testTimer.retriveValueInPreferredUnit(atIndex: 0), value * 1000, accuracy: 0.000001, "expected value to match")
+
+        testTimer.preferDisplayUnit(.seconds)
+        XCTAssertEqual(testTimer.retriveValueInPreferredUnit(atIndex: 0), value, accuracy: 0.000000001, "expected value to match")
+
+        testTimer.preferDisplayUnit(.minutes)
+        XCTAssertEqual(testTimer.retriveValueInPreferredUnit(atIndex: 0), value / 60, accuracy: 0.000000001, "expected value to match")
+
+        testTimer.preferDisplayUnit(.hours)
+        XCTAssertEqual(testTimer.retriveValueInPreferredUnit(atIndex: 0), value / (60 * 60), accuracy: 0.000000001, "expected value to match")
+
+        testTimer.preferDisplayUnit(.days)
+        XCTAssertEqual(testTimer.retriveValueInPreferredUnit(atIndex: 0), value / (60 * 60 * 24), accuracy: 0.000000001, "expected value to match")
+    }
 }
 
 // https://bugs.swift.org/browse/SR-6310

diff --git a/Tests/MetricsTests/TestMetrics.swift b/Tests/MetricsTests/TestMetrics.swift
@@ -154,20 +154,13 @@ internal class TestTimer: TimerHandler, Equatable {
         }
     }
 
-    func retriveValueInPreferredUnit(atIndex i: Int) -> Int64 {
+    func retriveValueInPreferredUnit(atIndex i: Int) -> Double {
         return self.lock.withLock {
             let value = values[i].1
             guard let displayUnit = self.displayUnit else {
-                return value
-            }
-            switch displayUnit {
-            case .days: return (value / 1_000_000_000) * 60 * 60 * 24
-            case .hours: return (value / 1_000_000_000) * 60 * 60
-            case .minutes: return (value / 1_000_000_000) * 60
-            case .seconds: return value / 1_000_000_000
-            case .milliseconds: return value / 1_000_000
-            case .nanoseconds: return value
+                return Double(value)
             }
+            return Double(value) / Double(displayUnit.scaleFromNanoseconds)
         }
     }