From 9a83036251c691c6559ebfe2f719a1f7b4453af4 Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Thu, 28 Aug 2025 10:12:50 -0700 Subject: [PATCH 1/4] feat: add {emitToString, emitToBuffer} options + dedup TYPE and HELP per metric name Add {emitToString, emitToBuffer} thread-safe internal buffer management variants to emit metrics. Deduplicate TYPE and HELP lines per metric name by default on emit + enforce same HELP per metric name. Signed-off-by: Melissa Kilby --- .../PrometheusCollectorRegistry.swift | 268 +++++++++++++----- Tests/PrometheusTests/CounterTests.swift | 22 +- Tests/PrometheusTests/GaugeTests.swift | 22 +- Tests/PrometheusTests/HistogramTests.swift | 44 ++- .../PrometheusCollectorRegistryTests.swift | 234 +++++++++++++++ 5 files changed, 467 insertions(+), 123 deletions(-) diff --git a/Sources/Prometheus/PrometheusCollectorRegistry.swift b/Sources/Prometheus/PrometheusCollectorRegistry.swift index a573f4c..888c75a 100644 --- a/Sources/Prometheus/PrometheusCollectorRegistry.swift +++ b/Sources/Prometheus/PrometheusCollectorRegistry.swift @@ -47,11 +47,6 @@ public final class PrometheusCollectorRegistry: Sendable { } } - private struct MetricWithHelp: Sendable { - var metric: Metric - let help: String - } - private enum HistogramBuckets: Sendable, Hashable { case duration([Duration]) case value([Double]) @@ -65,12 +60,14 @@ public final class PrometheusCollectorRegistry: Sendable { /// For histograms, the buckets are immutable for a MetricGroup once initialized with the first /// metric. See also https://github.com/prometheus/OpenMetrics/issues/197. private struct MetricGroup: Sendable { - var metricsByLabelSets: [LabelsKey: MetricWithHelp] + var metricsByLabelSets: [LabelsKey: Metric] let buckets: HistogramBuckets? + let help: String? - init(metricsByLabelSets: [LabelsKey: MetricWithHelp] = [:], buckets: HistogramBuckets? = nil) { + init(metricsByLabelSets: [LabelsKey: Metric] = [:], buckets: HistogramBuckets? = nil, help: String? = nil) { self.metricsByLabelSets = metricsByLabelSets self.buckets = buckets + self.help = help } } @@ -83,7 +80,10 @@ public final class PrometheusCollectorRegistry: Sendable { private let box = NIOLockedValueBox([String: Metric]()) - /// Create a new collector registry + /// Creates a new PrometheusCollectorRegistry with default configuration. + /// + /// Uses deduplication for TYPE and HELP lines according to Prometheus specifications, + /// where only one TYPE and HELP line is emitted per metric name regardless of label sets. public init() {} // MARK: Creating Metrics @@ -145,23 +145,35 @@ public final class PrometheusCollectorRegistry: Sendable { guard let entry = store[name] else { // First time a Counter is registered with this name. let counter = Counter(name: name, labels: labels) - let counterWithHelp = MetricWithHelp(metric: counter, help: help) let counterGroup = MetricGroup( - metricsByLabelSets: [key: counterWithHelp] + metricsByLabelSets: [key: counter], + help: help ) store[name] = .counter(counterGroup) return counter } + switch entry { case .counter(var existingCounterGroup): - if let existingCounterWithHelp = existingCounterGroup.metricsByLabelSets[key] { - return existingCounterWithHelp.metric + // Validate help text consistency + if let existingHelp = existingCounterGroup.help, existingHelp != help { + fatalError( + """ + Help text mismatch for metric '\(name)': + Existing help: '\(existingHelp)' + New help: '\(help)' + All metrics with the same name must have identical help text. + """ + ) + } + + if let existingCounter = existingCounterGroup.metricsByLabelSets[key] { + return existingCounter } // Even if the metric name is identical, each label set defines a unique time series. let counter = Counter(name: name, labels: labels) - let counterWithHelp = MetricWithHelp(metric: counter, help: help) - existingCounterGroup.metricsByLabelSets[key] = counterWithHelp + existingCounterGroup.metricsByLabelSets[key] = counter // Write the modified entry back to the store. store[name] = .counter(existingCounterGroup) @@ -266,23 +278,35 @@ public final class PrometheusCollectorRegistry: Sendable { guard let entry = store[name] else { // First time a Gauge is registered with this name. let gauge = Gauge(name: name, labels: labels) - let gaugeWithHelp = MetricWithHelp(metric: gauge, help: help) let gaugeGroup = MetricGroup( - metricsByLabelSets: [key: gaugeWithHelp] + metricsByLabelSets: [key: gauge], + help: help ) store[name] = .gauge(gaugeGroup) return gauge } + switch entry { case .gauge(var existingGaugeGroup): - if let existingGaugeWithHelp = existingGaugeGroup.metricsByLabelSets[key] { - return existingGaugeWithHelp.metric + // Validate help text consistency + if let existingHelp = existingGaugeGroup.help, existingHelp != help { + fatalError( + """ + Help text mismatch for metric '\(name)': + Existing help: '\(existingHelp)' + New help: '\(help)' + All metrics with the same name must have identical help text. + """ + ) + } + + if let existingGauge = existingGaugeGroup.metricsByLabelSets[key] { + return existingGauge } // Even if the metric name is identical, each label set defines a unique time series. let gauge = Gauge(name: name, labels: labels) - let gaugeWithHelp = MetricWithHelp(metric: gauge, help: help) - existingGaugeGroup.metricsByLabelSets[key] = gaugeWithHelp + existingGaugeGroup.metricsByLabelSets[key] = gauge // Write the modified entry back to the store. store[name] = .gauge(existingGaugeGroup) @@ -401,10 +425,10 @@ public final class PrometheusCollectorRegistry: Sendable { guard let entry = store[name] else { // First time a DurationHistogram is registered with this name. This defines the buckets. let histogram = DurationHistogram(name: name, labels: labels, buckets: buckets) - let histogramWithHelp = MetricWithHelp(metric: histogram, help: help) let histogramGroup = MetricGroup( - metricsByLabelSets: [key: histogramWithHelp], - buckets: .duration(buckets) + metricsByLabelSets: [key: histogram], + buckets: .duration(buckets), + help: help ) store[name] = .durationHistogram(histogramGroup) return histogram @@ -412,6 +436,18 @@ public final class PrometheusCollectorRegistry: Sendable { switch entry { case .durationHistogram(var existingHistogramGroup): + // Validate help text consistency + if let existingHelp = existingHistogramGroup.help, existingHelp != help { + fatalError( + """ + Help text mismatch for metric '\(name)': + Existing help: '\(existingHelp)' + New help: '\(help)' + All metrics with the same name must have identical help text. + """ + ) + } + // Validate buckets match the stored ones. if case .duration(let storedBuckets) = existingHistogramGroup.buckets { guard storedBuckets == buckets else { @@ -426,14 +462,13 @@ public final class PrometheusCollectorRegistry: Sendable { } } - if let existingHistogramWithHelp = existingHistogramGroup.metricsByLabelSets[key] { - return existingHistogramWithHelp.metric + if let existingHistogram = existingHistogramGroup.metricsByLabelSets[key] { + return existingHistogram } // Even if the metric name is identical, each label set defines a unique time series. let histogram = DurationHistogram(name: name, labels: labels, buckets: buckets) - let histogramWithHelp = MetricWithHelp(metric: histogram, help: help) - existingHistogramGroup.metricsByLabelSets[key] = histogramWithHelp + existingHistogramGroup.metricsByLabelSets[key] = histogram // Write the modified entry back to the store. store[name] = .durationHistogram(existingHistogramGroup) @@ -572,10 +607,10 @@ public final class PrometheusCollectorRegistry: Sendable { guard let entry = store[name] else { // First time a ValueHistogram is registered with this name. This defines the buckets. let histogram = ValueHistogram(name: name, labels: labels, buckets: buckets) - let histogramWithHelp = MetricWithHelp(metric: histogram, help: help) let histogramGroup = MetricGroup( - metricsByLabelSets: [key: histogramWithHelp], - buckets: .value(buckets) + metricsByLabelSets: [key: histogram], + buckets: .value(buckets), + help: help ) store[name] = .valueHistogram(histogramGroup) return histogram @@ -583,6 +618,18 @@ public final class PrometheusCollectorRegistry: Sendable { switch entry { case .valueHistogram(var existingHistogramGroup): + // Validate help text consistency + if let existingHelp = existingHistogramGroup.help, existingHelp != help { + fatalError( + """ + Help text mismatch for metric '\(name)': + Existing help: '\(existingHelp)' + New help: '\(help)' + All metrics with the same name must have identical help text. + """ + ) + } + // Validate buckets match the stored ones. if case .value(let storedBuckets) = existingHistogramGroup.buckets { guard storedBuckets == buckets else { @@ -597,14 +644,13 @@ public final class PrometheusCollectorRegistry: Sendable { } } - if let existingHistogramWithHelp = existingHistogramGroup.metricsByLabelSets[key] { - return existingHistogramWithHelp.metric + if let existingHistogram = existingHistogramGroup.metricsByLabelSets[key] { + return existingHistogram } // Even if the metric name is identical, each label set defines a unique time series. let histogram = ValueHistogram(name: name, labels: labels, buckets: buckets) - let histogramWithHelp = MetricWithHelp(metric: histogram, help: help) - existingHistogramGroup.metricsByLabelSets[key] = histogramWithHelp + existingHistogramGroup.metricsByLabelSets[key] = histogram // Write the modified entry back to the store. store[name] = .valueHistogram(existingHistogramGroup) @@ -686,8 +732,8 @@ public final class PrometheusCollectorRegistry: Sendable { switch store[counter.name] { case .counter(var counterGroup): let key = LabelsKey(counter.labels) - guard let existingCounterGroup = counterGroup.metricsByLabelSets[key], - existingCounterGroup.metric === counter + guard let existingCounter = counterGroup.metricsByLabelSets[key], + existingCounter === counter else { return } @@ -714,8 +760,8 @@ public final class PrometheusCollectorRegistry: Sendable { switch store[gauge.name] { case .gauge(var gaugeGroup): let key = LabelsKey(gauge.labels) - guard let existingGaugeGroup = gaugeGroup.metricsByLabelSets[key], - existingGaugeGroup.metric === gauge + guard let existingGauge = gaugeGroup.metricsByLabelSets[key], + existingGauge === gauge else { return } @@ -742,8 +788,8 @@ public final class PrometheusCollectorRegistry: Sendable { switch store[histogram.name] { case .durationHistogram(var histogramGroup): let key = LabelsKey(histogram.labels) - guard let existingHistogramGroup = histogramGroup.metricsByLabelSets[key], - existingHistogramGroup.metric === histogram + guard let existingHistogram = histogramGroup.metricsByLabelSets[key], + existingHistogram === histogram else { return } @@ -770,8 +816,8 @@ public final class PrometheusCollectorRegistry: Sendable { switch store[histogram.name] { case .valueHistogram(var histogramGroup): let key = LabelsKey(histogram.labels) - guard let existingHistogramGroup = histogramGroup.metricsByLabelSets[key], - existingHistogramGroup.metric === histogram + guard let existingHistogram = histogramGroup.metricsByLabelSets[key], + existingHistogram === histogram else { return } @@ -790,59 +836,147 @@ public final class PrometheusCollectorRegistry: Sendable { // MARK: Emitting + private let bufferBox = NIOLockedValueBox([UInt8]()) + + /// Resets the internal buffer used by ``emitToString()`` and ``emitToBuffer()``. + /// + /// Forces the buffer capacity back to 0, which will trigger re-calibration on the next emission call. + /// This is useful when the registry's metric composition has changed significantly and you want to + /// optimize buffer size for the new workload. + /// + /// - Note: Thread-safe. Does not affect ``emit(into:)`` calls which use external buffers + public func resetInternalBuffer() { + bufferBox.withLockedValue { buffer in + // Resets capacity to 0, forcing re-calibration + buffer.removeAll() + } + } + + /// Returns the current capacity of the internal buffer used by ``emitToString()`` and ``emitToBuffer()``. + /// + /// The capacity represents the allocated memory size, not the current content length. A capacity of 0 + /// indicates the buffer will auto-size on the next emission call. The capacity may grow over time as + /// the registry's output requirements increase. + /// + /// - Returns: The current buffer capacity in bytes + /// - Note: Thread-safe. Primarily useful for testing and monitoring buffer behavior + public func internalBufferCapacity() -> Int { + return bufferBox.withLockedValue { buffer in + buffer.capacity + } + } + + /// Emits all registered metrics in Prometheus text format as a String. + /// + /// Uses an internal buffer that auto-sizes on first call to find optimal initial capacity. The buffer + /// may resize during the registry's lifetime if output grows significantly. Subsequent calls reuse the + /// established capacity, clearing content but preserving the initially allocated memory. + /// + /// - Returns: A String containing all registered metrics in Prometheus text format + /// - Note: Thread-safe. Use ``resetInternalBuffer()`` to force recalibration + /// - SeeAlso: ``emitToBuffer()`` for raw UTF-8 bytes, ``emit(into:)`` for custom buffer + public func emitToString() -> String { + return bufferBox.withLockedValue { buffer in + guard buffer.capacity == 0 else { + // Subsequent times: clear content but keep the capacity + buffer.removeAll(keepingCapacity: true) + emit(into: &buffer) + return String(decoding: buffer, as: UTF8.self) + } + // First time: emit and let buffer auto-resize to find the initial optimal size + emit(into: &buffer) + return String(decoding: buffer, as: UTF8.self) + } + } + + /// Emits all registered metrics in Prometheus text format as a UTF-8 byte array. + /// + /// Uses an internal buffer that auto-sizes on first call to find optimal initial capacity. The buffer + /// may resize during the registry's lifetime if output grows significantly. Subsequent calls reuse the + /// established capacity, clearing content but preserving the initially allocated memory. Returns a copy. + /// + /// - Returns: A copy of the UTF-8 encoded byte array containing all registered metrics + /// - Note: Thread-safe. Use ``resetInternalBuffer()`` to force recalibration + /// - SeeAlso: ``emitToString()`` for String output, ``emit(into:)`` for custom buffer + public func emitToBuffer() -> [UInt8] { + return bufferBox.withLockedValue { buffer in + guard buffer.capacity == 0 else { + buffer.removeAll(keepingCapacity: true) + emit(into: &buffer) + return Array(buffer) // Creates a copy + } + emit(into: &buffer) + return Array(buffer) // Creates a copy + } + } + + /// Emits all registered metrics in Prometheus text format into the provided buffer. + /// + /// Writes directly into the supplied buffer without any internal buffer management or thread synchronization. + /// The caller is responsible for buffer sizing, clearing, and thread safety. This method provides maximum + /// performance and control but requires manual buffer lifecycle management. + /// + /// - Parameter buffer: The buffer to write metrics data into. Content will be appended to existing data + /// - Note: Not thread-safe. Caller must handle synchronization and may optimize buffer capacity for + /// maximum performance by reducing reallocations + /// - SeeAlso: ``emitToString()`` and ``emitToBuffer()`` for automatic buffer management public func emit(into buffer: inout [UInt8]) { let metrics = self.box.withLockedValue { $0 } let prefixHelp = "HELP" let prefixType = "TYPE" - + // Emit TYPE/HELP once per metric, then all instances for (name, metric) in metrics { switch metric { case .counter(let counterGroup): - // Should not be empty, as a safeguard skip if it is. guard let _ = counterGroup.metricsByLabelSets.first?.value else { continue } - for counterWithHelp in counterGroup.metricsByLabelSets.values { - let help = counterWithHelp.help - help.isEmpty ? () : buffer.addLine(prefix: prefixHelp, name: name, value: help) - buffer.addLine(prefix: prefixType, name: name, value: "counter") - counterWithHelp.metric.emit(into: &buffer) + + if let help = counterGroup.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "counter") + for counter in counterGroup.metricsByLabelSets.values { + counter.emit(into: &buffer) } case .gauge(let gaugeGroup): - // Should not be empty, as a safeguard skip if it is. guard let _ = gaugeGroup.metricsByLabelSets.first?.value else { continue } - for gaugeWithHelp in gaugeGroup.metricsByLabelSets.values { - let help = gaugeWithHelp.help - help.isEmpty ? () : buffer.addLine(prefix: prefixHelp, name: name, value: help) - buffer.addLine(prefix: prefixType, name: name, value: "gauge") - gaugeWithHelp.metric.emit(into: &buffer) + + if let help = gaugeGroup.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "gauge") + for gauge in gaugeGroup.metricsByLabelSets.values { + gauge.emit(into: &buffer) } case .durationHistogram(let histogramGroup): - // Should not be empty, as a safeguard skip if it is. guard let _ = histogramGroup.metricsByLabelSets.first?.value else { continue } - for histogramWithHelp in histogramGroup.metricsByLabelSets.values { - let help = histogramWithHelp.help - help.isEmpty ? () : buffer.addLine(prefix: prefixHelp, name: name, value: help) - buffer.addLine(prefix: prefixType, name: name, value: "histogram") - histogramWithHelp.metric.emit(into: &buffer) + + if let help = histogramGroup.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "histogram") + for histogram in histogramGroup.metricsByLabelSets.values { + histogram.emit(into: &buffer) } case .valueHistogram(let histogramGroup): - // Should not be empty, as a safeguard skip if it is. guard let _ = histogramGroup.metricsByLabelSets.first?.value else { continue } - for histogramWithHelp in histogramGroup.metricsByLabelSets.values { - let help = histogramWithHelp.help - help.isEmpty ? () : buffer.addLine(prefix: prefixHelp, name: name, value: help) - buffer.addLine(prefix: prefixType, name: name, value: "histogram") - histogramWithHelp.metric.emit(into: &buffer) + + if let help = histogramGroup.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "histogram") + for histogram in histogramGroup.metricsByLabelSets.values { + histogram.emit(into: &buffer) } } } diff --git a/Tests/PrometheusTests/CounterTests.swift b/Tests/PrometheusTests/CounterTests.swift index bef97b9..15f0553 100644 --- a/Tests/PrometheusTests/CounterTests.swift +++ b/Tests/PrometheusTests/CounterTests.swift @@ -171,19 +171,19 @@ final class CounterTests: XCTestCase { let counter0 = client.makeCounter( name: "foo", labels: [], - help: "Base metric name with no labels" + help: "Shared help text" ) let counter1 = client.makeCounter( name: "foo", labels: [("bar", "baz")], - help: "Base metric name with one label set variant" + help: "Shared help text" ) let counter2 = client.makeCounter( name: "foo", labels: [("bar", "newBaz"), ("newKey1", "newValue1")], - help: "Base metric name with a different label set variant" + help: "Shared help text" ) var buffer = [UInt8]() @@ -197,16 +197,12 @@ final class CounterTests: XCTestCase { var outputString = String(decoding: buffer, as: Unicode.UTF8.self) var actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) var expectedLines = Set([ - "# HELP foo Base metric name with no labels", + "# HELP foo Shared help text", "# TYPE foo counter", "foo 2", - "# HELP foo Base metric name with one label set variant", - "# TYPE foo counter", #"foo{bar="baz"} 12"#, - "# HELP foo Base metric name with a different label set variant", - "# TYPE foo counter", #"foo{bar="newBaz",newKey1="newValue1"} 24"#, ]) XCTAssertEqual(actualLines, expectedLines) @@ -218,12 +214,10 @@ final class CounterTests: XCTestCase { outputString = String(decoding: buffer, as: Unicode.UTF8.self) actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) expectedLines = Set([ - "# HELP foo Base metric name with one label set variant", + "# HELP foo Shared help text", "# TYPE foo counter", #"foo{bar="baz"} 12"#, - "# HELP foo Base metric name with a different label set variant", - "# TYPE foo counter", #"foo{bar="newBaz",newKey1="newValue1"} 24"#, ]) XCTAssertEqual(actualLines, expectedLines) @@ -234,7 +228,7 @@ final class CounterTests: XCTestCase { outputString = String(decoding: buffer, as: Unicode.UTF8.self) actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) expectedLines = Set([ - "# HELP foo Base metric name with a different label set variant", + "# HELP foo Shared help text", "# TYPE foo counter", #"foo{bar="newBaz",newKey1="newValue1"} 24"#, ]) @@ -251,14 +245,14 @@ final class CounterTests: XCTestCase { let _ = client.makeGauge( name: "foo", labels: [], - help: "Base metric name used for new metric of type gauge" + help: "Shared help text" ) buffer.removeAll(keepingCapacity: true) client.emit(into: &buffer) XCTAssertEqual( String(decoding: buffer, as: Unicode.UTF8.self), """ - # HELP foo Base metric name used for new metric of type gauge + # HELP foo Shared help text # TYPE foo gauge foo 0.0 diff --git a/Tests/PrometheusTests/GaugeTests.swift b/Tests/PrometheusTests/GaugeTests.swift index 36ab213..a8de29e 100644 --- a/Tests/PrometheusTests/GaugeTests.swift +++ b/Tests/PrometheusTests/GaugeTests.swift @@ -158,19 +158,19 @@ final class GaugeTests: XCTestCase { let gauge0 = client.makeGauge( name: "foo", labels: [], - help: "Base metric name with no labels" + help: "Shared help text" ) let gauge1 = client.makeGauge( name: "foo", labels: [("bar", "baz")], - help: "Base metric name with one label set variant" + help: "Shared help text" ) let gauge2 = client.makeGauge( name: "foo", labels: [("bar", "newBaz"), ("newKey1", "newValue1")], - help: "Base metric name with a different label set variant" + help: "Shared help text" ) var buffer = [UInt8]() @@ -184,16 +184,12 @@ final class GaugeTests: XCTestCase { var outputString = String(decoding: buffer, as: Unicode.UTF8.self) var actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) var expectedLines = Set([ - "# HELP foo Base metric name with no labels", + "# HELP foo Shared help text", "# TYPE foo gauge", "foo 2.0", - "# HELP foo Base metric name with one label set variant", - "# TYPE foo gauge", #"foo{bar="baz"} -3.0"#, - "# HELP foo Base metric name with a different label set variant", - "# TYPE foo gauge", #"foo{bar="newBaz",newKey1="newValue1"} 28.0"#, ]) XCTAssertEqual(actualLines, expectedLines) @@ -205,12 +201,10 @@ final class GaugeTests: XCTestCase { outputString = String(decoding: buffer, as: Unicode.UTF8.self) actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) expectedLines = Set([ - "# HELP foo Base metric name with one label set variant", + "# HELP foo Shared help text", "# TYPE foo gauge", #"foo{bar="baz"} -3.0"#, - "# HELP foo Base metric name with a different label set variant", - "# TYPE foo gauge", #"foo{bar="newBaz",newKey1="newValue1"} 28.0"#, ]) XCTAssertEqual(actualLines, expectedLines) @@ -221,7 +215,7 @@ final class GaugeTests: XCTestCase { outputString = String(decoding: buffer, as: Unicode.UTF8.self) actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) expectedLines = Set([ - "# HELP foo Base metric name with a different label set variant", + "# HELP foo Shared help text", "# TYPE foo gauge", #"foo{bar="newBaz",newKey1="newValue1"} 28.0"#, ]) @@ -238,14 +232,14 @@ final class GaugeTests: XCTestCase { let _ = client.makeCounter( name: "foo", labels: [], - help: "Base metric name used for new metric of type counter" + help: "Shared help text" ) buffer.removeAll(keepingCapacity: true) client.emit(into: &buffer) XCTAssertEqual( String(decoding: buffer, as: Unicode.UTF8.self), """ - # HELP foo Base metric name used for new metric of type counter + # HELP foo Shared help text # TYPE foo counter foo 0 diff --git a/Tests/PrometheusTests/HistogramTests.swift b/Tests/PrometheusTests/HistogramTests.swift index db7bcfe..32c8672 100644 --- a/Tests/PrometheusTests/HistogramTests.swift +++ b/Tests/PrometheusTests/HistogramTests.swift @@ -385,21 +385,21 @@ final class HistogramTests: XCTestCase { name: "foo", labels: [], buckets: sharedBuckets, - help: "Base metric name with no labels" + help: "Shared help text" ) let histogram1 = client.makeDurationHistogram( name: "foo", labels: [("bar", "baz")], buckets: sharedBuckets, // Must match the first registration - help: "Base metric name with one label set variant" + help: "Shared help text" ) let histogram2 = client.makeDurationHistogram( name: "foo", labels: [("bar", "newBaz"), ("newKey1", "newValue1")], buckets: sharedBuckets, // Must match the first registration - help: "Base metric name with a different label set variant" + help: "Shared help text" ) var buffer = [UInt8]() @@ -414,7 +414,7 @@ final class HistogramTests: XCTestCase { var outputString = String(decoding: buffer, as: Unicode.UTF8.self) var actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) var expectedLines = Set([ - "# HELP foo Base metric name with no labels", + "# HELP foo Shared help text", "# TYPE foo histogram", "foo_bucket{le=\"0.1\"} 0", "foo_bucket{le=\"0.5\"} 1", @@ -423,8 +423,6 @@ final class HistogramTests: XCTestCase { "foo_sum 1.1", "foo_count 2", - "# HELP foo Base metric name with one label set variant", - "# TYPE foo histogram", #"foo_bucket{bar="baz",le="0.1"} 0"#, #"foo_bucket{bar="baz",le="0.5"} 0"#, #"foo_bucket{bar="baz",le="1.0"} 1"#, @@ -432,8 +430,6 @@ final class HistogramTests: XCTestCase { #"foo_sum{bar="baz"} 2.1"#, #"foo_count{bar="baz"} 2"#, - "# HELP foo Base metric name with a different label set variant", - "# TYPE foo histogram", #"foo_bucket{bar="newBaz",newKey1="newValue1",le="0.1"} 1"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="0.5"} 2"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="1.0"} 2"#, @@ -450,7 +446,7 @@ final class HistogramTests: XCTestCase { outputString = String(decoding: buffer, as: Unicode.UTF8.self) actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) expectedLines = Set([ - "# HELP foo Base metric name with one label set variant", + "# HELP foo Shared help text", "# TYPE foo histogram", #"foo_bucket{bar="baz",le="0.1"} 0"#, #"foo_bucket{bar="baz",le="0.5"} 0"#, @@ -459,8 +455,6 @@ final class HistogramTests: XCTestCase { #"foo_sum{bar="baz"} 2.1"#, #"foo_count{bar="baz"} 2"#, - "# HELP foo Base metric name with a different label set variant", - "# TYPE foo histogram", #"foo_bucket{bar="newBaz",newKey1="newValue1",le="0.1"} 1"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="0.5"} 2"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="1.0"} 2"#, @@ -476,7 +470,7 @@ final class HistogramTests: XCTestCase { outputString = String(decoding: buffer, as: Unicode.UTF8.self) actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) expectedLines = Set([ - "# HELP foo Base metric name with a different label set variant", + "# HELP foo Shared help text", "# TYPE foo histogram", #"foo_bucket{bar="newBaz",newKey1="newValue1",le="0.1"} 1"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="0.5"} 2"#, @@ -498,14 +492,14 @@ final class HistogramTests: XCTestCase { let _ = client.makeGauge( name: "foo", labels: [], - help: "Base metric name used for new metric of type gauge" + help: "Shared help text" ) buffer.removeAll(keepingCapacity: true) client.emit(into: &buffer) XCTAssertEqual( String(decoding: buffer, as: Unicode.UTF8.self), """ - # HELP foo Base metric name used for new metric of type gauge + # HELP foo Shared help text # TYPE foo gauge foo 0.0 @@ -525,21 +519,21 @@ final class HistogramTests: XCTestCase { name: "foo", labels: [], buckets: sharedBuckets, - help: "Base metric name with no labels" + help: "Shared help text" ) let histogram1 = client.makeValueHistogram( name: "foo", labels: [("bar", "baz")], buckets: sharedBuckets, // Must match the first registration - help: "Base metric name with one label set variant" + help: "Shared help text" ) let histogram2 = client.makeValueHistogram( name: "foo", labels: [("bar", "newBaz"), ("newKey1", "newValue1")], buckets: sharedBuckets, // Must match the first registration - help: "Base metric name with a different label set variant" + help: "Shared help text" ) var buffer = [UInt8]() @@ -554,7 +548,7 @@ final class HistogramTests: XCTestCase { var outputString = String(decoding: buffer, as: Unicode.UTF8.self) var actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) var expectedLines = Set([ - "# HELP foo Base metric name with no labels", + "# HELP foo Shared help text", "# TYPE foo histogram", "foo_bucket{le=\"1.0\"} 0", "foo_bucket{le=\"5.0\"} 1", @@ -563,8 +557,6 @@ final class HistogramTests: XCTestCase { "foo_sum 11.0", "foo_count 2", - "# HELP foo Base metric name with one label set variant", - "# TYPE foo histogram", #"foo_bucket{bar="baz",le="1.0"} 0"#, #"foo_bucket{bar="baz",le="5.0"} 0"#, #"foo_bucket{bar="baz",le="10.0"} 1"#, @@ -572,8 +564,6 @@ final class HistogramTests: XCTestCase { #"foo_sum{bar="baz"} 18.0"#, #"foo_count{bar="baz"} 2"#, - "# HELP foo Base metric name with a different label set variant", - "# TYPE foo histogram", #"foo_bucket{bar="newBaz",newKey1="newValue1",le="1.0"} 0"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="5.0"} 2"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="10.0"} 2"#, @@ -590,7 +580,7 @@ final class HistogramTests: XCTestCase { outputString = String(decoding: buffer, as: Unicode.UTF8.self) actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) expectedLines = Set([ - "# HELP foo Base metric name with one label set variant", + "# HELP foo Shared help text", "# TYPE foo histogram", #"foo_bucket{bar="baz",le="1.0"} 0"#, #"foo_bucket{bar="baz",le="5.0"} 0"#, @@ -599,8 +589,6 @@ final class HistogramTests: XCTestCase { #"foo_sum{bar="baz"} 18.0"#, #"foo_count{bar="baz"} 2"#, - "# HELP foo Base metric name with a different label set variant", - "# TYPE foo histogram", #"foo_bucket{bar="newBaz",newKey1="newValue1",le="1.0"} 0"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="5.0"} 2"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="10.0"} 2"#, @@ -616,7 +604,7 @@ final class HistogramTests: XCTestCase { outputString = String(decoding: buffer, as: Unicode.UTF8.self) actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) expectedLines = Set([ - "# HELP foo Base metric name with a different label set variant", + "# HELP foo Shared help text", "# TYPE foo histogram", #"foo_bucket{bar="newBaz",newKey1="newValue1",le="1.0"} 0"#, #"foo_bucket{bar="newBaz",newKey1="newValue1",le="5.0"} 2"#, @@ -638,14 +626,14 @@ final class HistogramTests: XCTestCase { let _ = client.makeGauge( name: "foo", labels: [], - help: "Base metric name used for new metric of type gauge" + help: "Shared help text" ) buffer.removeAll(keepingCapacity: true) client.emit(into: &buffer) XCTAssertEqual( String(decoding: buffer, as: Unicode.UTF8.self), """ - # HELP foo Base metric name used for new metric of type gauge + # HELP foo Shared help text # TYPE foo gauge foo 0.0 diff --git a/Tests/PrometheusTests/PrometheusCollectorRegistryTests.swift b/Tests/PrometheusTests/PrometheusCollectorRegistryTests.swift index c883f81..e6988b0 100644 --- a/Tests/PrometheusTests/PrometheusCollectorRegistryTests.swift +++ b/Tests/PrometheusTests/PrometheusCollectorRegistryTests.swift @@ -261,4 +261,238 @@ final class PrometheusCollectorRegistryTests: XCTestCase { _ = registry.makeCounter(name: "name", labels: [("a", "1")]) } + + func testInternalBufferEmitToStringEmitToBuffer() { + let client = PrometheusCollectorRegistry() + + // Initially, buffer should have no capacity + XCTAssertEqual(client.internalBufferCapacity(), 0) + + // Create some metrics to establish buffer size. + let gauge1 = client.makeGauge(name: "test_gauge_1", labels: []) + let gauge2 = client.makeGauge(name: "test_gauge_2", labels: [("label", "value")]) + let counter = client.makeCounter(name: "test_counter", labels: []) + + gauge1.set(42.0) + gauge2.set(100.0) + counter.increment(by: 5.0) + + // First emission - start with emitToBuffer (this will auto-size the internal buffer). + let output1Buffer = client.emitToBuffer() + let output1String = client.emitToString() + + XCTAssertFalse(output1String.isEmpty) + XCTAssertFalse(output1Buffer.isEmpty) + + // Verify both outputs represent the same data. + XCTAssertEqual(output1String, String(decoding: output1Buffer, as: UTF8.self)) + + // Buffer should now have some capacity. + let initialCapacity = client.internalBufferCapacity() + XCTAssertGreaterThan(initialCapacity, 0) + + // Second emission - start with emitToString this time. + let output2String = client.emitToString() + let output2Buffer = client.emitToBuffer() + + // Same content regardless of method used. + XCTAssertEqual(output1String, output2String) + XCTAssertEqual(output1Buffer, output2Buffer) + XCTAssertEqual(output2String, String(decoding: output2Buffer, as: UTF8.self)) + XCTAssertEqual(client.internalBufferCapacity(), initialCapacity) // Same capacity + + // Reset the internal buffer. + client.resetInternalBuffer() + XCTAssertEqual(client.internalBufferCapacity(), 0) // Capacity should be reset to 0 + + // Add more metrics to change the required buffer size. + let histogram = client.makeValueHistogram( + name: "test_histogram", + labels: [("method", "GET"), ("status", "200")], + buckets: [0.1, 0.5, 1.0, 5.0, 10.0] + ) + histogram.record(2.5) + + // This emission should re-calibrate the buffer size - start with emitToString. + let output3String = client.emitToString() + let output3Buffer = client.emitToBuffer() + + XCTAssertTrue(output3String.contains("test_histogram")) + XCTAssertTrue(output3String.contains("test_gauge_1")) + XCTAssertEqual(output3String, String(decoding: output3Buffer, as: UTF8.self)) + + // Buffer should have a new capacity after re-calibration. + let recalibratedCapacity = client.internalBufferCapacity() + XCTAssertGreaterThan(recalibratedCapacity, 0) + XCTAssertGreaterThan(recalibratedCapacity, initialCapacity) + + // Add many more metrics after re-calibration to force buffer resizing. + // Add multiple gauges with long names and labels to increase buffer requirements. + var additionalGauges: [Gauge] = [] + for i in 0..<10 { + let gauge = client.makeGauge( + name: "test_gauge_with_very_long_name_to_increase_buffer_size_\(i)", + labels: [ + ("environment", "production_environment_with_long_value"), + ("service", "microservice_with_extremely_long_service_name"), + ("region", "us-west-2-availability-zone-1a-with-long-suffix"), + ("version", "v1.2.3-build-12345-commit-abcdef123456789"), + ] + ) + gauge.set(Double(i * 100)) + additionalGauges.append(gauge) + } + + // Add multiple counters with extensive labels. + var additionalCounters: [Counter] = [] + for i in 0..<5 { + let counter = client.makeCounter( + name: "http_requests_total_with_very_descriptive_name_\(i)", + labels: [ + ("method", "POST"), + ("endpoint", "/api/v1/users/profile/settings/notifications/preferences"), + ("status_code", "200"), + ("user_agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36"), + ("client_ip", "192.168.1.100"), + ("request_id", "req-\(UUID().uuidString)"), + ] + ) + counter.increment(by: Double(i + 1) * 50) + additionalCounters.append(counter) + } + + // Add multiple histograms with many buckets. + var additionalHistograms: [ValueHistogram] = [] + for i in 0..<3 { + let histogram = client.makeValueHistogram( + name: "request_duration_seconds_detailed_histogram_\(i)", + labels: [ + ("service", "authentication-service-with-long-name"), + ("operation", "validate-user-credentials-and-permissions"), + ("database", "postgresql-primary-read-write-instance"), + ], + buckets: [ + 0.001, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1.0, 2.5, 5.0, 7.5, 10.0, 15.0, 20.0, + 30.0, + ] + ) + for j in 0..<20 { + histogram.record(Double(j) * 0.1 + Double(i)) + } + additionalHistograms.append(histogram) + } + + // This should definitely trigger buffer resizing due to massive amount of new content. + let output4String = client.emitToString() + let output4Buffer = client.emitToBuffer() + + XCTAssertTrue(output4String.contains("test_histogram")) + XCTAssertTrue(output4String.contains("test_gauge_with_very_long_name")) + XCTAssertTrue(output4String.contains("http_requests_total_with_very_descriptive_name")) + XCTAssertTrue(output4String.contains("request_duration_seconds_detailed_histogram")) + XCTAssertEqual(output4String, String(decoding: output4Buffer, as: UTF8.self)) + + // Buffer capacity should have grown significantly to accommodate all the additional metrics. + let newCapacity = client.internalBufferCapacity() + XCTAssertGreaterThan(newCapacity, 0) + XCTAssertGreaterThan(newCapacity, recalibratedCapacity) // Should be much larger now + + // Subsequent emission should reuse the new buffer size - start with emitToBuffer. + let output5Buffer = client.emitToBuffer() + let output5String = client.emitToString() + + XCTAssertEqual(output4String, output5String) + XCTAssertEqual(output4Buffer, output5Buffer) + XCTAssertEqual(output5String, String(decoding: output5Buffer, as: UTF8.self)) + XCTAssertEqual(client.internalBufferCapacity(), newCapacity) // Capacity unchanged + + // Verify buffer reset works with empty registry - unregister all metrics + client.unregisterGauge(gauge1) + client.unregisterGauge(gauge2) + client.unregisterCounter(counter) + client.unregisterValueHistogram(histogram) + + // Unregister all additional metrics. + for gauge in additionalGauges { + client.unregisterGauge(gauge) + } + for counter in additionalCounters { + client.unregisterCounter(counter) + } + for histogram in additionalHistograms { + client.unregisterValueHistogram(histogram) + } + + client.resetInternalBuffer() + XCTAssertEqual(client.internalBufferCapacity(), 0) // Reset to 0 again + + // Test empty output with both methods - start with emitToString. + let emptyOutputString = client.emitToString() + let emptyOutputBuffer = client.emitToBuffer() + + XCTAssertTrue(emptyOutputString.isEmpty) + XCTAssertTrue(emptyOutputBuffer.isEmpty) + XCTAssertEqual(emptyOutputString, String(decoding: emptyOutputBuffer, as: UTF8.self)) + + // With empty output, buffer capacity should remain 0. + let emptyCapacity = client.internalBufferCapacity() + XCTAssertEqual(emptyCapacity, 0) + } + + func testDefaultRegistryDedupTypeHelpPerMetricNameOnEmitWhenMetricNameSharedInMetricGroup() { + let client = PrometheusCollectorRegistry() + + let gauge0 = client.makeGauge( + name: "foo", + labels: [], + help: "Shared help text for all variants" + ) + + let gauge1 = client.makeGauge( + name: "foo", + labels: [("bar", "baz")], + help: "Shared help text for all variants" + ) + + let gauge2 = client.makeGauge( + name: "foo", + labels: [("bar", "newBaz"), ("newKey1", "newValue1")], + help: "Shared help text for all variants" + ) + + gauge0.set(to: 1.0) + gauge1.set(to: 9.0) + gauge2.set(to: 4.0) + + var buffer = [UInt8]() + client.emit(into: &buffer) + let outputString = String(decoding: buffer, as: Unicode.UTF8.self) + let lines = outputString.components(separatedBy: .newlines).filter { !$0.isEmpty } + + // Should have exactly one HELP and one TYPE line. + let helpLines = lines.filter { $0.hasPrefix("# HELP foo") } + let typeLines = lines.filter { $0.hasPrefix("# TYPE foo") } + let metricLines = lines.filter { $0.hasPrefix("foo") && !$0.hasPrefix("# ") } + + XCTAssertEqual(helpLines.count, 1, "Should have exactly one HELP line") + XCTAssertEqual(typeLines.count, 1, "Should have exactly one TYPE line") + XCTAssertEqual(metricLines.count, 3, "Should have three metric value lines") + + XCTAssertEqual(helpLines.first, "# HELP foo Shared help text for all variants") + XCTAssertEqual(typeLines.first, "# TYPE foo gauge") + + // Verify HELP and TYPE appear before any metric values. + let helpIndex = lines.firstIndex { $0.hasPrefix("# HELP foo") }! + let typeIndex = lines.firstIndex { $0.hasPrefix("# TYPE foo") }! + let firstMetricIndex = lines.firstIndex { $0.hasPrefix("foo") && !$0.hasPrefix("# ") }! + + XCTAssertLessThan(helpIndex, firstMetricIndex, "HELP should appear before metric values") + XCTAssertLessThan(typeIndex, firstMetricIndex, "TYPE should appear before metric values") + + // Verify all three metric values are present (order doesn't matter). + XCTAssertTrue(metricLines.contains("foo 1.0")) + XCTAssertTrue(metricLines.contains(#"foo{bar="baz"} 9.0"#)) + XCTAssertTrue(metricLines.contains(#"foo{bar="newBaz",newKey1="newValue1"} 4.0"#)) + } + } From 1184ea7e05db50c8051dcdc6e1e8c308febbd793 Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Thu, 28 Aug 2025 11:28:40 -0700 Subject: [PATCH 2/4] fix: slightly stricter mutual exclusivity constraints for metric name sharing Revert to stricter mutual exclusivity constraints for metric name sharing with respect to labeled and unlabeled cases while still allowing different label sets per metric name. While Prometheus wouldn't break with sharing a metric name for labeled and unlabeled variants, the client enforces that each unique metric name must either be labeled (allowing multiple label sets) or unlabeled in order to support correct Prometheus aggregations. Use MetricFamily instead of MetricGroup. Signed-off-by: Melissa Kilby --- .../PrometheusCollectorRegistry.swift | 439 ++++++++++++------ Tests/PrometheusTests/CounterTests.swift | 23 - Tests/PrometheusTests/GaugeTests.swift | 23 - Tests/PrometheusTests/HistogramTests.swift | 79 ---- .../PrometheusCollectorRegistryTests.swift | 12 +- 5 files changed, 307 insertions(+), 269 deletions(-) diff --git a/Sources/Prometheus/PrometheusCollectorRegistry.swift b/Sources/Prometheus/PrometheusCollectorRegistry.swift index 888c75a..c984083 100644 --- a/Sources/Prometheus/PrometheusCollectorRegistry.swift +++ b/Sources/Prometheus/PrometheusCollectorRegistry.swift @@ -52,30 +52,37 @@ public final class PrometheusCollectorRegistry: Sendable { case value([Double]) } - /// A collection of metrics, each with a unique label set, that share the same metric name. - /// Distinct help strings for the same metric name are permitted, but Prometheus retains only the - /// first one. For an unlabelled metric, the empty label set is used as the key, and the - /// collection contains only one entry. Finally, for clarification, the same metric name can - /// simultaneously be labeled and unlabeled. - /// For histograms, the buckets are immutable for a MetricGroup once initialized with the first + /// A MetricFamily. + /// + /// A metric name can either map to multiple labeled metrics OR a single unlabeled metric, + /// but not both simultaneously to ensure proper Prometheus aggregations. + /// All metrics with the same name must have identical help text for consistency. + /// For histograms, the buckets are immutable for a MetricFamily once initialized with the first /// metric. See also https://github.com/prometheus/OpenMetrics/issues/197. - private struct MetricGroup: Sendable { + private struct MetricFamily: Sendable { var metricsByLabelSets: [LabelsKey: Metric] let buckets: HistogramBuckets? let help: String? - - init(metricsByLabelSets: [LabelsKey: Metric] = [:], buckets: HistogramBuckets? = nil, help: String? = nil) { + let metricUnlabeled: Metric? + + init( + metricsByLabelSets: [LabelsKey: Metric] = [:], + buckets: HistogramBuckets? = nil, + help: String? = nil, + metricUnlabeled: Metric? = nil + ) { self.metricsByLabelSets = metricsByLabelSets self.buckets = buckets self.help = help + self.metricUnlabeled = metricUnlabeled } } private enum Metric { - case counter(MetricGroup) - case gauge(MetricGroup) - case durationHistogram(MetricGroup) - case valueHistogram(MetricGroup) + case counter(MetricFamily) + case gauge(MetricFamily) + case durationHistogram(MetricFamily) + case valueHistogram(MetricFamily) } private let box = NIOLockedValueBox([String: Metric]()) @@ -145,18 +152,50 @@ public final class PrometheusCollectorRegistry: Sendable { guard let entry = store[name] else { // First time a Counter is registered with this name. let counter = Counter(name: name, labels: labels) - let counterGroup = MetricGroup( - metricsByLabelSets: [key: counter], - help: help + let counterFamily = MetricFamily( + metricsByLabelSets: labels.isEmpty ? [:] : [key: counter], + help: help, + metricUnlabeled: labels.isEmpty ? counter : nil ) - store[name] = .counter(counterGroup) + store[name] = .counter(counterFamily) return counter } switch entry { - case .counter(var existingCounterGroup): - // Validate help text consistency - if let existingHelp = existingCounterGroup.help, existingHelp != help { + case .counter(var existingCounterFamily): + // Check mutual exclusivity constraints. While Prometheus wouldn't break with sharing a + // metric name for labeled and unlabeled variants, the client enforces that each unique + // metric name must either be labeled (allowing multiple label sets) or unlabeled + // in order to support correct Prometheus aggregations. + if existingCounterFamily.metricUnlabeled != nil && !labels.isEmpty { + fatalError( + """ + Label conflict for metric '\(name)': + Cannot register a labeled metric when an unlabeled metric already exists. + """ + ) + } + if labels.isEmpty && !existingCounterFamily.metricsByLabelSets.isEmpty { + fatalError( + """ + Label conflict for metric '\(name)': + Cannot register an unlabeled metric when labeled metrics already exist. + """ + ) + } + + // For unlabeled metrics, return the existing one if it exists. + if labels.isEmpty, let existingUnlabeled = existingCounterFamily.metricUnlabeled { + return existingUnlabeled + } + + if let existingCounter = existingCounterFamily.metricsByLabelSets[key] { + return existingCounter + } + + // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct + // HELP lines, the client enforces uniqueness and consistency. + if let existingHelp = existingCounterFamily.help, existingHelp != help { fatalError( """ Help text mismatch for metric '\(name)': @@ -167,16 +206,12 @@ public final class PrometheusCollectorRegistry: Sendable { ) } - if let existingCounter = existingCounterGroup.metricsByLabelSets[key] { - return existingCounter - } - // Even if the metric name is identical, each label set defines a unique time series. let counter = Counter(name: name, labels: labels) - existingCounterGroup.metricsByLabelSets[key] = counter + existingCounterFamily.metricsByLabelSets[key] = counter // Write the modified entry back to the store. - store[name] = .counter(existingCounterGroup) + store[name] = .counter(existingCounterFamily) return counter @@ -278,18 +313,50 @@ public final class PrometheusCollectorRegistry: Sendable { guard let entry = store[name] else { // First time a Gauge is registered with this name. let gauge = Gauge(name: name, labels: labels) - let gaugeGroup = MetricGroup( - metricsByLabelSets: [key: gauge], - help: help + let gaugeFamily = MetricFamily( + metricsByLabelSets: labels.isEmpty ? [:] : [key: gauge], + help: help, + metricUnlabeled: labels.isEmpty ? gauge : nil ) - store[name] = .gauge(gaugeGroup) + store[name] = .gauge(gaugeFamily) return gauge } switch entry { - case .gauge(var existingGaugeGroup): - // Validate help text consistency - if let existingHelp = existingGaugeGroup.help, existingHelp != help { + case .gauge(var existingGaugeFamily): + // Check mutual exclusivity constraints. While Prometheus wouldn't break with sharing a + // metric name for labeled and unlabeled variants, the client enforces that each unique + // metric name must either be labeled (allowing multiple label sets) or unlabeled + // in order to support correct Prometheus aggregations. + if existingGaugeFamily.metricUnlabeled != nil && !labels.isEmpty { + fatalError( + """ + Label conflict for metric '\(name)': + Cannot register a labeled metric when an unlabeled metric already exists. + """ + ) + } + if labels.isEmpty && !existingGaugeFamily.metricsByLabelSets.isEmpty { + fatalError( + """ + Label conflict for metric '\(name)': + Cannot register an unlabeled metric when labeled metrics already exist. + """ + ) + } + + // For unlabeled metrics, return the existing one if it exists. + if labels.isEmpty, let existingUnlabeled = existingGaugeFamily.metricUnlabeled { + return existingUnlabeled + } + + if let existingGauge = existingGaugeFamily.metricsByLabelSets[key] { + return existingGauge + } + + // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct + // HELP lines, the client enforces uniqueness and consistency. + if let existingHelp = existingGaugeFamily.help, existingHelp != help { fatalError( """ Help text mismatch for metric '\(name)': @@ -300,16 +367,12 @@ public final class PrometheusCollectorRegistry: Sendable { ) } - if let existingGauge = existingGaugeGroup.metricsByLabelSets[key] { - return existingGauge - } - // Even if the metric name is identical, each label set defines a unique time series. let gauge = Gauge(name: name, labels: labels) - existingGaugeGroup.metricsByLabelSets[key] = gauge + existingGaugeFamily.metricsByLabelSets[key] = gauge // Write the modified entry back to the store. - store[name] = .gauge(existingGaugeGroup) + store[name] = .gauge(existingGaugeFamily) return gauge @@ -425,31 +488,41 @@ public final class PrometheusCollectorRegistry: Sendable { guard let entry = store[name] else { // First time a DurationHistogram is registered with this name. This defines the buckets. let histogram = DurationHistogram(name: name, labels: labels, buckets: buckets) - let histogramGroup = MetricGroup( - metricsByLabelSets: [key: histogram], + let histogramFamily = MetricFamily( + metricsByLabelSets: labels.isEmpty ? [:] : [key: histogram], buckets: .duration(buckets), - help: help + help: help, + metricUnlabeled: labels.isEmpty ? histogram : nil ) - store[name] = .durationHistogram(histogramGroup) + store[name] = .durationHistogram(histogramFamily) return histogram } switch entry { - case .durationHistogram(var existingHistogramGroup): - // Validate help text consistency - if let existingHelp = existingHistogramGroup.help, existingHelp != help { + case .durationHistogram(var existingHistogramFamily): + // Check mutual exclusivity constraints. While Prometheus wouldn't break with sharing a + // metric name for labeled and unlabeled variants, the client enforces that each unique + // metric name must either be labeled (allowing multiple label sets) or unlabeled + // in order to support correct Prometheus aggregations. + if existingHistogramFamily.metricUnlabeled != nil && !labels.isEmpty { fatalError( """ - Help text mismatch for metric '\(name)': - Existing help: '\(existingHelp)' - New help: '\(help)' - All metrics with the same name must have identical help text. + Label conflict for metric '\(name)': + Cannot register a labeled metric when an unlabeled metric already exists. + """ + ) + } + if labels.isEmpty && !existingHistogramFamily.metricsByLabelSets.isEmpty { + fatalError( + """ + Label conflict for metric '\(name)': + Cannot register an unlabeled metric when labeled metrics already exist. """ ) } // Validate buckets match the stored ones. - if case .duration(let storedBuckets) = existingHistogramGroup.buckets { + if case .duration(let storedBuckets) = existingHistogramFamily.buckets { guard storedBuckets == buckets else { fatalError( """ @@ -462,16 +535,34 @@ public final class PrometheusCollectorRegistry: Sendable { } } - if let existingHistogram = existingHistogramGroup.metricsByLabelSets[key] { + // For unlabeled metrics, return the existing one if it exists. + if labels.isEmpty, let existingUnlabeled = existingHistogramFamily.metricUnlabeled { + return existingUnlabeled + } + + if let existingHistogram = existingHistogramFamily.metricsByLabelSets[key] { return existingHistogram } + // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct + // HELP lines, the client enforces uniqueness and consistency. + if let existingHelp = existingHistogramFamily.help, existingHelp != help { + fatalError( + """ + Help text mismatch for metric '\(name)': + Existing help: '\(existingHelp)' + New help: '\(help)' + All metrics with the same name must have identical help text. + """ + ) + } + // Even if the metric name is identical, each label set defines a unique time series. let histogram = DurationHistogram(name: name, labels: labels, buckets: buckets) - existingHistogramGroup.metricsByLabelSets[key] = histogram + existingHistogramFamily.metricsByLabelSets[key] = histogram // Write the modified entry back to the store. - store[name] = .durationHistogram(existingHistogramGroup) + store[name] = .durationHistogram(existingHistogramFamily) return histogram @@ -607,31 +698,41 @@ public final class PrometheusCollectorRegistry: Sendable { guard let entry = store[name] else { // First time a ValueHistogram is registered with this name. This defines the buckets. let histogram = ValueHistogram(name: name, labels: labels, buckets: buckets) - let histogramGroup = MetricGroup( - metricsByLabelSets: [key: histogram], + let histogramFamily = MetricFamily( + metricsByLabelSets: labels.isEmpty ? [:] : [key: histogram], buckets: .value(buckets), - help: help + help: help, + metricUnlabeled: labels.isEmpty ? histogram : nil ) - store[name] = .valueHistogram(histogramGroup) + store[name] = .valueHistogram(histogramFamily) return histogram } switch entry { - case .valueHistogram(var existingHistogramGroup): - // Validate help text consistency - if let existingHelp = existingHistogramGroup.help, existingHelp != help { + case .valueHistogram(var existingHistogramFamily): + // Check mutual exclusivity constraints. While Prometheus wouldn't break with sharing a + // metric name for labeled and unlabeled variants, the client enforces that each unique + // metric name must either be labeled (allowing multiple label sets) or unlabeled + // in order to support correct Prometheus aggregations. + if existingHistogramFamily.metricUnlabeled != nil && !labels.isEmpty { fatalError( """ - Help text mismatch for metric '\(name)': - Existing help: '\(existingHelp)' - New help: '\(help)' - All metrics with the same name must have identical help text. + Label conflict for metric '\(name)': + Cannot register a labeled metric when an unlabeled metric already exists. + """ + ) + } + if labels.isEmpty && !existingHistogramFamily.metricsByLabelSets.isEmpty { + fatalError( + """ + Label conflict for metric '\(name)': + Cannot register an unlabeled metric when labeled metrics already exist. """ ) } // Validate buckets match the stored ones. - if case .value(let storedBuckets) = existingHistogramGroup.buckets { + if case .value(let storedBuckets) = existingHistogramFamily.buckets { guard storedBuckets == buckets else { fatalError( """ @@ -644,16 +745,34 @@ public final class PrometheusCollectorRegistry: Sendable { } } - if let existingHistogram = existingHistogramGroup.metricsByLabelSets[key] { + // For unlabeled metrics, return the existing one if it exists. + if labels.isEmpty, let existingUnlabeled = existingHistogramFamily.metricUnlabeled { + return existingUnlabeled + } + + if let existingHistogram = existingHistogramFamily.metricsByLabelSets[key] { return existingHistogram } + // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct + // HELP lines, the client enforces uniqueness and consistency. + if let existingHelp = existingHistogramFamily.help, existingHelp != help { + fatalError( + """ + Help text mismatch for metric '\(name)': + Existing help: '\(existingHelp)' + New help: '\(help)' + All metrics with the same name must have identical help text. + """ + ) + } + // Even if the metric name is identical, each label set defines a unique time series. let histogram = ValueHistogram(name: name, labels: labels, buckets: buckets) - existingHistogramGroup.metricsByLabelSets[key] = histogram + existingHistogramFamily.metricsByLabelSets[key] = histogram // Write the modified entry back to the store. - store[name] = .valueHistogram(existingHistogramGroup) + store[name] = .valueHistogram(existingHistogramFamily) return histogram @@ -730,19 +849,28 @@ public final class PrometheusCollectorRegistry: Sendable { public func unregisterCounter(_ counter: Counter) { self.box.withLockedValue { store in switch store[counter.name] { - case .counter(var counterGroup): + case .counter(var counterFamily): + // Check if it's the unlabeled metric + if let unlabeledCounter = counterFamily.metricUnlabeled, unlabeledCounter === counter { + // Remove the entirea family since unlabeled metrics are exclusive + store.removeValue(forKey: counter.name) + return + } + + // Check if it's a labeled metric let key = LabelsKey(counter.labels) - guard let existingCounter = counterGroup.metricsByLabelSets[key], + guard let existingCounter = counterFamily.metricsByLabelSets[key], existingCounter === counter else { return } - counterGroup.metricsByLabelSets.removeValue(forKey: key) - if counterGroup.metricsByLabelSets.isEmpty { + counterFamily.metricsByLabelSets.removeValue(forKey: key) + + if counterFamily.metricsByLabelSets.isEmpty { store.removeValue(forKey: counter.name) } else { - store[counter.name] = .counter(counterGroup) + store[counter.name] = .counter(counterFamily) } default: return @@ -758,19 +886,28 @@ public final class PrometheusCollectorRegistry: Sendable { public func unregisterGauge(_ gauge: Gauge) { self.box.withLockedValue { store in switch store[gauge.name] { - case .gauge(var gaugeGroup): + case .gauge(var gaugeFamily): + // Check if it's the unlabeled metric + if let unlabeledGauge = gaugeFamily.metricUnlabeled, unlabeledGauge === gauge { + // Remove the entirea family since unlabeled metrics are exclusive + store.removeValue(forKey: gauge.name) + return + } + + // Check if it's a labeled metric let key = LabelsKey(gauge.labels) - guard let existingGauge = gaugeGroup.metricsByLabelSets[key], + guard let existingGauge = gaugeFamily.metricsByLabelSets[key], existingGauge === gauge else { return } - gaugeGroup.metricsByLabelSets.removeValue(forKey: key) - if gaugeGroup.metricsByLabelSets.isEmpty { + gaugeFamily.metricsByLabelSets.removeValue(forKey: key) + + if gaugeFamily.metricsByLabelSets.isEmpty { store.removeValue(forKey: gauge.name) } else { - store[gauge.name] = .gauge(gaugeGroup) + store[gauge.name] = .gauge(gaugeFamily) } default: return @@ -786,19 +923,28 @@ public final class PrometheusCollectorRegistry: Sendable { public func unregisterDurationHistogram(_ histogram: DurationHistogram) { self.box.withLockedValue { store in switch store[histogram.name] { - case .durationHistogram(var histogramGroup): + case .durationHistogram(var histogramFamily): + // Check if it's the unlabeled metric + if let unlabeledHistogram = histogramFamily.metricUnlabeled, unlabeledHistogram === histogram { + // Remove the entirea family since unlabeled metrics are exclusive + store.removeValue(forKey: histogram.name) + return + } + + // Check if it's a labeled metric let key = LabelsKey(histogram.labels) - guard let existingHistogram = histogramGroup.metricsByLabelSets[key], + guard let existingHistogram = histogramFamily.metricsByLabelSets[key], existingHistogram === histogram else { return } - histogramGroup.metricsByLabelSets.removeValue(forKey: key) - if histogramGroup.metricsByLabelSets.isEmpty { + histogramFamily.metricsByLabelSets.removeValue(forKey: key) + + if histogramFamily.metricsByLabelSets.isEmpty { store.removeValue(forKey: histogram.name) } else { - store[histogram.name] = .durationHistogram(histogramGroup) + store[histogram.name] = .durationHistogram(histogramFamily) } default: return @@ -814,19 +960,28 @@ public final class PrometheusCollectorRegistry: Sendable { public func unregisterValueHistogram(_ histogram: ValueHistogram) { self.box.withLockedValue { store in switch store[histogram.name] { - case .valueHistogram(var histogramGroup): + case .valueHistogram(var histogramFamily): + // Check if it's the unlabeled metric + if let unlabeledHistogram = histogramFamily.metricUnlabeled, unlabeledHistogram === histogram { + // Remove the entirea family since unlabeled metrics are exclusive + store.removeValue(forKey: histogram.name) + return + } + + // Check if it's a labeled metric let key = LabelsKey(histogram.labels) - guard let existingHistogram = histogramGroup.metricsByLabelSets[key], + guard let existingHistogram = histogramFamily.metricsByLabelSets[key], existingHistogram === histogram else { return } - histogramGroup.metricsByLabelSets.removeValue(forKey: key) - if histogramGroup.metricsByLabelSets.isEmpty { + histogramFamily.metricsByLabelSets.removeValue(forKey: key) + + if histogramFamily.metricsByLabelSets.isEmpty { store.removeValue(forKey: histogram.name) } else { - store[histogram.name] = .valueHistogram(histogramGroup) + store[histogram.name] = .valueHistogram(histogramFamily) } default: return @@ -924,59 +1079,75 @@ public final class PrometheusCollectorRegistry: Sendable { let metrics = self.box.withLockedValue { $0 } let prefixHelp = "HELP" let prefixType = "TYPE" - // Emit TYPE/HELP once per metric, then all instances + for (name, metric) in metrics { switch metric { - case .counter(let counterGroup): - guard let _ = counterGroup.metricsByLabelSets.first?.value else { - continue - } - - if let help = counterGroup.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "counter") - for counter in counterGroup.metricsByLabelSets.values { - counter.emit(into: &buffer) - } - - case .gauge(let gaugeGroup): - guard let _ = gaugeGroup.metricsByLabelSets.first?.value else { - continue - } - - if let help = gaugeGroup.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "gauge") - for gauge in gaugeGroup.metricsByLabelSets.values { - gauge.emit(into: &buffer) - } - - case .durationHistogram(let histogramGroup): - guard let _ = histogramGroup.metricsByLabelSets.first?.value else { - continue + case .counter(let counterFamily): + if let unlabeledCounter = counterFamily.metricUnlabeled { + if let help = counterFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "counter") + unlabeledCounter.emit(into: &buffer) + } else if !counterFamily.metricsByLabelSets.isEmpty { + if let help = counterFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "counter") + for counter in counterFamily.metricsByLabelSets.values { + counter.emit(into: &buffer) + } } - if let help = histogramGroup.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "histogram") - for histogram in histogramGroup.metricsByLabelSets.values { - histogram.emit(into: &buffer) + case .gauge(let gaugeFamily): + if let unlabeledGauge = gaugeFamily.metricUnlabeled { + if let help = gaugeFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "gauge") + unlabeledGauge.emit(into: &buffer) + } else if !gaugeFamily.metricsByLabelSets.isEmpty { + if let help = gaugeFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "gauge") + for gauge in gaugeFamily.metricsByLabelSets.values { + gauge.emit(into: &buffer) + } } - case .valueHistogram(let histogramGroup): - guard let _ = histogramGroup.metricsByLabelSets.first?.value else { - continue + case .durationHistogram(let histogramFamily): + if let unlabeledHistogram = histogramFamily.metricUnlabeled { + if let help = histogramFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "histogram") + unlabeledHistogram.emit(into: &buffer) + } else if !histogramFamily.metricsByLabelSets.isEmpty { + if let help = histogramFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "histogram") + for histogram in histogramFamily.metricsByLabelSets.values { + histogram.emit(into: &buffer) + } } - if let help = histogramGroup.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "histogram") - for histogram in histogramGroup.metricsByLabelSets.values { - histogram.emit(into: &buffer) + case .valueHistogram(let histogramFamily): + if let unlabeledHistogram = histogramFamily.metricUnlabeled { + if let help = histogramFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "histogram") + unlabeledHistogram.emit(into: &buffer) + } else if !histogramFamily.metricsByLabelSets.isEmpty { + if let help = histogramFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "histogram") + for histogram in histogramFamily.metricsByLabelSets.values { + histogram.emit(into: &buffer) + } } } } diff --git a/Tests/PrometheusTests/CounterTests.swift b/Tests/PrometheusTests/CounterTests.swift index 15f0553..572aa8f 100644 --- a/Tests/PrometheusTests/CounterTests.swift +++ b/Tests/PrometheusTests/CounterTests.swift @@ -168,12 +168,6 @@ final class CounterTests: XCTestCase { func testCounterWithSharedMetricNamDistinctLabelSets() { let client = PrometheusCollectorRegistry() - let counter0 = client.makeCounter( - name: "foo", - labels: [], - help: "Shared help text" - ) - let counter1 = client.makeCounter( name: "foo", labels: [("bar", "baz")], @@ -187,11 +181,9 @@ final class CounterTests: XCTestCase { ) var buffer = [UInt8]() - counter0.increment() counter1.increment(by: Int64(9)) counter2.increment(by: Int64(4)) counter1.increment(by: Int64(3)) - counter0.increment() counter2.increment(by: Int64(20)) client.emit(into: &buffer) var outputString = String(decoding: buffer, as: Unicode.UTF8.self) @@ -199,7 +191,6 @@ final class CounterTests: XCTestCase { var expectedLines = Set([ "# HELP foo Shared help text", "# TYPE foo counter", - "foo 2", #"foo{bar="baz"} 12"#, @@ -208,19 +199,6 @@ final class CounterTests: XCTestCase { XCTAssertEqual(actualLines, expectedLines) // Counters are unregistered in a cascade. - client.unregisterCounter(counter0) - buffer.removeAll(keepingCapacity: true) - client.emit(into: &buffer) - outputString = String(decoding: buffer, as: Unicode.UTF8.self) - actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) - expectedLines = Set([ - "# HELP foo Shared help text", - "# TYPE foo counter", - #"foo{bar="baz"} 12"#, - - #"foo{bar="newBaz",newKey1="newValue1"} 24"#, - ]) - XCTAssertEqual(actualLines, expectedLines) client.unregisterCounter(counter1) buffer.removeAll(keepingCapacity: true) @@ -361,7 +339,6 @@ final class CounterTests: XCTestCase { // 2. Define the label combinations to test. let labelCases: [(labels: [(String, String)], expectedLabelString: String, description: String)] = [ - (labels: [], expectedLabelString: "", description: "without labels"), (labels: [("method", "get")], expectedLabelString: "{method=\"get\"}", description: "with one label"), ( labels: [("status", "200"), ("path", "/api/v1")], diff --git a/Tests/PrometheusTests/GaugeTests.swift b/Tests/PrometheusTests/GaugeTests.swift index a8de29e..81e8762 100644 --- a/Tests/PrometheusTests/GaugeTests.swift +++ b/Tests/PrometheusTests/GaugeTests.swift @@ -155,12 +155,6 @@ final class GaugeTests: XCTestCase { func testGaugeWithSharedMetricNameDistinctLabelSets() { let client = PrometheusCollectorRegistry() - let gauge0 = client.makeGauge( - name: "foo", - labels: [], - help: "Shared help text" - ) - let gauge1 = client.makeGauge( name: "foo", labels: [("bar", "baz")], @@ -174,11 +168,9 @@ final class GaugeTests: XCTestCase { ) var buffer = [UInt8]() - gauge0.set(to: 1.0) gauge1.set(to: 9.0) gauge2.set(to: 4.0) gauge1.decrement(by: 12.0) - gauge0.record(2.0) gauge2.increment(by: 24.0) client.emit(into: &buffer) var outputString = String(decoding: buffer, as: Unicode.UTF8.self) @@ -186,7 +178,6 @@ final class GaugeTests: XCTestCase { var expectedLines = Set([ "# HELP foo Shared help text", "# TYPE foo gauge", - "foo 2.0", #"foo{bar="baz"} -3.0"#, @@ -195,19 +186,6 @@ final class GaugeTests: XCTestCase { XCTAssertEqual(actualLines, expectedLines) // Gauges are unregistered in a cascade. - client.unregisterGauge(gauge0) - buffer.removeAll(keepingCapacity: true) - client.emit(into: &buffer) - outputString = String(decoding: buffer, as: Unicode.UTF8.self) - actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) - expectedLines = Set([ - "# HELP foo Shared help text", - "# TYPE foo gauge", - #"foo{bar="baz"} -3.0"#, - - #"foo{bar="newBaz",newKey1="newValue1"} 28.0"#, - ]) - XCTAssertEqual(actualLines, expectedLines) client.unregisterGauge(gauge1) buffer.removeAll(keepingCapacity: true) @@ -372,7 +350,6 @@ final class GaugeTests: XCTestCase { // 2. Define the label combinations to test. let labelCases: [(labels: [(String, String)], expectedLabelString: String, description: String)] = [ - (labels: [], expectedLabelString: "", description: "without labels"), (labels: [("method", "get")], expectedLabelString: "{method=\"get\"}", description: "with one label"), ( labels: [("status", "200"), ("path", "/api/v1")], diff --git a/Tests/PrometheusTests/HistogramTests.swift b/Tests/PrometheusTests/HistogramTests.swift index 32c8672..a15c080 100644 --- a/Tests/PrometheusTests/HistogramTests.swift +++ b/Tests/PrometheusTests/HistogramTests.swift @@ -381,13 +381,6 @@ final class HistogramTests: XCTestCase { .seconds(1), ] - let histogram0 = client.makeDurationHistogram( - name: "foo", - labels: [], - buckets: sharedBuckets, - help: "Shared help text" - ) - let histogram1 = client.makeDurationHistogram( name: "foo", labels: [("bar", "baz")], @@ -403,11 +396,9 @@ final class HistogramTests: XCTestCase { ) var buffer = [UInt8]() - histogram0.recordNanoseconds(300_000_000) // 300ms histogram1.recordNanoseconds(600_000_000) // 600ms histogram2.recordNanoseconds(150_000_000) // 150ms histogram1.recordNanoseconds(1_500_000_000) // 1500ms - histogram0.recordNanoseconds(800_000_000) // 800ms histogram2.recordNanoseconds(100_000_000) // 100ms client.emit(into: &buffer) @@ -416,12 +407,6 @@ final class HistogramTests: XCTestCase { var expectedLines = Set([ "# HELP foo Shared help text", "# TYPE foo histogram", - "foo_bucket{le=\"0.1\"} 0", - "foo_bucket{le=\"0.5\"} 1", - "foo_bucket{le=\"1.0\"} 2", - "foo_bucket{le=\"+Inf\"} 2", - "foo_sum 1.1", - "foo_count 2", #"foo_bucket{bar="baz",le="0.1"} 0"#, #"foo_bucket{bar="baz",le="0.5"} 0"#, @@ -440,29 +425,6 @@ final class HistogramTests: XCTestCase { XCTAssertEqual(actualLines, expectedLines) // Histograms are unregistered in a cascade. - client.unregisterDurationHistogram(histogram0) - buffer.removeAll(keepingCapacity: true) - client.emit(into: &buffer) - outputString = String(decoding: buffer, as: Unicode.UTF8.self) - actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) - expectedLines = Set([ - "# HELP foo Shared help text", - "# TYPE foo histogram", - #"foo_bucket{bar="baz",le="0.1"} 0"#, - #"foo_bucket{bar="baz",le="0.5"} 0"#, - #"foo_bucket{bar="baz",le="1.0"} 1"#, - #"foo_bucket{bar="baz",le="+Inf"} 2"#, - #"foo_sum{bar="baz"} 2.1"#, - #"foo_count{bar="baz"} 2"#, - - #"foo_bucket{bar="newBaz",newKey1="newValue1",le="0.1"} 1"#, - #"foo_bucket{bar="newBaz",newKey1="newValue1",le="0.5"} 2"#, - #"foo_bucket{bar="newBaz",newKey1="newValue1",le="1.0"} 2"#, - #"foo_bucket{bar="newBaz",newKey1="newValue1",le="+Inf"} 2"#, - #"foo_sum{bar="newBaz",newKey1="newValue1"} 0.25"#, - #"foo_count{bar="newBaz",newKey1="newValue1"} 2"#, - ]) - XCTAssertEqual(actualLines, expectedLines) client.unregisterDurationHistogram(histogram1) buffer.removeAll(keepingCapacity: true) @@ -515,13 +477,6 @@ final class HistogramTests: XCTestCase { 1.0, 5.0, 10.0, ] - let histogram0 = client.makeValueHistogram( - name: "foo", - labels: [], - buckets: sharedBuckets, - help: "Shared help text" - ) - let histogram1 = client.makeValueHistogram( name: "foo", labels: [("bar", "baz")], @@ -537,11 +492,9 @@ final class HistogramTests: XCTestCase { ) var buffer = [UInt8]() - histogram0.record(3.0) histogram1.record(6.0) histogram2.record(2.0) histogram1.record(12.0) - histogram0.record(8.0) histogram2.record(1.5) client.emit(into: &buffer) @@ -550,12 +503,6 @@ final class HistogramTests: XCTestCase { var expectedLines = Set([ "# HELP foo Shared help text", "# TYPE foo histogram", - "foo_bucket{le=\"1.0\"} 0", - "foo_bucket{le=\"5.0\"} 1", - "foo_bucket{le=\"10.0\"} 2", - "foo_bucket{le=\"+Inf\"} 2", - "foo_sum 11.0", - "foo_count 2", #"foo_bucket{bar="baz",le="1.0"} 0"#, #"foo_bucket{bar="baz",le="5.0"} 0"#, @@ -574,30 +521,6 @@ final class HistogramTests: XCTestCase { XCTAssertEqual(actualLines, expectedLines) // Histograms are unregistered in a cascade. - client.unregisterValueHistogram(histogram0) - buffer.removeAll(keepingCapacity: true) - client.emit(into: &buffer) - outputString = String(decoding: buffer, as: Unicode.UTF8.self) - actualLines = Set(outputString.components(separatedBy: .newlines).filter { !$0.isEmpty }) - expectedLines = Set([ - "# HELP foo Shared help text", - "# TYPE foo histogram", - #"foo_bucket{bar="baz",le="1.0"} 0"#, - #"foo_bucket{bar="baz",le="5.0"} 0"#, - #"foo_bucket{bar="baz",le="10.0"} 1"#, - #"foo_bucket{bar="baz",le="+Inf"} 2"#, - #"foo_sum{bar="baz"} 18.0"#, - #"foo_count{bar="baz"} 2"#, - - #"foo_bucket{bar="newBaz",newKey1="newValue1",le="1.0"} 0"#, - #"foo_bucket{bar="newBaz",newKey1="newValue1",le="5.0"} 2"#, - #"foo_bucket{bar="newBaz",newKey1="newValue1",le="10.0"} 2"#, - #"foo_bucket{bar="newBaz",newKey1="newValue1",le="+Inf"} 2"#, - #"foo_sum{bar="newBaz",newKey1="newValue1"} 3.5"#, - #"foo_count{bar="newBaz",newKey1="newValue1"} 2"#, - ]) - XCTAssertEqual(actualLines, expectedLines) - client.unregisterValueHistogram(histogram1) buffer.removeAll(keepingCapacity: true) client.emit(into: &buffer) @@ -745,7 +668,6 @@ final class HistogramTests: XCTestCase { // 2. Define the label combinations to test. let labelCases: [(labels: [(String, String)], expectedLabelString: String, description: String)] = [ - (labels: [], expectedLabelString: "", description: "without labels"), (labels: [("method", "get")], expectedLabelString: "{method=\"get\"}", description: "with one label"), ( labels: [("status", "200"), ("path", "/api/v1")], @@ -967,7 +889,6 @@ final class HistogramTests: XCTestCase { // 2. Define the label combinations to test. let labelCases: [(labels: [(String, String)], expectedLabelString: String, description: String)] = [ - (labels: [], expectedLabelString: "", description: "without labels"), (labels: [("method", "get")], expectedLabelString: "{method=\"get\"}", description: "with one label"), ( labels: [("status", "200"), ("path", "/api/v1")], diff --git a/Tests/PrometheusTests/PrometheusCollectorRegistryTests.swift b/Tests/PrometheusTests/PrometheusCollectorRegistryTests.swift index e6988b0..c70d641 100644 --- a/Tests/PrometheusTests/PrometheusCollectorRegistryTests.swift +++ b/Tests/PrometheusTests/PrometheusCollectorRegistryTests.swift @@ -439,15 +439,9 @@ final class PrometheusCollectorRegistryTests: XCTestCase { XCTAssertEqual(emptyCapacity, 0) } - func testDefaultRegistryDedupTypeHelpPerMetricNameOnEmitWhenMetricNameSharedInMetricGroup() { + func testDefaultRegistryDedupTypeHelpPerMetricNameOnEmitWhenMetricNameSharedInMetricFamily() { let client = PrometheusCollectorRegistry() - let gauge0 = client.makeGauge( - name: "foo", - labels: [], - help: "Shared help text for all variants" - ) - let gauge1 = client.makeGauge( name: "foo", labels: [("bar", "baz")], @@ -460,7 +454,6 @@ final class PrometheusCollectorRegistryTests: XCTestCase { help: "Shared help text for all variants" ) - gauge0.set(to: 1.0) gauge1.set(to: 9.0) gauge2.set(to: 4.0) @@ -476,7 +469,7 @@ final class PrometheusCollectorRegistryTests: XCTestCase { XCTAssertEqual(helpLines.count, 1, "Should have exactly one HELP line") XCTAssertEqual(typeLines.count, 1, "Should have exactly one TYPE line") - XCTAssertEqual(metricLines.count, 3, "Should have three metric value lines") + XCTAssertEqual(metricLines.count, 2, "Should have three metric value lines") XCTAssertEqual(helpLines.first, "# HELP foo Shared help text for all variants") XCTAssertEqual(typeLines.first, "# TYPE foo gauge") @@ -490,7 +483,6 @@ final class PrometheusCollectorRegistryTests: XCTestCase { XCTAssertLessThan(typeIndex, firstMetricIndex, "TYPE should appear before metric values") // Verify all three metric values are present (order doesn't matter). - XCTAssertTrue(metricLines.contains("foo 1.0")) XCTAssertTrue(metricLines.contains(#"foo{bar="baz"} 9.0"#)) XCTAssertTrue(metricLines.contains(#"foo{bar="newBaz",newKey1="newValue1"} 4.0"#)) } From 5ceadc54b6da460a15a7553566bcd55fdf1dc657 Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Thu, 28 Aug 2025 21:56:18 -0700 Subject: [PATCH 3/4] update: apply reviewers suggestions Move constraint checks into MetricFamily and adopt an approach that favors immutability of MetricFamily members. Co-authored-by: Konrad `ktoso` Malawski Signed-off-by: Melissa Kilby --- .../PrometheusCollectorRegistry.swift | 239 +++++++++--------- 1 file changed, 116 insertions(+), 123 deletions(-) diff --git a/Sources/Prometheus/PrometheusCollectorRegistry.swift b/Sources/Prometheus/PrometheusCollectorRegistry.swift index c984083..569ec4f 100644 --- a/Sources/Prometheus/PrometheusCollectorRegistry.swift +++ b/Sources/Prometheus/PrometheusCollectorRegistry.swift @@ -60,21 +60,72 @@ public final class PrometheusCollectorRegistry: Sendable { /// For histograms, the buckets are immutable for a MetricFamily once initialized with the first /// metric. See also https://github.com/prometheus/OpenMetrics/issues/197. private struct MetricFamily: Sendable { - var metricsByLabelSets: [LabelsKey: Metric] + private enum State { + case labeled([LabelsKey: Metric]) + case unlabeled(Metric) + case empty + } + + let metricsByLabelSets: [LabelsKey: Metric] let buckets: HistogramBuckets? let help: String? let metricUnlabeled: Metric? + private let state: State + init( metricsByLabelSets: [LabelsKey: Metric] = [:], buckets: HistogramBuckets? = nil, help: String? = nil, metricUnlabeled: Metric? = nil ) { + // Validate mutual exclusivity on creation. + if metricUnlabeled != nil && !metricsByLabelSets.isEmpty { + fatalError("Cannot have both labeled and unlabeled metrics in the same family.") + } + self.metricsByLabelSets = metricsByLabelSets self.buckets = buckets self.help = help self.metricUnlabeled = metricUnlabeled + + // Set internal state for validation. + if let unlabeled = metricUnlabeled { + self.state = .unlabeled(unlabeled) + } else if !metricsByLabelSets.isEmpty { + self.state = .labeled(metricsByLabelSets) + } else { + self.state = .empty + } + } + + func add(metric: Metric, for labels: [(String, String)]) -> MetricFamily { + // This method should only be called for labeled metrics. + guard !labels.isEmpty else { + fatalError("Use initializer for unlabeled metrics, not add method") + } + + switch state { + case .unlabeled: + fatalError("Cannot register a labeled metric when an unlabeled metric already exists.") + case .labeled, .empty: + break + } + + let labelsKey = LabelsKey(labels) + guard metricsByLabelSets[labelsKey] == nil else { + return self + } + + var newMetricsByLabelSets = metricsByLabelSets + newMetricsByLabelSets[labelsKey] = metric + + return MetricFamily( + metricsByLabelSets: newMetricsByLabelSets, + buckets: buckets, + help: help, + metricUnlabeled: metricUnlabeled + ) } } @@ -162,27 +213,7 @@ public final class PrometheusCollectorRegistry: Sendable { } switch entry { - case .counter(var existingCounterFamily): - // Check mutual exclusivity constraints. While Prometheus wouldn't break with sharing a - // metric name for labeled and unlabeled variants, the client enforces that each unique - // metric name must either be labeled (allowing multiple label sets) or unlabeled - // in order to support correct Prometheus aggregations. - if existingCounterFamily.metricUnlabeled != nil && !labels.isEmpty { - fatalError( - """ - Label conflict for metric '\(name)': - Cannot register a labeled metric when an unlabeled metric already exists. - """ - ) - } - if labels.isEmpty && !existingCounterFamily.metricsByLabelSets.isEmpty { - fatalError( - """ - Label conflict for metric '\(name)': - Cannot register an unlabeled metric when labeled metrics already exist. - """ - ) - } + case .counter(let existingCounterFamily): // For unlabeled metrics, return the existing one if it exists. if labels.isEmpty, let existingUnlabeled = existingCounterFamily.metricUnlabeled { @@ -208,10 +239,10 @@ public final class PrometheusCollectorRegistry: Sendable { // Even if the metric name is identical, each label set defines a unique time series. let counter = Counter(name: name, labels: labels) - existingCounterFamily.metricsByLabelSets[key] = counter + let updatedFamily = existingCounterFamily.add(metric: counter, for: labels) // Write the modified entry back to the store. - store[name] = .counter(existingCounterFamily) + store[name] = .counter(updatedFamily) return counter @@ -323,27 +354,7 @@ public final class PrometheusCollectorRegistry: Sendable { } switch entry { - case .gauge(var existingGaugeFamily): - // Check mutual exclusivity constraints. While Prometheus wouldn't break with sharing a - // metric name for labeled and unlabeled variants, the client enforces that each unique - // metric name must either be labeled (allowing multiple label sets) or unlabeled - // in order to support correct Prometheus aggregations. - if existingGaugeFamily.metricUnlabeled != nil && !labels.isEmpty { - fatalError( - """ - Label conflict for metric '\(name)': - Cannot register a labeled metric when an unlabeled metric already exists. - """ - ) - } - if labels.isEmpty && !existingGaugeFamily.metricsByLabelSets.isEmpty { - fatalError( - """ - Label conflict for metric '\(name)': - Cannot register an unlabeled metric when labeled metrics already exist. - """ - ) - } + case .gauge(let existingGaugeFamily): // For unlabeled metrics, return the existing one if it exists. if labels.isEmpty, let existingUnlabeled = existingGaugeFamily.metricUnlabeled { @@ -369,10 +380,10 @@ public final class PrometheusCollectorRegistry: Sendable { // Even if the metric name is identical, each label set defines a unique time series. let gauge = Gauge(name: name, labels: labels) - existingGaugeFamily.metricsByLabelSets[key] = gauge + let updatedFamily = existingGaugeFamily.add(metric: gauge, for: labels) // Write the modified entry back to the store. - store[name] = .gauge(existingGaugeFamily) + store[name] = .gauge(updatedFamily) return gauge @@ -499,28 +510,7 @@ public final class PrometheusCollectorRegistry: Sendable { } switch entry { - case .durationHistogram(var existingHistogramFamily): - // Check mutual exclusivity constraints. While Prometheus wouldn't break with sharing a - // metric name for labeled and unlabeled variants, the client enforces that each unique - // metric name must either be labeled (allowing multiple label sets) or unlabeled - // in order to support correct Prometheus aggregations. - if existingHistogramFamily.metricUnlabeled != nil && !labels.isEmpty { - fatalError( - """ - Label conflict for metric '\(name)': - Cannot register a labeled metric when an unlabeled metric already exists. - """ - ) - } - if labels.isEmpty && !existingHistogramFamily.metricsByLabelSets.isEmpty { - fatalError( - """ - Label conflict for metric '\(name)': - Cannot register an unlabeled metric when labeled metrics already exist. - """ - ) - } - + case .durationHistogram(let existingHistogramFamily): // Validate buckets match the stored ones. if case .duration(let storedBuckets) = existingHistogramFamily.buckets { guard storedBuckets == buckets else { @@ -559,10 +549,10 @@ public final class PrometheusCollectorRegistry: Sendable { // Even if the metric name is identical, each label set defines a unique time series. let histogram = DurationHistogram(name: name, labels: labels, buckets: buckets) - existingHistogramFamily.metricsByLabelSets[key] = histogram + let updatedFamily = existingHistogramFamily.add(metric: histogram, for: labels) // Write the modified entry back to the store. - store[name] = .durationHistogram(existingHistogramFamily) + store[name] = .durationHistogram(updatedFamily) return histogram @@ -709,28 +699,7 @@ public final class PrometheusCollectorRegistry: Sendable { } switch entry { - case .valueHistogram(var existingHistogramFamily): - // Check mutual exclusivity constraints. While Prometheus wouldn't break with sharing a - // metric name for labeled and unlabeled variants, the client enforces that each unique - // metric name must either be labeled (allowing multiple label sets) or unlabeled - // in order to support correct Prometheus aggregations. - if existingHistogramFamily.metricUnlabeled != nil && !labels.isEmpty { - fatalError( - """ - Label conflict for metric '\(name)': - Cannot register a labeled metric when an unlabeled metric already exists. - """ - ) - } - if labels.isEmpty && !existingHistogramFamily.metricsByLabelSets.isEmpty { - fatalError( - """ - Label conflict for metric '\(name)': - Cannot register an unlabeled metric when labeled metrics already exist. - """ - ) - } - + case .valueHistogram(let existingHistogramFamily): // Validate buckets match the stored ones. if case .value(let storedBuckets) = existingHistogramFamily.buckets { guard storedBuckets == buckets else { @@ -769,10 +738,10 @@ public final class PrometheusCollectorRegistry: Sendable { // Even if the metric name is identical, each label set defines a unique time series. let histogram = ValueHistogram(name: name, labels: labels, buckets: buckets) - existingHistogramFamily.metricsByLabelSets[key] = histogram + let updatedFamily = existingHistogramFamily.add(metric: histogram, for: labels) // Write the modified entry back to the store. - store[name] = .valueHistogram(existingHistogramFamily) + store[name] = .valueHistogram(updatedFamily) return histogram @@ -849,15 +818,14 @@ public final class PrometheusCollectorRegistry: Sendable { public func unregisterCounter(_ counter: Counter) { self.box.withLockedValue { store in switch store[counter.name] { - case .counter(var counterFamily): - // Check if it's the unlabeled metric + case .counter(let counterFamily): + if let unlabeledCounter = counterFamily.metricUnlabeled, unlabeledCounter === counter { - // Remove the entirea family since unlabeled metrics are exclusive + // Remove the entire family since unlabeled metrics are exclusive. store.removeValue(forKey: counter.name) return } - // Check if it's a labeled metric let key = LabelsKey(counter.labels) guard let existingCounter = counterFamily.metricsByLabelSets[key], existingCounter === counter @@ -865,12 +833,19 @@ public final class PrometheusCollectorRegistry: Sendable { return } - counterFamily.metricsByLabelSets.removeValue(forKey: key) + var newMetricsByLabelSets = counterFamily.metricsByLabelSets + newMetricsByLabelSets.removeValue(forKey: key) - if counterFamily.metricsByLabelSets.isEmpty { + if newMetricsByLabelSets.isEmpty { store.removeValue(forKey: counter.name) } else { - store[counter.name] = .counter(counterFamily) + let updatedFamily = MetricFamily( + metricsByLabelSets: newMetricsByLabelSets, + buckets: counterFamily.buckets, + help: counterFamily.help, + metricUnlabeled: counterFamily.metricUnlabeled + ) + store[counter.name] = .counter(updatedFamily) } default: return @@ -886,15 +861,14 @@ public final class PrometheusCollectorRegistry: Sendable { public func unregisterGauge(_ gauge: Gauge) { self.box.withLockedValue { store in switch store[gauge.name] { - case .gauge(var gaugeFamily): - // Check if it's the unlabeled metric + case .gauge(let gaugeFamily): + if let unlabeledGauge = gaugeFamily.metricUnlabeled, unlabeledGauge === gauge { - // Remove the entirea family since unlabeled metrics are exclusive + // Remove the entire family since unlabeled metrics are exclusive. store.removeValue(forKey: gauge.name) return } - // Check if it's a labeled metric let key = LabelsKey(gauge.labels) guard let existingGauge = gaugeFamily.metricsByLabelSets[key], existingGauge === gauge @@ -902,12 +876,19 @@ public final class PrometheusCollectorRegistry: Sendable { return } - gaugeFamily.metricsByLabelSets.removeValue(forKey: key) + var newMetricsByLabelSets = gaugeFamily.metricsByLabelSets + newMetricsByLabelSets.removeValue(forKey: key) - if gaugeFamily.metricsByLabelSets.isEmpty { + if newMetricsByLabelSets.isEmpty { store.removeValue(forKey: gauge.name) } else { - store[gauge.name] = .gauge(gaugeFamily) + let updatedFamily = MetricFamily( + metricsByLabelSets: newMetricsByLabelSets, + buckets: gaugeFamily.buckets, + help: gaugeFamily.help, + metricUnlabeled: gaugeFamily.metricUnlabeled + ) + store[gauge.name] = .gauge(updatedFamily) } default: return @@ -923,15 +904,14 @@ public final class PrometheusCollectorRegistry: Sendable { public func unregisterDurationHistogram(_ histogram: DurationHistogram) { self.box.withLockedValue { store in switch store[histogram.name] { - case .durationHistogram(var histogramFamily): - // Check if it's the unlabeled metric + case .durationHistogram(let histogramFamily): + if let unlabeledHistogram = histogramFamily.metricUnlabeled, unlabeledHistogram === histogram { - // Remove the entirea family since unlabeled metrics are exclusive + // Remove the entire family since unlabeled metrics are exclusive. store.removeValue(forKey: histogram.name) return } - // Check if it's a labeled metric let key = LabelsKey(histogram.labels) guard let existingHistogram = histogramFamily.metricsByLabelSets[key], existingHistogram === histogram @@ -939,12 +919,19 @@ public final class PrometheusCollectorRegistry: Sendable { return } - histogramFamily.metricsByLabelSets.removeValue(forKey: key) + var newMetricsByLabelSets = histogramFamily.metricsByLabelSets + newMetricsByLabelSets.removeValue(forKey: key) - if histogramFamily.metricsByLabelSets.isEmpty { + if newMetricsByLabelSets.isEmpty { store.removeValue(forKey: histogram.name) } else { - store[histogram.name] = .durationHistogram(histogramFamily) + let updatedFamily = MetricFamily( + metricsByLabelSets: newMetricsByLabelSets, + buckets: histogramFamily.buckets, + help: histogramFamily.help, + metricUnlabeled: histogramFamily.metricUnlabeled + ) + store[histogram.name] = .durationHistogram(updatedFamily) } default: return @@ -960,15 +947,14 @@ public final class PrometheusCollectorRegistry: Sendable { public func unregisterValueHistogram(_ histogram: ValueHistogram) { self.box.withLockedValue { store in switch store[histogram.name] { - case .valueHistogram(var histogramFamily): - // Check if it's the unlabeled metric + case .valueHistogram(let histogramFamily): + if let unlabeledHistogram = histogramFamily.metricUnlabeled, unlabeledHistogram === histogram { - // Remove the entirea family since unlabeled metrics are exclusive + // Remove the entire family since unlabeled metrics are exclusive. store.removeValue(forKey: histogram.name) return } - // Check if it's a labeled metric let key = LabelsKey(histogram.labels) guard let existingHistogram = histogramFamily.metricsByLabelSets[key], existingHistogram === histogram @@ -976,12 +962,19 @@ public final class PrometheusCollectorRegistry: Sendable { return } - histogramFamily.metricsByLabelSets.removeValue(forKey: key) + var newMetricsByLabelSets = histogramFamily.metricsByLabelSets + newMetricsByLabelSets.removeValue(forKey: key) - if histogramFamily.metricsByLabelSets.isEmpty { + if newMetricsByLabelSets.isEmpty { store.removeValue(forKey: histogram.name) } else { - store[histogram.name] = .valueHistogram(histogramFamily) + let updatedFamily = MetricFamily( + metricsByLabelSets: newMetricsByLabelSets, + buckets: histogramFamily.buckets, + help: histogramFamily.help, + metricUnlabeled: histogramFamily.metricUnlabeled + ) + store[histogram.name] = .valueHistogram(updatedFamily) } default: return From 2dd37a054db9692755764627ff0e39758d4beb2b Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Fri, 29 Aug 2025 10:11:45 -0700 Subject: [PATCH 4/4] update: apply reviewers suggestions Reduce code duplication further by adding `removing`, `getMetric` and `forEachMetric` to `MetricFamily`. Rename `add` -> `adding` to indicate non mutating nature (Swift convention). Co-authored-by: Konrad `ktoso` Malawski Signed-off-by: Melissa Kilby --- .../PrometheusCollectorRegistry.swift | 403 +++++++----------- 1 file changed, 161 insertions(+), 242 deletions(-) diff --git a/Sources/Prometheus/PrometheusCollectorRegistry.swift b/Sources/Prometheus/PrometheusCollectorRegistry.swift index 569ec4f..86f2990 100644 --- a/Sources/Prometheus/PrometheusCollectorRegistry.swift +++ b/Sources/Prometheus/PrometheusCollectorRegistry.swift @@ -66,11 +66,8 @@ public final class PrometheusCollectorRegistry: Sendable { case empty } - let metricsByLabelSets: [LabelsKey: Metric] let buckets: HistogramBuckets? let help: String? - let metricUnlabeled: Metric? - private let state: State init( @@ -84,12 +81,10 @@ public final class PrometheusCollectorRegistry: Sendable { fatalError("Cannot have both labeled and unlabeled metrics in the same family.") } - self.metricsByLabelSets = metricsByLabelSets self.buckets = buckets self.help = help - self.metricUnlabeled = metricUnlabeled - // Set internal state for validation. + // Set internal state based on inputs. if let unlabeled = metricUnlabeled { self.state = .unlabeled(unlabeled) } else if !metricsByLabelSets.isEmpty { @@ -99,33 +94,96 @@ public final class PrometheusCollectorRegistry: Sendable { } } - func add(metric: Metric, for labels: [(String, String)]) -> MetricFamily { - // This method should only be called for labeled metrics. + func adding(metric: Metric, for labels: [(String, String)]) -> MetricFamily { guard !labels.isEmpty else { - fatalError("Use initializer for unlabeled metrics, not add method") + fatalError("Use initializer for unlabeled metrics, not adding method") } switch state { case .unlabeled: fatalError("Cannot register a labeled metric when an unlabeled metric already exists.") - case .labeled, .empty: - break + case .labeled(let existingMetrics): + let labelsKey = LabelsKey(labels) + guard existingMetrics[labelsKey] == nil else { + return self + } + + var newMetricsByLabelSets = existingMetrics + newMetricsByLabelSets[labelsKey] = metric + + return MetricFamily( + metricsByLabelSets: newMetricsByLabelSets, + buckets: buckets, + help: help, + metricUnlabeled: nil + ) + case .empty: + let labelsKey = LabelsKey(labels) + return MetricFamily( + metricsByLabelSets: [labelsKey: metric], + buckets: buckets, + help: help, + metricUnlabeled: nil + ) } + } - let labelsKey = LabelsKey(labels) - guard metricsByLabelSets[labelsKey] == nil else { - return self + func removing(metric: Metric, for labels: [(String, String)]) -> MetricFamily? { + switch state { + case .unlabeled(let unlabeledMetric): + if labels.isEmpty && unlabeledMetric === metric { + return nil // Remove entire family. + } + return self // Metric not found, return unchanged. + + case .labeled(let labeledMetrics): + let key = LabelsKey(labels) + guard let existingMetric = labeledMetrics[key], + existingMetric === metric + else { + return self // Metric not found, return unchanged. + } + + var newMetricsByLabelSets = labeledMetrics + newMetricsByLabelSets.removeValue(forKey: key) + + guard newMetricsByLabelSets.isEmpty else { + return MetricFamily( + metricsByLabelSets: newMetricsByLabelSets, + buckets: buckets, + help: help, + metricUnlabeled: nil + ) + } + return nil // Remove entire family. + + case .empty: + return self // Nothing to remove. } + } - var newMetricsByLabelSets = metricsByLabelSets - newMetricsByLabelSets[labelsKey] = metric + func getMetric(for labels: [(String, String)]) -> Metric? { + switch state { + case .labeled(let metrics): + return metrics[LabelsKey(labels)] + case .unlabeled(let metric): + return labels.isEmpty ? metric : nil + case .empty: + return nil + } + } - return MetricFamily( - metricsByLabelSets: newMetricsByLabelSets, - buckets: buckets, - help: help, - metricUnlabeled: metricUnlabeled - ) + func forEachMetric(_ closure: (Metric) -> Void) { + switch state { + case .unlabeled(let metric): + closure(metric) + case .labeled(let metrics): + for metric in metrics.values { + closure(metric) + } + case .empty: + break + } } } @@ -215,15 +273,6 @@ public final class PrometheusCollectorRegistry: Sendable { switch entry { case .counter(let existingCounterFamily): - // For unlabeled metrics, return the existing one if it exists. - if labels.isEmpty, let existingUnlabeled = existingCounterFamily.metricUnlabeled { - return existingUnlabeled - } - - if let existingCounter = existingCounterFamily.metricsByLabelSets[key] { - return existingCounter - } - // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct // HELP lines, the client enforces uniqueness and consistency. if let existingHelp = existingCounterFamily.help, existingHelp != help { @@ -237,9 +286,13 @@ public final class PrometheusCollectorRegistry: Sendable { ) } + if let existingCounter = existingCounterFamily.getMetric(for: labels) { + return existingCounter + } + // Even if the metric name is identical, each label set defines a unique time series. let counter = Counter(name: name, labels: labels) - let updatedFamily = existingCounterFamily.add(metric: counter, for: labels) + let updatedFamily = existingCounterFamily.adding(metric: counter, for: labels) // Write the modified entry back to the store. store[name] = .counter(updatedFamily) @@ -356,15 +409,6 @@ public final class PrometheusCollectorRegistry: Sendable { switch entry { case .gauge(let existingGaugeFamily): - // For unlabeled metrics, return the existing one if it exists. - if labels.isEmpty, let existingUnlabeled = existingGaugeFamily.metricUnlabeled { - return existingUnlabeled - } - - if let existingGauge = existingGaugeFamily.metricsByLabelSets[key] { - return existingGauge - } - // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct // HELP lines, the client enforces uniqueness and consistency. if let existingHelp = existingGaugeFamily.help, existingHelp != help { @@ -378,9 +422,13 @@ public final class PrometheusCollectorRegistry: Sendable { ) } + if let existingGauge = existingGaugeFamily.getMetric(for: labels) { + return existingGauge + } + // Even if the metric name is identical, each label set defines a unique time series. let gauge = Gauge(name: name, labels: labels) - let updatedFamily = existingGaugeFamily.add(metric: gauge, for: labels) + let updatedFamily = existingGaugeFamily.adding(metric: gauge, for: labels) // Write the modified entry back to the store. store[name] = .gauge(updatedFamily) @@ -511,6 +559,20 @@ public final class PrometheusCollectorRegistry: Sendable { switch entry { case .durationHistogram(let existingHistogramFamily): + + // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct + // HELP lines, the client enforces uniqueness and consistency. + if let existingHelp = existingHistogramFamily.help, existingHelp != help { + fatalError( + """ + Help text mismatch for metric '\(name)': + Existing help: '\(existingHelp)' + New help: '\(help)' + All metrics with the same name must have identical help text. + """ + ) + } + // Validate buckets match the stored ones. if case .duration(let storedBuckets) = existingHistogramFamily.buckets { guard storedBuckets == buckets else { @@ -525,31 +587,13 @@ public final class PrometheusCollectorRegistry: Sendable { } } - // For unlabeled metrics, return the existing one if it exists. - if labels.isEmpty, let existingUnlabeled = existingHistogramFamily.metricUnlabeled { - return existingUnlabeled - } - - if let existingHistogram = existingHistogramFamily.metricsByLabelSets[key] { + if let existingHistogram = existingHistogramFamily.getMetric(for: labels) { return existingHistogram } - // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct - // HELP lines, the client enforces uniqueness and consistency. - if let existingHelp = existingHistogramFamily.help, existingHelp != help { - fatalError( - """ - Help text mismatch for metric '\(name)': - Existing help: '\(existingHelp)' - New help: '\(help)' - All metrics with the same name must have identical help text. - """ - ) - } - // Even if the metric name is identical, each label set defines a unique time series. let histogram = DurationHistogram(name: name, labels: labels, buckets: buckets) - let updatedFamily = existingHistogramFamily.add(metric: histogram, for: labels) + let updatedFamily = existingHistogramFamily.adding(metric: histogram, for: labels) // Write the modified entry back to the store. store[name] = .durationHistogram(updatedFamily) @@ -700,6 +744,19 @@ public final class PrometheusCollectorRegistry: Sendable { switch entry { case .valueHistogram(let existingHistogramFamily): + + // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct + // HELP lines, the client enforces uniqueness and consistency. + if let existingHelp = existingHistogramFamily.help, existingHelp != help { + fatalError( + """ + Help text mismatch for metric '\(name)': + Existing help: '\(existingHelp)' + New help: '\(help)' + All metrics with the same name must have identical help text. + """ + ) + } // Validate buckets match the stored ones. if case .value(let storedBuckets) = existingHistogramFamily.buckets { guard storedBuckets == buckets else { @@ -714,31 +771,13 @@ public final class PrometheusCollectorRegistry: Sendable { } } - // For unlabeled metrics, return the existing one if it exists. - if labels.isEmpty, let existingUnlabeled = existingHistogramFamily.metricUnlabeled { - return existingUnlabeled - } - - if let existingHistogram = existingHistogramFamily.metricsByLabelSets[key] { + if let existingHistogram = existingHistogramFamily.getMetric(for: labels) { return existingHistogram } - // Validate help text consistency. While Prometheus wouldn't break with duplicate and distinct - // HELP lines, the client enforces uniqueness and consistency. - if let existingHelp = existingHistogramFamily.help, existingHelp != help { - fatalError( - """ - Help text mismatch for metric '\(name)': - Existing help: '\(existingHelp)' - New help: '\(help)' - All metrics with the same name must have identical help text. - """ - ) - } - // Even if the metric name is identical, each label set defines a unique time series. let histogram = ValueHistogram(name: name, labels: labels, buckets: buckets) - let updatedFamily = existingHistogramFamily.add(metric: histogram, for: labels) + let updatedFamily = existingHistogramFamily.adding(metric: histogram, for: labels) // Write the modified entry back to the store. store[name] = .valueHistogram(updatedFamily) @@ -819,33 +858,11 @@ public final class PrometheusCollectorRegistry: Sendable { self.box.withLockedValue { store in switch store[counter.name] { case .counter(let counterFamily): - - if let unlabeledCounter = counterFamily.metricUnlabeled, unlabeledCounter === counter { - // Remove the entire family since unlabeled metrics are exclusive. - store.removeValue(forKey: counter.name) - return - } - - let key = LabelsKey(counter.labels) - guard let existingCounter = counterFamily.metricsByLabelSets[key], - existingCounter === counter - else { - return - } - - var newMetricsByLabelSets = counterFamily.metricsByLabelSets - newMetricsByLabelSets.removeValue(forKey: key) - - if newMetricsByLabelSets.isEmpty { - store.removeValue(forKey: counter.name) - } else { - let updatedFamily = MetricFamily( - metricsByLabelSets: newMetricsByLabelSets, - buckets: counterFamily.buckets, - help: counterFamily.help, - metricUnlabeled: counterFamily.metricUnlabeled - ) + if let updatedFamily = counterFamily.removing(metric: counter, for: counter.labels) { store[counter.name] = .counter(updatedFamily) + } else { + // Remove the entire family. + store.removeValue(forKey: counter.name) } default: return @@ -862,33 +879,11 @@ public final class PrometheusCollectorRegistry: Sendable { self.box.withLockedValue { store in switch store[gauge.name] { case .gauge(let gaugeFamily): - - if let unlabeledGauge = gaugeFamily.metricUnlabeled, unlabeledGauge === gauge { - // Remove the entire family since unlabeled metrics are exclusive. - store.removeValue(forKey: gauge.name) - return - } - - let key = LabelsKey(gauge.labels) - guard let existingGauge = gaugeFamily.metricsByLabelSets[key], - existingGauge === gauge - else { - return - } - - var newMetricsByLabelSets = gaugeFamily.metricsByLabelSets - newMetricsByLabelSets.removeValue(forKey: key) - - if newMetricsByLabelSets.isEmpty { - store.removeValue(forKey: gauge.name) - } else { - let updatedFamily = MetricFamily( - metricsByLabelSets: newMetricsByLabelSets, - buckets: gaugeFamily.buckets, - help: gaugeFamily.help, - metricUnlabeled: gaugeFamily.metricUnlabeled - ) + if let updatedFamily = gaugeFamily.removing(metric: gauge, for: gauge.labels) { store[gauge.name] = .gauge(updatedFamily) + } else { + // Remove the entire family. + store.removeValue(forKey: gauge.name) } default: return @@ -905,33 +900,11 @@ public final class PrometheusCollectorRegistry: Sendable { self.box.withLockedValue { store in switch store[histogram.name] { case .durationHistogram(let histogramFamily): - - if let unlabeledHistogram = histogramFamily.metricUnlabeled, unlabeledHistogram === histogram { - // Remove the entire family since unlabeled metrics are exclusive. - store.removeValue(forKey: histogram.name) - return - } - - let key = LabelsKey(histogram.labels) - guard let existingHistogram = histogramFamily.metricsByLabelSets[key], - existingHistogram === histogram - else { - return - } - - var newMetricsByLabelSets = histogramFamily.metricsByLabelSets - newMetricsByLabelSets.removeValue(forKey: key) - - if newMetricsByLabelSets.isEmpty { - store.removeValue(forKey: histogram.name) - } else { - let updatedFamily = MetricFamily( - metricsByLabelSets: newMetricsByLabelSets, - buckets: histogramFamily.buckets, - help: histogramFamily.help, - metricUnlabeled: histogramFamily.metricUnlabeled - ) + if let updatedFamily = histogramFamily.removing(metric: histogram, for: histogram.labels) { store[histogram.name] = .durationHistogram(updatedFamily) + } else { + // Remove the entire family. + store.removeValue(forKey: histogram.name) } default: return @@ -948,33 +921,11 @@ public final class PrometheusCollectorRegistry: Sendable { self.box.withLockedValue { store in switch store[histogram.name] { case .valueHistogram(let histogramFamily): - - if let unlabeledHistogram = histogramFamily.metricUnlabeled, unlabeledHistogram === histogram { - // Remove the entire family since unlabeled metrics are exclusive. - store.removeValue(forKey: histogram.name) - return - } - - let key = LabelsKey(histogram.labels) - guard let existingHistogram = histogramFamily.metricsByLabelSets[key], - existingHistogram === histogram - else { - return - } - - var newMetricsByLabelSets = histogramFamily.metricsByLabelSets - newMetricsByLabelSets.removeValue(forKey: key) - - if newMetricsByLabelSets.isEmpty { - store.removeValue(forKey: histogram.name) - } else { - let updatedFamily = MetricFamily( - metricsByLabelSets: newMetricsByLabelSets, - buckets: histogramFamily.buckets, - help: histogramFamily.help, - metricUnlabeled: histogramFamily.metricUnlabeled - ) + if let updatedFamily = histogramFamily.removing(metric: histogram, for: histogram.labels) { store[histogram.name] = .valueHistogram(updatedFamily) + } else { + // Remove the entire family. + store.removeValue(forKey: histogram.name) } default: return @@ -1076,71 +1027,39 @@ public final class PrometheusCollectorRegistry: Sendable { for (name, metric) in metrics { switch metric { case .counter(let counterFamily): - if let unlabeledCounter = counterFamily.metricUnlabeled { - if let help = counterFamily.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "counter") - unlabeledCounter.emit(into: &buffer) - } else if !counterFamily.metricsByLabelSets.isEmpty { - if let help = counterFamily.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "counter") - for counter in counterFamily.metricsByLabelSets.values { - counter.emit(into: &buffer) - } + if let help = counterFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "counter") + counterFamily.forEachMetric { counter in + counter.emit(into: &buffer) } case .gauge(let gaugeFamily): - if let unlabeledGauge = gaugeFamily.metricUnlabeled { - if let help = gaugeFamily.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "gauge") - unlabeledGauge.emit(into: &buffer) - } else if !gaugeFamily.metricsByLabelSets.isEmpty { - if let help = gaugeFamily.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "gauge") - for gauge in gaugeFamily.metricsByLabelSets.values { - gauge.emit(into: &buffer) - } + if let help = gaugeFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "gauge") + gaugeFamily.forEachMetric { gauge in + gauge.emit(into: &buffer) } case .durationHistogram(let histogramFamily): - if let unlabeledHistogram = histogramFamily.metricUnlabeled { - if let help = histogramFamily.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "histogram") - unlabeledHistogram.emit(into: &buffer) - } else if !histogramFamily.metricsByLabelSets.isEmpty { - if let help = histogramFamily.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "histogram") - for histogram in histogramFamily.metricsByLabelSets.values { - histogram.emit(into: &buffer) - } + if let help = histogramFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "histogram") + histogramFamily.forEachMetric { histogram in + histogram.emit(into: &buffer) } case .valueHistogram(let histogramFamily): - if let unlabeledHistogram = histogramFamily.metricUnlabeled { - if let help = histogramFamily.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "histogram") - unlabeledHistogram.emit(into: &buffer) - } else if !histogramFamily.metricsByLabelSets.isEmpty { - if let help = histogramFamily.help, !help.isEmpty { - buffer.addLine(prefix: prefixHelp, name: name, value: help) - } - buffer.addLine(prefix: prefixType, name: name, value: "histogram") - for histogram in histogramFamily.metricsByLabelSets.values { - histogram.emit(into: &buffer) - } + if let help = histogramFamily.help, !help.isEmpty { + buffer.addLine(prefix: prefixHelp, name: name, value: help) + } + buffer.addLine(prefix: prefixType, name: name, value: "histogram") + histogramFamily.forEachMetric { histogram in + histogram.emit(into: &buffer) } } }