Histogram backed timer (#46)

* implement histogram backed timer

* hide timer implementation enum; namespace PrometheusMetricsFactory classes

* address review

* fix test testHistogramBackedTimer_scaleFromNanoseconds

* address PR review comments

Author: avolokhov

Sources/Prometheus/Prometheus.swift

@@ -16,15 +16,10 @@ public class PrometheusClient {
     /// Lock used for thread safety
     private let lock: Lock
 
-    /// Sanitizers used to clean up label values provided through
-    /// swift-metrics.
-    public let sanitizer: LabelSanitizer
-    
     /// Create a PrometheusClient instance
-    public init(labelSanitizer sanitizer: LabelSanitizer = PrometheusLabelSanitizer()) {
+    public init() {
         self.metrics = []
         self.metricTypeMap = [:]
-        self.sanitizer = sanitizer
         self.lock = Lock()
     }
 

Sources/Prometheus/PrometheusMetrics.swift

@@ -64,6 +64,24 @@ private class MetricsHistogram: RecorderHandler {
     }
 }
 
+class MetricsHistogramTimer: TimerHandler {
+    let histogram: PromHistogram<Int64, DimensionHistogramLabels>
+    let labels: DimensionHistogramLabels?
+
+    init(histogram: PromHistogram<Int64, DimensionHistogramLabels>, dimensions: [(String, String)]) {
+        self.histogram = histogram
+        if !dimensions.isEmpty {
+            self.labels = DimensionHistogramLabels(dimensions)
+        } else {
+            self.labels = nil
+        }
+    }
+
+    func recordNanoseconds(_ duration: Int64) {
+        return histogram.observe(duration, labels)
+    }
+}
+
 private class MetricsSummary: TimerHandler {
     let summary: PromSummary<Int64, DimensionSummaryLabels>
     let labels: DimensionSummaryLabels?
@@ -82,7 +100,7 @@ private class MetricsSummary: TimerHandler {
     }
 
     func recordNanoseconds(_ duration: Int64) {
-        summary.observe(duration, labels)
+        return summary.observe(duration, labels)
     }
 }
 
@@ -94,7 +112,7 @@ private class MetricsSummary: TimerHandler {
 ///     let sanitizer: LabelSanitizer = ...
 ///     let prometheusLabel = sanitizer.sanitize(nonPrometheusLabel)
 ///
-/// By default `PrometheusLabelSanitizer` is used by `PrometheusClient`
+/// By default `PrometheusLabelSanitizer` is used by `PrometheusMetricsFactory`
 public protocol LabelSanitizer {
     /// Sanitize the passed in label to a Prometheus accepted value.
     ///
@@ -121,73 +139,119 @@ public struct PrometheusLabelSanitizer: LabelSanitizer {
     }
 }
 
-extension PrometheusClient: MetricsFactory {
+/// A bridge between PrometheusClient and swift-metrics. Prometheus types don't map perfectly on swift-metrics API,
+/// which makes bridge implementation non trivial. This class defines how exactly swift-metrics types should be backed
+/// with Prometheus types, e.g. how to sanitize labels, what buckets/quantiles to use for recorder/timer, etc.
+public struct PrometheusMetricsFactory: MetricsFactory {
+
+    /// Prometheus client to bridge swift-metrics API to.
+    private let client: PrometheusClient
+
+    /// Bridge configuration.
+    private let configuration: Configuration
+
+    public init(client: PrometheusClient,
+                configuration: Configuration = Configuration()) {
+        self.client = client
+        self.configuration = configuration
+    }
+
     public func destroyCounter(_ handler: CounterHandler) {
         guard let handler = handler as? MetricsCounter else { return }
-        self.removeMetric(handler.counter)
+        client.removeMetric(handler.counter)
     }
 
     public func destroyRecorder(_ handler: RecorderHandler) {
         if let handler = handler as? MetricsGauge {
-            self.removeMetric(handler.gauge)
+            client.removeMetric(handler.gauge)
         }
         if let handler = handler as? MetricsHistogram {
-            self.removeMetric(handler.histogram)
+            client.removeMetric(handler.histogram)
         }
     }
-    
+
     public func destroyTimer(_ handler: TimerHandler) {
-        guard let handler = handler as? MetricsSummary else { return }
-        self.removeMetric(handler.summary)
+        switch self.configuration.timerImplementation._wrapped {
+        case .summary:
+            guard let handler = handler as? MetricsSummary else { return }
+            client.removeMetric(handler.summary)
+        case .histogram:
+            guard let handler = handler as? MetricsHistogramTimer else { return }
+            client.removeMetric(handler.histogram)
+        }
     }
 
     public func makeCounter(label: String, dimensions: [(String, String)]) -> CounterHandler {
-        let label = self.sanitizer.sanitize(label)
+        let label = configuration.labelSanitizer.sanitize(label)
         let createHandler = { (counter: PromCounter) -> CounterHandler in
             return MetricsCounter(counter: counter, dimensions: dimensions)
         }
-        if let counter: PromCounter<Int64, DimensionLabels> = self.getMetricInstance(with: label, andType: .counter) {
+        if let counter: PromCounter<Int64, DimensionLabels> = client.getMetricInstance(with: label, andType: .counter) {
             return createHandler(counter)
         }
-        return createHandler(self.createCounter(forType: Int64.self, named: label, withLabelType: DimensionLabels.self))
+        return createHandler(client.createCounter(forType: Int64.self, named: label, withLabelType: DimensionLabels.self))
     }
 
     public func makeRecorder(label: String, dimensions: [(String, String)], aggregate: Bool) -> RecorderHandler {
-        let label = self.sanitizer.sanitize(label)
+        let label = configuration.labelSanitizer.sanitize(label)
         return aggregate ? makeHistogram(label: label, dimensions: dimensions) : makeGauge(label: label, dimensions: dimensions)
     }
 
     private func makeGauge(label: String, dimensions: [(String, String)]) -> RecorderHandler {
-        let label = self.sanitizer.sanitize(label)
+        let label = configuration.labelSanitizer.sanitize(label)
         let createHandler = { (gauge: PromGauge) -> RecorderHandler in
             return MetricsGauge(gauge: gauge, dimensions: dimensions)
         }
-        if let gauge: PromGauge<Double, DimensionLabels> = self.getMetricInstance(with: label, andType: .gauge) {
+        if let gauge: PromGauge<Double, DimensionLabels> = client.getMetricInstance(with: label, andType: .gauge) {
             return createHandler(gauge)
         }
-        return createHandler(createGauge(forType: Double.self, named: label, withLabelType: DimensionLabels.self))
+        return createHandler(client.createGauge(forType: Double.self, named: label, withLabelType: DimensionLabels.self))
     }
 
     private func makeHistogram(label: String, dimensions: [(String, String)]) -> RecorderHandler {
-        let label = self.sanitizer.sanitize(label)
+        let label = configuration.labelSanitizer.sanitize(label)
         let createHandler = { (histogram: PromHistogram) -> RecorderHandler in
             return MetricsHistogram(histogram: histogram, dimensions: dimensions)
         }
-        if let histogram: PromHistogram<Double, DimensionHistogramLabels> = self.getMetricInstance(with: label, andType: .histogram) {
+        if let histogram: PromHistogram<Double, DimensionHistogramLabels> = client.getMetricInstance(with: label, andType: .histogram) {
             return createHandler(histogram)
         }
-        return createHandler(createHistogram(forType: Double.self, named: label, labels: DimensionHistogramLabels.self))
+        return createHandler(client.createHistogram(forType: Double.self, named: label, labels: DimensionHistogramLabels.self))
     }
 
     public func makeTimer(label: String, dimensions: [(String, String)]) -> TimerHandler {
-        let label = self.sanitizer.sanitize(label)
+        switch configuration.timerImplementation._wrapped {
+        case .summary(let quantiles):
+            return self.makeSummaryTimer(label: label, dimensions: dimensions, quantiles: quantiles)
+        case .histogram(let buckets):
+            return self.makeHistogramTimer(label: label, dimensions: dimensions, buckets: buckets)
+        }
+    }
+
+    /// There's two different ways to back swift-api `Timer` with Prometheus classes.
+    /// This method creates `Summary` backed timer implementation
+    private func makeSummaryTimer(label: String, dimensions: [(String, String)], quantiles: [Double]) -> TimerHandler {
+        let label = configuration.labelSanitizer.sanitize(label)
         let createHandler = { (summary: PromSummary) -> TimerHandler in
             return MetricsSummary(summary: summary, dimensions: dimensions)
         }
-        if let summary: PromSummary<Int64, DimensionSummaryLabels> = self.getMetricInstance(with: label, andType: .summary) {
+        if let summary: PromSummary<Int64, DimensionSummaryLabels> = client.getMetricInstance(with: label, andType: .summary) {
             return createHandler(summary)
         }
-        return createHandler(createSummary(forType: Int64.self, named: label, labels: DimensionSummaryLabels.self))
+        return createHandler(client.createSummary(forType: Int64.self, named: label, quantiles: quantiles, labels: DimensionSummaryLabels.self))
+    }
+
+    /// There's two different ways to back swift-api `Timer` with Prometheus classes.
+    /// This method creates `Histogram` backed timer implementation
+    private func makeHistogramTimer(label: String, dimensions: [(String, String)], buckets: Buckets) -> TimerHandler {
+        let createHandler = { (histogram: PromHistogram) -> TimerHandler in
+            MetricsHistogramTimer(histogram: histogram, dimensions: dimensions)
+        }
+        // PromHistogram should be reused when created for the same label, so we try to look it up
+        if let histogram: PromHistogram<Int64, DimensionHistogramLabels> = client.getMetricInstance(with: label, andType: .histogram) {
+            return createHandler(histogram)
+        }
+        return createHandler(client.createHistogram(forType: Int64.self, named: label, buckets: buckets, labels: DimensionHistogramLabels.self))
     }
 }
 

Sources/Prometheus/PrometheusMetricsConfiguration.swift

@@ -0,0 +1,45 @@
+extension PrometheusMetricsFactory {
+
+    public struct TimerImplementation {
+        enum _Wrapped {
+            case summary(defaultQuantiles: [Double])
+            case histogram(defaultBuckets: Buckets)
+        }
+
+        var _wrapped: _Wrapped
+
+        private init(_ wrapped: _Wrapped) {
+            self._wrapped = wrapped
+        }
+
+        public static func summary(defaultQuantiles: [Double] = Prometheus.defaultQuantiles) -> TimerImplementation {
+            return TimerImplementation(.summary(defaultQuantiles: defaultQuantiles))
+        }
+
+        public static func histogram(defaultBuckets: Buckets = Buckets.defaultBuckets) -> TimerImplementation {
+            return TimerImplementation(.histogram(defaultBuckets: defaultBuckets))
+        }
+    }
+
+
+    /// Configuration for PrometheusClient to swift-metrics api bridge.
+    public struct Configuration {
+        /// Sanitizers used to clean up label values provided through
+        /// swift-metrics.
+        public var labelSanitizer: LabelSanitizer
+
+        /// This parameter will define what implementation will be used for bridging `swift-metrics` to Prometheus types.
+        public var timerImplementation: PrometheusMetricsFactory.TimerImplementation
+
+        /// Default buckets for `Recorder` with aggregation.
+        public var defaultRecorderBuckets: Buckets
+
+        public init(labelSanitizer: LabelSanitizer = PrometheusLabelSanitizer(),
+                    timerImplementation: PrometheusMetricsFactory.TimerImplementation = .summary(),
+                    defaultRecorderBuckets: Buckets = .defaultBuckets) {
+            self.labelSanitizer = labelSanitizer
+            self.timerImplementation = timerImplementation
+            self.defaultRecorderBuckets = defaultRecorderBuckets
+        }
+    }
+}

Sources/PrometheusExample/main.swift

@@ -4,7 +4,7 @@ import NIO
 
 let myProm = PrometheusClient()
 
-MetricsSystem.bootstrap(myProm)
+MetricsSystem.bootstrap(PrometheusMetricsFactory(client: myProm))
 
 for _ in 0...Int.random(in: 10...100) {
     let c = Counter(label: "test")

Tests/SwiftPrometheusTests/GaugeTests.swift

@@ -25,7 +25,7 @@ final class GaugeTests: XCTestCase {
     override func setUp() {
         self.prom = PrometheusClient()
         self.group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
-        MetricsSystem.bootstrapInternal(prom)
+        MetricsSystem.bootstrapInternal(PrometheusMetricsFactory(client: prom))
     }
 
     override func tearDown() {

Tests/SwiftPrometheusTests/HistogramTests.swift

@@ -26,7 +26,7 @@ final class HistogramTests: XCTestCase {
     override func setUp() {
         self.prom = PrometheusClient()
         self.group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
-        MetricsSystem.bootstrapInternal(prom)
+        MetricsSystem.bootstrapInternal(PrometheusMetricsFactory(client: prom))
     }
 
     override func tearDown() {

Tests/SwiftPrometheusTests/PrometheusMetricsTests.swift

@@ -14,7 +14,7 @@ final class PrometheusMetricsTests: XCTestCase {
     override func setUp() {
         self.prom = PrometheusClient()
         self.group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
-        MetricsSystem.bootstrapInternal(prom)
+        MetricsSystem.bootstrapInternal(PrometheusMetricsFactory(client: prom))
     }
 
     override func tearDown() {
@@ -145,4 +145,42 @@ final class PrometheusMetricsTests: XCTestCase {
             my_gauge 100.0\n
             """)
     }
+
+    func testHistogramBackedTimer() {
+        let prom = PrometheusClient()
+        var config = PrometheusMetricsFactory.Configuration()
+        config.timerImplementation = .histogram()
+        let metricsFactory = PrometheusMetricsFactory(client: prom, configuration: config)
+        metricsFactory.makeTimer(label: "duration_nanos", dimensions: []).recordNanoseconds(1)
+        guard let histogram: PromHistogram<Int64, DimensionHistogramLabels> = prom.getMetricInstance(with: "duration_nanos", andType: .histogram) else {
+            XCTFail("Timer should be backed by Histogram")
+            return
+        }
+        let result = histogram.collect()
+        let buckets = result.split(separator: "\n").filter { $0.contains("duration_nanos_bucket") }
+        XCTAssertFalse(buckets.isEmpty, "default histogram backed timer buckets")
+    }
+
+    func testDestroyHistogramTimer() {
+        let prom = PrometheusClient()
+        var config = PrometheusMetricsFactory.Configuration()
+        config.timerImplementation = .histogram()
+        let metricsFactory = PrometheusMetricsFactory(client: prom, configuration: config)
+        let timer = metricsFactory.makeTimer(label: "duration_nanos", dimensions: [])
+        timer.recordNanoseconds(1)
+        metricsFactory.destroyTimer(timer)
+        let histogram: PromHistogram<Int64, DimensionHistogramLabels>? = prom.getMetricInstance(with: "duration_nanos", andType: .histogram)
+        XCTAssertNil(histogram)
+    }
+    func testDestroySummaryTimer() {
+        let prom = PrometheusClient()
+        var config = PrometheusMetricsFactory.Configuration()
+        config.timerImplementation = .summary()
+        let metricsFactory = PrometheusMetricsFactory(client: prom)
+        let timer = metricsFactory.makeTimer(label: "duration_nanos", dimensions: [])
+        timer.recordNanoseconds(1)
+        metricsFactory.destroyTimer(timer)
+        let summary: PromSummary<Int64, DimensionSummaryLabels>? = prom.getMetricInstance(with: "duration_nanos", andType: .summary)
+        XCTAssertNil(summary)
+    }
 }

Tests/SwiftPrometheusTests/SanitizerTests.swift

@@ -38,7 +38,7 @@ final class SanitizerTests: XCTestCase {
 
     func testIntegratedSanitizer() throws {
         let prom = PrometheusClient()
-        MetricsSystem.bootstrapInternal(prom)
+        MetricsSystem.bootstrapInternal(PrometheusMetricsFactory(client: prom))
 
         CoreMetrics.Counter(label: "Test.Counter").increment(by: 10)
 

Tests/SwiftPrometheusTests/SummaryTests.swift

@@ -26,7 +26,7 @@ final class SummaryTests: XCTestCase {
     override func setUp() {
         self.prom = PrometheusClient()
         self.group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
-        MetricsSystem.bootstrapInternal(prom)
+        MetricsSystem.bootstrapInternal(PrometheusMetricsFactory(client: prom))
     }
 
     override func tearDown() {