Skip to content

Finer concurrency in 'MetricType.observe' / 'MetricType.collect' #59

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

Merged
merged 3 commits into from
Aug 24, 2021
Merged
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
34 changes: 17 additions & 17 deletions Sources/Prometheus/MetricTypes/Counter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,24 @@ public class PromCounter<NumType: Numeric, Labels: MetricLabels>: PromMetric, Pr
/// - Returns:
/// Newline separated Prometheus formatted metric string
public func collect() -> String {
return self.lock.withLock {
var output = [String]()

if let help = self.help {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")

output.append("\(self.name) \(self.value)")

self.metrics.forEach { (labels, value) in
let labelsString = encodeLabels(labels)
output.append("\(self.name)\(labelsString) \(value)")
}

return output.joined(separator: "\n")
let (value, metrics) = self.lock.withLock {
(self.value, self.metrics)
}
var output = [String]()

if let help = self.help {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")

output.append("\(self.name) \(value)")

metrics.forEach { (labels, value) in
let labelsString = encodeLabels(labels)
output.append("\(self.name)\(labelsString) \(value)")
}

return output.joined(separator: "\n")
}

/// Increments the Counter
Expand Down Expand Up @@ -104,4 +105,3 @@ public class PromCounter<NumType: Numeric, Labels: MetricLabels>: PromMetric, Pr
}
}
}

33 changes: 17 additions & 16 deletions Sources/Prometheus/MetricTypes/Gauge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,24 @@ public class PromGauge<NumType: DoubleRepresentable, Labels: MetricLabels>: Prom
/// - Returns:
/// Newline separated Prometheus formatted metric string
public func collect() -> String {
return self.lock.withLock {
var output = [String]()

if let help = self.help {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")

output.append("\(self.name) \(self.value)")

self.metrics.forEach { (labels, value) in
let labelsString = encodeLabels(labels)
output.append("\(self.name)\(labelsString) \(value)")
}

return output.joined(separator: "\n")
let (value, metrics) = self.lock.withLock {
(self.value, self.metrics)
}
var output = [String]()

if let help = self.help {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")

output.append("\(self.name) \(value)")

metrics.forEach { (labels, value) in
let labelsString = encodeLabels(labels)
output.append("\(self.name)\(labelsString) \(value)")
}

return output.joined(separator: "\n")
}

/// Sets the Gauge to the current unix-time in seconds
Expand Down
139 changes: 71 additions & 68 deletions Sources/Prometheus/MetricTypes/Histogram.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ extension HistogramLabels {
public class PromHistogram<NumType: DoubleRepresentable, Labels: HistogramLabels>: PromMetric, PrometheusHandled {
/// Prometheus instance that created this Histogram
internal weak var prometheus: PrometheusClient?

/// Name of this Histogram, required
public let name: String
/// Help text of this Histogram, optional
Expand All @@ -99,10 +99,10 @@ public class PromHistogram<NumType: DoubleRepresentable, Labels: HistogramLabels
internal let upperBounds: [Double]

/// Labels for this Histogram
internal private(set) var labels: Labels
internal let labels: Labels

/// Sub Histograms for this Histogram
fileprivate var subHistograms: [PromHistogram<NumType, Labels>] = []
fileprivate var subHistograms: [Labels: PromHistogram<NumType, Labels>] = [:]

/// Total value of the Histogram
private let sum: PromCounter<NumType, EmptyLabels>
Expand All @@ -121,9 +121,9 @@ public class PromHistogram<NumType: DoubleRepresentable, Labels: HistogramLabels
internal init(_ name: String, _ help: String? = nil, _ labels: Labels = Labels(), _ buckets: Buckets = .defaultBuckets, _ p: PrometheusClient) {
self.name = name
self.help = help

self.prometheus = p

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

self.labels = labels
Expand All @@ -142,68 +142,76 @@ public class PromHistogram<NumType: DoubleRepresentable, Labels: HistogramLabels
/// - Returns:
/// Newline separated Prometheus formatted metric string
public func collect() -> String {
return self.lock.withLock {
var output = [String]()

if let help = self.help {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")

var acc: NumType = 0
for (i, bound) in self.upperBounds.enumerated() {
acc += self.buckets[i].get()
self.labels.le = bound.description
let labelsString = encodeLabels(self.labels)
output.append("\(self.name)_bucket\(labelsString) \(acc)")
}

let labelsString = encodeLabels(self.labels, ["le"])
output.append("\(self.name)_count\(labelsString) \(acc)")

output.append("\(self.name)_sum\(labelsString) \(self.sum.get())")

self.subHistograms.forEach { subHistogram in
var acc: NumType = 0
for (i, bound) in subHistogram.upperBounds.enumerated() {
acc += subHistogram.buckets[i].get()
subHistogram.labels.le = bound.description
let labelsString = encodeLabels(subHistogram.labels)
output.append("\(subHistogram.name)_bucket\(labelsString) \(acc)")
}

let labelsString = encodeLabels(subHistogram.labels, ["le"])
output.append("\(subHistogram.name)_count\(labelsString) \(acc)")

output.append("\(subHistogram.name)_sum\(labelsString) \(subHistogram.sum.get())")

subHistogram.labels.le = ""
let (buckets, subHistograms, labels) = self.lock.withLock {
(self.buckets, self.subHistograms, self.labels)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


var output = [String]()

if let help = self.help {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")
collectBuckets(buckets: buckets,
upperBounds: self.upperBounds,
name: self.name,
labels: labels,
sum: self.sum.get(),
into: &output)

subHistograms.forEach { subHistogram in
let (subHistogramBuckets, subHistogramLabels) = self.lock.withLock {
(subHistogram.value.buckets, subHistogram.value.labels)
}

self.labels.le = ""

return output.joined(separator: "\n")
collectBuckets(buckets: subHistogramBuckets,
upperBounds: subHistogram.value.upperBounds,
name: subHistogram.value.name,
labels: subHistogramLabels,
sum: subHistogram.value.sum.get(),
into: &output)
}
return output.joined(separator: "\n")
}


private func collectBuckets(buckets: [PromCounter<NumType, EmptyLabels>],
upperBounds: [Double],
name: String,
labels: Labels,
sum: NumType,
into output: inout [String]) {
var labels = labels
var acc: NumType = 0
for (i, bound) in upperBounds.enumerated() {
acc += buckets[i].get()
labels.le = bound.description
let labelsString = encodeLabels(labels)
output.append("\(name)_bucket\(labelsString) \(acc)")
}

let labelsString = encodeLabels(labels, ["le"])
output.append("\(name)_count\(labelsString) \(acc)")

output.append("\(name)_sum\(labelsString) \(sum)")
}

/// Observe a value
///
/// - Parameters:
/// - value: Value to observe
/// - labels: Labels to attach to the observed value
public func observe(_ value: NumType, _ labels: Labels? = nil) {
self.lock.withLock {
if let labels = labels, type(of: labels) != type(of: EmptySummaryLabels()) {
guard let his = self.prometheus?.getOrCreateHistogram(with: labels, for: self) else { fatalError("Lingering Histogram") }
his.observe(value)
if let labels = labels, type(of: labels) != type(of: EmptySummaryLabels()) {
let his: PromHistogram<NumType, Labels> = self.lock.withLock {
self.getOrCreateHistogram(with: labels)
}
self.sum.inc(value)

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

for (i, bound) in self.upperBounds.enumerated() {
if bound >= value.doubleValue {
self.buckets[i].inc()
return
}
}
}
Expand All @@ -222,21 +230,16 @@ public class PromHistogram<NumType: DoubleRepresentable, Labels: HistogramLabels
}
return try body()
}
}

extension PrometheusClient {
/// Helper for histograms & labels
fileprivate func getOrCreateHistogram<T: Numeric, U: HistogramLabels>(with labels: U, for histogram: PromHistogram<T, U>) -> PromHistogram<T, U> {
let histograms = histogram.subHistograms.filter { (metric) -> Bool in
guard metric.name == histogram.name, metric.help == histogram.help, metric.labels == labels else { return false }
return true
}
if histograms.count > 2 { fatalError("Somehow got 2 histograms with the same data type") }
if let histogram = histograms.first {
fileprivate func getOrCreateHistogram(with labels: Labels) -> PromHistogram<NumType, Labels> {
let histogram = self.subHistograms[labels]
if let histogram = histogram, histogram.name == self.name, histogram.help == self.help {
return histogram
} else {
let newHistogram = PromHistogram<T, U>(histogram.name, histogram.help, labels, Buckets(histogram.upperBounds), self)
histogram.subHistograms.append(newHistogram)
guard let prometheus = prometheus else { fatalError("Lingering Histogram") }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm can this actually ever happen or just annoyance because that weak there that shoulnd't really manifest in reality?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is a weak reference thing. I think reference to Prometheus is redundant, I'll try to track down its origins and potentially remove if indeed unnecessary.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah i see the error just moved around, it was there before.

let newHistogram = PromHistogram(self.name, self.help, labels, Buckets(self.upperBounds), prometheus)
self.subHistograms[labels] = newHistogram
return newHistogram
}
}
Expand Down
67 changes: 34 additions & 33 deletions Sources/Prometheus/MetricTypes/Summary.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,43 +95,44 @@ public class PromSummary<NumType: DoubleRepresentable, Labels: SummaryLabels>: P
/// - Returns:
/// Newline separated Prometheus formatted metric string
public func collect() -> String {
return self.lock.withLock {
var output = [String]()

if let help = self.help {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")
let (subSummaries, values) = lock.withLock {
(self.subSummaries, self.values)
}

var output = [String]()

if let help = self.help {
output.append("# HELP \(self.name) \(help)")
}
output.append("# TYPE \(self.name) \(self._type)")
var labels = self.labels
calculateQuantiles(quantiles: self.quantiles, values: values.map { $0.doubleValue }).sorted { $0.key < $1.key }.forEach { (arg) in
let (q, v) = arg
labels.quantile = "\(q)"
let labelsString = encodeLabels(labels)
output.append("\(self.name)\(labelsString) \(format(v))")
}

let labelsString = encodeLabels(labels, ["quantile"])
output.append("\(self.name)_count\(labelsString) \(self.count.get())")
output.append("\(self.name)_sum\(labelsString) \(format(self.sum.get().doubleValue))")

calculateQuantiles(quantiles: self.quantiles, values: self.values.map { $0.doubleValue }).sorted { $0.key < $1.key }.forEach { (arg) in
subSummaries.forEach { subSum in
var subSumLabels = subSum.labels
let subSumValues = lock.withLock { subSum.values }
calculateQuantiles(quantiles: self.quantiles, values: subSumValues.map { $0.doubleValue }).sorted { $0.key < $1.key }.forEach { (arg) in
let (q, v) = arg
self.labels.quantile = "\(q)"
let labelsString = encodeLabels(self.labels)
output.append("\(self.name)\(labelsString) \(format(v))")
}

let labelsString = encodeLabels(self.labels, ["quantile"])
output.append("\(self.name)_count\(labelsString) \(self.count.get())")
output.append("\(self.name)_sum\(labelsString) \(format(self.sum.get().doubleValue))")

self.subSummaries.forEach { subSum in
calculateQuantiles(quantiles: self.quantiles, values: subSum.values.map { $0.doubleValue }).sorted { $0.key < $1.key }.forEach { (arg) in
let (q, v) = arg
subSum.labels.quantile = "\(q)"
let labelsString = encodeLabels(subSum.labels)
output.append("\(subSum.name)\(labelsString) \(format(v))")
}

let labelsString = encodeLabels(subSum.labels, ["quantile"])
output.append("\(subSum.name)_count\(labelsString) \(subSum.count.get())")
output.append("\(subSum.name)_sum\(labelsString) \(format(subSum.sum.get().doubleValue))")
subSum.labels.quantile = ""
subSumLabels.quantile = "\(q)"
let labelsString = encodeLabels(subSumLabels)
output.append("\(subSum.name)\(labelsString) \(format(v))")
}
self.labels.quantile = ""

return output.joined(separator: "\n")

let labelsString = encodeLabels(subSumLabels, ["quantile"])
output.append("\(subSum.name)_count\(labelsString) \(subSum.count.get())")
output.append("\(subSum.name)_sum\(labelsString) \(format(subSum.sum.get().doubleValue))")
}

return output.joined(separator: "\n")
}

// Updated for SwiftMetrics 2.0 to be unit agnostic if displayUnit is set or default to nanoseconds.
Expand Down
1 change: 0 additions & 1 deletion Sources/Prometheus/Prometheus.swift
Original file line number Diff line number Diff line change
Expand Up @@ -290,4 +290,3 @@ public enum PrometheusError: Error {
/// but there was no `PrometheusClient` bootstrapped
case prometheusFactoryNotBootstrapped(bootstrappedWith: String)
}