Awkward collection API

[ptoffy]

I've switched from v1 to v2 and this was the switch I've had to make to collect metrics

- return try await MetricsSystem.prometheus().collect()
+ var buffer = [UInt8]()
+ (MetricsSystem.factory as? PrometheusMetricsFactory)?.registry.emit(into: &buffer)
+ return String(bytes: buffer, encoding: .utf8) ?? ""

which seems quite the downgrade in terms of usability. It would be cool if the library provided some helpers to collect data. I created this

extension MetricsSystem {
    static var prometheus: PrometheusMetricsFactory {
        get throws {
            guard let factory = Self.factory as? PrometheusMetricsFactory else {
                throw Abort(.internalServerError, reason: "Metrics factory is not Prometheus")
            }
            return factory
        }
    }
}

extension PrometheusMetricsFactory {
    func emit() -> String {
        var buffer = [UInt8]()
        self.registry.emit(into: &buffer)
        return String(bytes: buffer, encoding: .utf8) ?? ""
    }
}

to account for it and the result is quite a bit simpler

try MetricsSystem.prometheus.emit()

so I was wondering if a PR would be accepted with this or a similar addition, maybe a [UInt8] version too if needed, especially as the use case is often likely to be an endpoint which we just want to return the data too and we probably won't need a buffer. I'm happy to be proven wrong in case there's a simpler way already or a reason for it to just be like this!

[mman]

@ptoffy

I have been bitten by this as well, but if you look into the implementation of registry.emit, it in the end the will call emit on all the the metrics, and they will all just append to a buffer. So when the buffer is small, it will cause frequent reallocations. If you do this every 15 seconds, and emitting a lot of metrics, you are essentially causing lot of unnecessary reallocations which may or may not affect your server app.

So I'd assume given the knowledge of how much metrics you expect to emit, you can pre-allocate the buffer once and use that on every emit call.

I'm not entirely sure if my understanding is correct, but if it is, I'd love to open a PR against the README and API docs to clarify this... as it looks like a FAQ

[fabianfett]

So I'd assume given the knowledge of how much metrics you expect to emit, you can pre-allocate the buffer once and use that on every emit call.

That is 100% the intention here! We should better document this however!

[incertum]

@fabianfett I believe there could be value in supporting various emit options and let the user choose, WDYT?

[ktoso]

Yeah the reason for the buffer API is to avoid churning allocations and just preallocate a big buffer to reuse.

In production that's what's preferable but I also understand it's a bit annoying and just getting a string can be useful sometimes.

It's fine if we offered some emitToString() and call it something like that that's "long" and put docs that the reusable buffer style is preferable