Gauge.record() is under-defined

[ktoso]

So I've been using the API a bit, and found an confusing thing:

It is confusing with regards how one should implement gauge.record(). An implementation I just reviewed, the prometheus one, implements it by delegating to it's underlying add which I think is incorrect, however we don't really provide any guidance about it, so I can see how the author ended up in this:

// SwiftPrometheus
    func record(_ value: Int64) {
        self.record(value.doubleValue)
    }
    
    func record(_ value: Double) {
        gauge.inc(value, labels)
    }

Remember also that we force a gauge to be an NOT aggregated recorder:

    public convenience init(label: String, dimensions: [(String, String)] = []) {
        self.init(label: label, dimensions: dimensions, aggregate: false)
    }

Which again strengthens my idea that it should be a "set" operation.

https://github.com/MrLotU/SwiftPrometheus/blob/master/Sources/PrometheusMetrics/PrometheusMetrics.swift#L37-L42

I think we need to:

• specify which semantics gauge.record -- I think it should be a "set" on the Gauge, and on Histogram it depends I guess but also sounds like a "set"
• I wonder if we are able to offer an add an add() really (case for this below)...

---

For reference, the only guidance we provide are:

/// A recorder collects observations within a time window (usually things like response sizes) and can provide aggregated information about the data sample,
/// for example, count, sum, min, max and various quantiles.

and

/// Record a value.

public func record<DataType: BinaryInteger>(_ value: DataType) {

Which does not explain if record is a set or add, or if its interpretation should change depending if the recorder / gauge is aggregate or not.

---

Note also that open telemetry is converging on gauges offering both and I'd tend to agree with that actually now that I had to use our API a bit:

https://github.com/open-telemetry/opentelemetry-java/blob/master/api/src/main/java/io/opentelemetry/metrics/GaugeLong.java

• add
• set -- like our record

[ktoso]

Paging @tomerd @weissi @MrLotU

[tomerd]

thanks @ktoso, as you suggest, the design for recorder and gauge is set semantics. do you think we should change the method name or improve the docs, or something else?

[MrLotU]

@ktoso @tomerd I would definitely go with additional documentation, though having both . add() and .set() or .record() also sounds like a great idea. I’ll at least update my prometheus implementation to use set() internally now

[MrLotU]

To extend on my previous comment, I think the extending the docs should be an AND kind of thing, so whichever way this goes, adding methods, renaming methods, or something completely different. IMO extending the docs should happen in all those cases.

[ktoso]

thanks @ktoso, as you suggest, the design for recorder and gauge is set semantics. do you think we should change the method name or improve the docs, or something else?

• Let's start with docs, to help people implement it the right way. I'll PR an addition there.
  • changing method name would be somewhat more of a change since we'd have to break the extending Recorder I guess...
• add would be a nice addition, but that's a separate topic.