Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove generic argument from Histogram #103

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove generic argument from Histogram #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 13 additions & 31 deletions Sources/Prometheus/Docs.docc/swift-metrics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,28 +111,19 @@ generated in a third party library.

#### Default buckets

Swift Metric ``Timer``s are backed by a Prometheus ``DurationHistogram`` and Swift Metric
``Recorder``s that aggregate are backed by a Prometheus ``ValueHistogram``. As a user, you can
Swift Metric ``Timer``s are backed by a Prometheus ``Histogram`` and Swift Metric
``Recorder``s that aggregate are also backed by a Prometheus ``Histogram``. As a user, you can
specify which buckets shall be used within the backing ``Histogram``s.

```swift
var factory = PrometheusMetricsFactory()

factory.defaultDurationHistogramBuckets = [
.milliseconds(5),
.milliseconds(10),
.milliseconds(25),
.milliseconds(50),
.milliseconds(100),
factory.defaultTimerHistogramBuckets = [
0.005, 0.01, 0.025, 0.05, 0.1
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels like a step backwards as well tbh

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that this feels like a step backwards


factory.defaultValueHistogramBuckets = [
5,
10,
25,
50,
100,
250,
factory.defaultRecorderHistogramBuckets = [
5, 10, 25, 50, 100, 250,
]
MetricSystem.bootstrap(factory)

Expand All @@ -148,28 +139,19 @@ You can also specify the buckets by metric name:
```swift
var factory = PrometheusMetricsFactory()

factory.defaultDurationHistogramBuckets = [
.milliseconds(5),
.milliseconds(10),
.milliseconds(25),
.milliseconds(50),
.milliseconds(100),
factory.defaultTimerHistogramBuckets = [
0.005, 0.01, 0.025, 0.05, 0.1
]

factory.durationHistogramBuckets["long"] = [
.seconds(5),
.seconds(10),
.seconds(25),
.seconds(50),
.seconds(100),
factory.timerHistogramBuckets["long"] = [
5, 10, 25, 50, 100
]
```

Now a `Timer` with the label "long" will use the buckets
`[.seconds(5), .seconds(10), .seconds(25), .seconds(50), .seconds(100),]`, whereas any other
`Timer` will use the default buckets
`[.milliseconds(5), .milliseconds(10), .milliseconds(25), .milliseconds(50), .milliseconds(100),]`.
`[5 sec, 10 sec, 25 sec, 50 sec, 100 sec]`, whereas any other
`Timer` will use the default buckets `[5 ms, 10ms, 25ms, 50ms, 100ms]`.

The same functionality is also available for ``ValueHistogram`` and aggregating `Recorder`s.
The same functionality is also available for ``Histogram`` that back aggregating `Recorder`s.

[Swift Metrics]: https://github.com/apple/swift-metrics
94 changes: 24 additions & 70 deletions Sources/Prometheus/Histogram.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,28 @@

import CoreMetrics

/// A type that can be used in a ``Histogram`` to create bucket boundaries
public protocol Bucketable: AdditiveArithmetic, Comparable, Sendable {
/// A string representation that is used in the Prometheus export
var bucketRepresentation: String { get }
}

/// A Histogram to record timings
public typealias DurationHistogram = Histogram<Duration>
/// A Histogram to record floating point values
public typealias ValueHistogram = Histogram<Double>

/// A generic Histogram implementation
public final class Histogram<Value: Bucketable>: Sendable {
/// A Histogram implementation, that is backed by buckets in Double
public final class Histogram: Sendable {
let name: String
let labels: [(String, String)]

@usableFromInline
struct State: Sendable {
@usableFromInline var buckets: [(Value, Int)]
@usableFromInline var sum: Value
@usableFromInline var count: Int
var buckets: [(Double, Int)]
var sum: Double
var count: Int

@inlinable
init(buckets: [Value]) {
init(buckets: [Double]) {
self.sum = .zero
self.count = 0
self.buckets = buckets.map { ($0, 0) }
}
}

@usableFromInline let box: NIOLockedValueBox<State>
let box: NIOLockedValueBox<State>
let prerenderedLabels: [UInt8]?

init(name: String, labels: [(String, String)], buckets: [Value]) {
init(name: String, labels: [(String, String)], buckets: [Double]) {
self.name = name
self.labels = labels

Expand All @@ -56,7 +44,7 @@ public final class Histogram<Value: Bucketable>: Sendable {
self.box = .init(.init(buckets: buckets))
}

public func record(_ value: Value) {
public func observe(_ value: Double) {
self.box.withLockedValue { state in
for i in state.buckets.startIndex..<state.buckets.endIndex {
if state.buckets[i].0 >= value {
Expand All @@ -67,20 +55,29 @@ public final class Histogram<Value: Bucketable>: Sendable {
state.count += 1
}
}

public func observe(_ value: Duration) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

So we could record durations into a histogram intended for values; this seems silly for our ecosystem in which we value type safety for such stuff.

let value = Double(value.components.seconds) + Double(value.components.attoseconds) / 1e18
self.observe(value)
}
}

extension Histogram: _SwiftMetricsSendableProtocol {}

extension Histogram: CoreMetrics.TimerHandler where Value == Duration {
extension Histogram: CoreMetrics.TimerHandler {
public func recordNanoseconds(_ duration: Int64) {
let value = Duration.nanoseconds(duration)
self.record(value)
self.observe(value)
}
}

extension Histogram: CoreMetrics.RecorderHandler where Value == Double {
extension Histogram: CoreMetrics.RecorderHandler {
public func record(_ value: Double) {
self.observe(value)
}

public func record(_ value: Int64) {
self.record(Double(value))
self.observe(Double(value))
}
}

Expand All @@ -96,7 +93,7 @@ extension Histogram: PrometheusMetric {
buffer.append(UInt8(ascii: #","#))
}
buffer.append(contentsOf: #"le=""#.utf8)
buffer.append(contentsOf: "\(bucket.0.bucketRepresentation)".utf8)
buffer.append(contentsOf: "\(bucket.0)".utf8)
buffer.append(UInt8(ascii: #"""#))
buffer.append(contentsOf: #"} "#.utf8)
buffer.append(contentsOf: "\(bucket.1)".utf8)
Expand Down Expand Up @@ -124,7 +121,7 @@ extension Histogram: PrometheusMetric {
} else {
buffer.append(UInt8(ascii: " "))
}
buffer.append(contentsOf: "\(state.sum.bucketRepresentation)".utf8)
buffer.append(contentsOf: "\(state.sum)".utf8)
buffer.append(contentsOf: #"\#n"#.utf8)

// count
Expand All @@ -141,46 +138,3 @@ extension Histogram: PrometheusMetric {
buffer.append(contentsOf: #"\#n"#.utf8)
}
}

extension Duration: Bucketable {
public var bucketRepresentation: String {
let attos = String(unsafeUninitializedCapacity: 18) { buffer in
var num = self.components.attoseconds

var positions = 17
var length: Int?
while positions >= 0 {
defer {
positions -= 1
num = num / 10
}
let remainder = num % 10

if length != nil {
buffer[positions] = UInt8(ascii: "0") + UInt8(remainder)
} else {
if remainder == 0 {
continue
}

length = positions + 1
buffer[positions] = UInt8(ascii: "0") + UInt8(remainder)
}
}

if length == nil {
buffer[0] = UInt8(ascii: "0")
length = 1
}

return length!
}
return "\(self.components.seconds).\(attos)"
}
}

extension Double: Bucketable {
public var bucketRepresentation: String {
self.description
}
}
Loading