Consider a type preserving API

[fabianfett]

This is a very raw first draft for a new API proposal.

Motivation

Swift Metrics API currently follows the same design principles as Swift Logging. The Server Metrics API forums thread links out to a PR.

Currently users create new metrics by calling initializers on different Metric types:

Counter(label: String, dimensions: [(String, String)])

When users call this, internally we will reach out to the registered MetricFactory through the global MetricSystem.factory to create a new CounterHandler, so that the new Counter can be backed by the just created CounterHandler.

Generally I think this behavior is great and I do not want to change it.

However whenever I use Swift Metrics, I pass around the MetricsFactory manually instead of going through the global MetricSystem.factory. The reason for this is to allow better testability, when using the TestMetrics from the MetricsTestKit module. If I don't use the global MetricSystem.factory, I can easily create a new TestMetrics instance for each test run, which even allows me to run tests in parallel.

Currently the MetricsFactory protocol creates existential MetricHandlers. For each supported Metric type, the factory returns an existential MetricHandler (CounterHandler, TimerHandler, etc.). The reason for not returning explicitly typed handlers here, is that the MetricsFactory needed to be stored as an existential itself in the MetricsSystem.global. Before Swift 5.7 existentials were not able to have associated types.

However thanks to SE-309 Unlock existentials for all protocols we are now able to make MetricsFactory type safe.

Proposed Solution

Since I don't want to break API, I propose a new Protocol that adopters can implement:

public protocol TypePreservingMetricsFactory: MetricsFactory {
    associatedtype Counter: CounterProtocol
    associatedtype FloatingPointCounter: FloatingPointCounterProtocol
    associatedtype Timer: TimerProtocol
    associatedtype Recorder: RecorderProtocol

    func makeSomeCounter(label: String, dimensions: [(String, String)]) -> Self.Counter

    func makeSomeFloatingPointCounter(label: String, dimensions: [(String, String)]) -> Self.FloatingPointCounter

    func makeSomeRecorder(label: String, dimensions: [(String, String)], aggregate: Bool) -> Self.Recorder

    func makeSomeTimer(label: String, dimensions: [(String, String)]) -> Self.Timer

    func destroyCounter(_ handler: Self.Counter)

    func destroyFloatingPointCounter(_ handler: Self.FloatingPointCounter)

    func destroyRecorder(_ handler: Self.Recorder)

    func destroyTimer(_ handler: Self.Timer)
}

I added the Some word into the make function calls to ensure those methods are not overloaded by return type. Other naming suggestions are highly welcome. The new CounterProtocol is very close to the existing CounterHandler protocol. However it adds requirements for the label and the dimensions.
The reason for this is simple: I would like to remove the current Counter, Timer and Recorder wrapper classes, that add one level of indirection, that isn't needed.

public protocol CounterProtocol: CounterHandler {
    var label: String { get }
    var dimensions: [(String, String)] { get }
}

public protocol FloatingPointCounterProtocol: FloatingPointCounterHandler {
    var label: String { get }
    var dimensions: [(String, String)] { get }
}

If we want to preserve the option to create an untyped Metric in the future with an init, we could use callAsFunction on top of a protocol.

If a user implements TypePreservingMetricsFactory all methods needed for MetricsFactory are implemented by a default implementation:

// This extension ensures TypePreservingMetricsFactory also implements MetricsFactory
extension TypePreservingMetricsFactory {
    public func makeRecorder(label: String, dimensions: [(String, String)], aggregate: Bool) -> RecorderHandler {
        self.makeSomeRecorder(label: label, dimensions: dimensions, aggregate: aggregate)
    }

    public func makeTimer(label: String, dimensions: [(String, String)]) -> TimerHandler {
        self.makeSomeTimer(label: label, dimensions: dimensions)
    }

    public func makeCounter(label: String, dimensions: [(String, String)]) -> CounterHandler {
        self.makeCounter(label: label, dimensions: dimensions)
    }

    public func makeFloatingPointCounter(label: String, dimensions: [(String, String)]) -> FloatingPointCounterHandler {
        self.makeSomeFloatingPointCounter(label: label, dimensions: dimensions)
    }

    public func destroyTimer(_ handler: TimerHandler) {
        if let handler = handler as? Self.Timer {
            self.destroySomeTimer(handler)
        }
    }

    public func destroyCounter(_ handler: CounterHandler) {
        if let handler = handler as? Self.Counter {
            self.destroySomeCounter(handler)
        }
    }

    public func destroyRecorder(_ handler: RecorderHandler) {
        if let handler = handler as? Self.Recorder {
            self.destroySomeRecorder(handler)
        }
    }

    public func destroyFloatingPointCounter(_ handler: FloatingPointCounterHandler) {
        if let handler = handler as? Self.FloatingPointCounter {
            self.destroySomeFloatingPointCounter(handler)
        }
    }
}

Future ideas

• The make and destroy methods on the TypePreservingMetricsFactory should be async to allow the use of an actor to create, maintain and destroy the created metrics. However this is a breaking change.
• We should make the Gauge type stand byitself and not be implemented on top of recorder.
• We should potentially offer simple Counter, Timer and Recorder implementations, that implementors of backends can use right away. So backend developers only have to worry about metric lifecycle management and exporting.

[fabianfett]

cc @tomerd, @weissi

[fabianfett]

@PeterAdams-A mentioned that TypePreservingMetricsFactory should probably not inherit from MetricsFactory, but that we should have an internal translation layer... I agree with this and will adjust the proposal later...

[Lukasa]

Do you have a proposed timeline for adopting this change? Without 5.7 it’s very hard to use, so we’ll presumably have to gate its availability, which is tricky to communicate downstream.