Skip to content
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.

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

Fix behaviour for all metric types when only calls with labels are made #81

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ jobs:
- swift:5.4
- swift:5.5
- swift:5.6
- swift:5.7
- swiftlang/swift:nightly-main
swiftos:
- focal
Expand Down
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ let package = Package(
targets: ["Prometheus"])
],
dependencies: [
.package(url: "https://github.com/apple/swift-metrics.git", from: "2.2.0"),
.package(url: "https://github.com/apple/swift-metrics.git", from: "2.3.2"),
.package(url: "https://github.com/apple/swift-nio.git", from: "2.0.0"),
],
targets: [
Expand Down
13 changes: 10 additions & 3 deletions Sources/Prometheus/MetricTypes/Counter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ public class PromCounter<NumType: Numeric>: PromMetric {

/// Initial value of the counter
private let initialValue: NumType

/// Indicates wether or not metric has been used without labels
private var usedWithoutLabels: Bool

/// Storage of values that have labels attached
internal var metrics: [DimensionLabels: NumType] = [:]
Expand All @@ -36,6 +39,7 @@ public class PromCounter<NumType: Numeric>: PromMetric {
self.help = help
self.initialValue = initialValue
self.value = initialValue
self.usedWithoutLabels = initialValue != 0
self.lock = Lock()
}

Expand All @@ -44,8 +48,8 @@ public class PromCounter<NumType: Numeric>: PromMetric {
/// - Returns:
/// Newline separated Prometheus formatted metric string
public func collect() -> String {
let (value, metrics) = self.lock.withLock {
(self.value, self.metrics)
let (value, metrics, usedWithoutLabels) = self.lock.withLock {
(self.value, self.metrics, self.usedWithoutLabels)
}
var output = [String]()

Expand All @@ -54,7 +58,9 @@ public class PromCounter<NumType: Numeric>: PromMetric {
}
output.append("# TYPE \(self.name) \(self._type)")

output.append("\(self.name) \(value)")
if usedWithoutLabels {
output.append("\(self.name) \(value)")
}

metrics.forEach { (labels, value) in
let labelsString = encodeLabels(labels)
Expand All @@ -79,6 +85,7 @@ public class PromCounter<NumType: Numeric>: PromMetric {
self.metrics[labels] = val
return val
} else {
self.usedWithoutLabels = true
self.value += amount
return self.value
}
Expand Down
17 changes: 13 additions & 4 deletions Sources/Prometheus/MetricTypes/Gauge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ public class PromGauge<NumType: DoubleRepresentable>: PromMetric {

/// Initial value of the Gauge
private let initialValue: NumType


/// Indicates wether or not metric has been used without labels
private var usedWithoutLabels: Bool

/// Storage of values that have labels attached
private var metrics: [DimensionLabels: NumType] = [:]

Expand All @@ -39,6 +42,7 @@ public class PromGauge<NumType: DoubleRepresentable>: PromMetric {
self.help = help
self.initialValue = initialValue
self.value = initialValue
self.usedWithoutLabels = initialValue != 0
self.lock = Lock()
}

Expand All @@ -47,8 +51,8 @@ public class PromGauge<NumType: DoubleRepresentable>: PromMetric {
/// - Returns:
/// Newline separated Prometheus formatted metric string
public func collect() -> String {
let (value, metrics) = self.lock.withLock {
(self.value, self.metrics)
let (value, metrics, usedWithoutLabels) = self.lock.withLock {
(self.value, self.metrics, self.usedWithoutLabels)
}
var output = [String]()

Expand All @@ -57,7 +61,9 @@ public class PromGauge<NumType: DoubleRepresentable>: PromMetric {
}
output.append("# TYPE \(self.name) \(self._type)")

output.append("\(self.name) \(value)")
if usedWithoutLabels {
output.append("\(self.name) \(value)")
}

metrics.forEach { (labels, value) in
let labelsString = encodeLabels(labels)
Expand Down Expand Up @@ -129,6 +135,7 @@ public class PromGauge<NumType: DoubleRepresentable>: PromMetric {
self.metrics[labels] = amount
return amount
} else {
self.usedWithoutLabels = true
self.value = amount
return self.value
}
Expand All @@ -151,6 +158,7 @@ public class PromGauge<NumType: DoubleRepresentable>: PromMetric {
self.metrics[labels] = val
return val
} else {
self.usedWithoutLabels = true
self.value += amount
return self.value
}
Expand Down Expand Up @@ -184,6 +192,7 @@ public class PromGauge<NumType: DoubleRepresentable>: PromMetric {
self.metrics[labels] = val
return val
} else {
self.usedWithoutLabels = true
self.value -= amount
return self.value
}
Expand Down
40 changes: 25 additions & 15 deletions Sources/Prometheus/MetricTypes/Histogram.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ public class PromHistogram<NumType: DoubleRepresentable>: PromMetric {
/// Total value of the Histogram
private let sum: PromCounter<NumType>

/// Indicates wether or not metric has been used without labels
private var usedWithoutLabels: Bool

/// Lock used for thread safety
private let lock: Lock

Expand All @@ -102,6 +105,7 @@ public class PromHistogram<NumType: DoubleRepresentable>: PromMetric {
self.help = help

self.sum = .init("\(self.name)_sum", nil, 0)
self.usedWithoutLabels = false

self.upperBounds = buckets.buckets

Expand All @@ -117,8 +121,8 @@ public class PromHistogram<NumType: DoubleRepresentable>: PromMetric {
/// - Returns:
/// Newline separated Prometheus formatted metric string
public func collect() -> String {
let (buckets, subHistograms) = self.lock.withLock {
(self.buckets, self.subHistograms)
let (buckets, subHistograms, usedWithoutLabels) = self.lock.withLock {
(self.buckets, self.subHistograms, self.usedWithoutLabels)
}

var output = [String]()
Expand All @@ -129,12 +133,14 @@ public class PromHistogram<NumType: DoubleRepresentable>: PromMetric {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")
collectBuckets(buckets: buckets,
upperBounds: self.upperBounds,
name: self.name,
labels: nil,
sum: self.sum.get(),
into: &output)
if usedWithoutLabels {
collectBuckets(buckets: buckets,
upperBounds: self.upperBounds,
name: self.name,
labels: nil,
sum: self.sum.get(),
into: &output)
}

subHistograms.forEach { subHistogram in
let (subHistogramBuckets, subHistogramLabels) = self.lock.withLock {
Expand Down Expand Up @@ -178,13 +184,17 @@ public class PromHistogram<NumType: DoubleRepresentable>: PromMetric {
if let labels = labels {
self.getOrCreateHistogram(with: labels)
.observe(value)
}
self.sum.inc(value)

for (i, bound) in self.upperBounds.enumerated() {
if bound >= value.doubleValue {
self.buckets[i].inc()
return
} else {
self.sum.inc(value)

self.lock.withLock {
self.usedWithoutLabels = true
for (i, bound) in self.upperBounds.enumerated() {
if bound >= value.doubleValue {
self.buckets[i].inc()
return
}
}
}
}
}
Expand Down
42 changes: 26 additions & 16 deletions Sources/Prometheus/MetricTypes/Summary.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public class PromSummary<NumType: DoubleRepresentable>: PromMetric {

/// Sub Summaries for this Summary
fileprivate var subSummaries: [DimensionLabels: PromSummary<NumType>] = [:]

/// Indicates wether or not metric has been used without labels
private var usedWithoutLabels: Bool

/// Lock used for thread safety
private let lock: Lock
Expand Down Expand Up @@ -62,6 +65,8 @@ public class PromSummary<NumType: DoubleRepresentable>: PromMetric {

self.quantiles = quantiles

self.usedWithoutLabels = false

self.lock = Lock()
}

Expand All @@ -70,8 +75,8 @@ public class PromSummary<NumType: DoubleRepresentable>: PromMetric {
/// - Returns:
/// Newline separated Prometheus formatted metric string
public func collect() -> String {
let (subSummaries, values) = lock.withLock {
(self.subSummaries, self.values)
let (subSummaries, values, usedWithoutLabels) = lock.withLock {
(self.subSummaries, self.values, self.usedWithoutLabels)
}

var output = [String]()
Expand All @@ -82,14 +87,17 @@ public class PromSummary<NumType: DoubleRepresentable>: PromMetric {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")
calculateQuantiles(quantiles: self.quantiles, values: values.map { $0.doubleValue }).sorted { $0.key < $1.key }.forEach { (arg) in
let (q, v) = arg
let labelsString = encodeLabels(EncodableSummaryLabels(labels: nil, quantile: "\(q)"))
output.append("\(self.name)\(labelsString) \(format(v))")
}

output.append("\(self.name)_count \(self.count.get())")
output.append("\(self.name)_sum \(format(self.sum.get().doubleValue))")
if usedWithoutLabels {
calculateQuantiles(quantiles: self.quantiles, values: values.map { $0.doubleValue }).sorted { $0.key < $1.key }.forEach { (arg) in
let (q, v) = arg
let labelsString = encodeLabels(EncodableSummaryLabels(labels: nil, quantile: "\(q)"))
output.append("\(self.name)\(labelsString) \(format(v))")
}

output.append("\(self.name)_count \(self.count.get())")
output.append("\(self.name)_sum \(format(self.sum.get().doubleValue))")
}

subSummaries.forEach { labels, subSum in
let subSumValues = lock.withLock { subSum.values }
Expand Down Expand Up @@ -138,14 +146,16 @@ public class PromSummary<NumType: DoubleRepresentable>: PromMetric {
if let labels = labels {
let sum = self.getOrCreateSummary(withLabels: labels)
sum.observe(value)
}
self.count.inc(1)
self.sum.inc(value)
self.lock.withLock {
if self.values.count == self.capacity {
_ = self.values.popFirst()
} else {
self.count.inc(1)
self.sum.inc(value)
self.lock.withLock {
self.usedWithoutLabels = true
if self.values.count == self.capacity {
_ = self.values.popFirst()
}
self.values.append(value)
}
self.values.append(value)
}
}

Expand Down
18 changes: 18 additions & 0 deletions Tests/SwiftPrometheusTests/GaugeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,22 @@ final class GaugeTests: XCTestCase {
my_gauge{myValue="labels"} 20
""")
}

func testGaugeDoesNotReportWithNoLabelUsed() {
let gauge = prom.createGauge(forType: Int.self, named: "my_gauge")
gauge.inc(1, [("a", "b")])

XCTAssertEqual(gauge.collect(), """
# TYPE my_gauge gauge
my_gauge{a="b"} 1
""")

gauge.inc()

XCTAssertEqual(gauge.collect(), """
# TYPE my_gauge gauge
my_gauge 1
my_gauge{a="b"} 1
""")
}
}
63 changes: 51 additions & 12 deletions Tests/SwiftPrometheusTests/HistogramTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ final class HistogramTests: XCTestCase {
try elg.syncShutdownGracefully()

let output = histogram.collect()
XCTAssertTrue(output.contains("my_histogram_count 4000.0"))
XCTAssertTrue(output.contains("my_histogram_sum 4000.0"))
XCTAssertFalse(output.contains("my_histogram_count 4000.0"))
XCTAssertFalse(output.contains("my_histogram_sum 4000.0"))

XCTAssertTrue(output.contains(#"my_histogram_count{myValue="1"} 2000.0"#))
XCTAssertTrue(output.contains(#"my_histogram_sum{myValue="1"} 2000.0"#))
Expand Down Expand Up @@ -88,11 +88,11 @@ final class HistogramTests: XCTestCase {
my_histogram_bucket{le="0.5"} 0.0
my_histogram_bucket{le="1.0"} 1.0
my_histogram_bucket{le="2.5"} 2.0
my_histogram_bucket{le="5.0"} 4.0
my_histogram_bucket{le="10.0"} 4.0
my_histogram_bucket{le="+Inf"} 4.0
my_histogram_count 4.0
my_histogram_sum 9.0
my_histogram_bucket{le="5.0"} 3.0
my_histogram_bucket{le="10.0"} 3.0
my_histogram_bucket{le="+Inf"} 3.0
my_histogram_count 3.0
my_histogram_sum 6.0
my_histogram_bucket{myValue="labels", le="0.005"} 0.0
my_histogram_bucket{myValue="labels", le="0.01"} 0.0
my_histogram_bucket{myValue="labels", le="0.025"} 0.0
Expand Down Expand Up @@ -144,11 +144,11 @@ final class HistogramTests: XCTestCase {
my_histogram_bucket{le="0.5"} 0.0
my_histogram_bucket{le="1.0"} 1.0
my_histogram_bucket{le="2.0"} 2.0
my_histogram_bucket{le="3.0"} 4.0
my_histogram_bucket{le="5.0"} 4.0
my_histogram_bucket{le="+Inf"} 4.0
my_histogram_count 4.0
my_histogram_sum 9.0
my_histogram_bucket{le="3.0"} 3.0
my_histogram_bucket{le="5.0"} 3.0
my_histogram_bucket{le="+Inf"} 3.0
my_histogram_count 3.0
my_histogram_sum 6.0
my_histogram_bucket{myValue="labels", le="0.5"} 0.0
my_histogram_bucket{myValue="labels", le="1.0"} 0.0
my_histogram_bucket{myValue="labels", le="2.0"} 0.0
Expand All @@ -159,4 +159,43 @@ final class HistogramTests: XCTestCase {
my_histogram_sum{myValue="labels"} 3.0
""")
}

func testHistogramDoesNotReportWithNoLabelUsed() {
let histogram = prom.createHistogram(forType: Double.self, named: "my_histogram", buckets: [0.5, 1, 2, 3, 5, Double.greatestFiniteMagnitude])
histogram.observe(3, [("a", "b")])

XCTAssertEqual(histogram.collect(), """
# TYPE my_histogram histogram
my_histogram_bucket{le="0.5", a="b"} 0.0
my_histogram_bucket{le="1.0", a="b"} 0.0
my_histogram_bucket{le="2.0", a="b"} 0.0
my_histogram_bucket{le="3.0", a="b"} 1.0
my_histogram_bucket{le="5.0", a="b"} 1.0
my_histogram_bucket{le="+Inf", a="b"} 1.0
my_histogram_count{a="b"} 1.0
my_histogram_sum{a="b"} 3.0
""")

histogram.observe(3)

XCTAssertEqual(histogram.collect(), """
# TYPE my_histogram histogram
my_histogram_bucket{le="0.5"} 0.0
my_histogram_bucket{le="1.0"} 0.0
my_histogram_bucket{le="2.0"} 0.0
my_histogram_bucket{le="3.0"} 1.0
my_histogram_bucket{le="5.0"} 1.0
my_histogram_bucket{le="+Inf"} 1.0
my_histogram_count 1.0
my_histogram_sum 3.0
my_histogram_bucket{le="0.5", a="b"} 0.0
my_histogram_bucket{le="1.0", a="b"} 0.0
my_histogram_bucket{le="2.0", a="b"} 0.0
my_histogram_bucket{le="3.0", a="b"} 1.0
my_histogram_bucket{le="5.0", a="b"} 1.0
my_histogram_bucket{le="+Inf", a="b"} 1.0
my_histogram_count{a="b"} 1.0
my_histogram_sum{a="b"} 3.0
""")
}
}
Loading