-
Notifications
You must be signed in to change notification settings - Fork 66
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.
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
API for creating metrics with specific buckets or quantiles #76
Comments
FYI @MrLotU as well :-) |
Big +1! This would be useful for both |
Right, sounds good |
thanks for bringing this up. what would backends that do not support such feature do with this information? |
I'm not sure if it sounds crazy, but I have this thought in mind: |
I think there's really only two ways out, one of them being as Anton suggests. Option 1) backends may ignore this information silentlySo given the example: let timer = Timer(
label: "hello",
dimensions: [("a": "aaa")],
buckets: [.005, .01, .025, .05, .075, .1, .25, .5, .75, 1, 2.5, 5, 7.5, 10]
) Say, statsd client would simply ignore it. It directly emits values without aggregating, so the setting has no effect. Option 2) optional initializersWe could make those new initializers failable, then they'd return nil if unable to support this feature, usage becomes more verbose then: guard let timer = Timer(
label: "hello",
dimensions: [("a": "aaa")],
buckets: [.005, .01, .025, .05, .075, .1, .25, .5, .75, 1, 2.5, 5, 7.5, 10]
) else { fatalError("OH NO, I DEFINITELY NEED BUCKETS") } or, falling back to non buckets if not necessary let timer = Timer(
label: "hello",
dimensions: [("a": "aaa")],
buckets: [.005, .01, .025, .05, .075, .1, .25, .5, .75, 1, 2.5, 5, 7.5, 10]
) ?? Timer(label: "hello", dimensions: [("a": "aaa")]) // "ok, i don't actually care/need buckets" The option 2 is technically more correct but seems like a large burden and could accidentally make some libraries not usable with some backends, only because they took the "oh well, lets crash" approach. We could try to make option 2 spell somewhat nicer, if we really believe in it? Personally I think Option 1 is fine... I'll check what similar APIs do in the Java ecosystem though |
Alternatively, we could double down and design some form of |
@ktoso did you want to do anything here for 2.1.0, or should we release 2.1.0 sooner? |
We can cut at any time as far as I'm concerned - this does not have to be in 2.1.0 :) you can cut or I'll do tomorrow |
I believe this feature would be nice to have. |
Some more users often need to pre-configure their metrics with pre-determined bucket sizes (and numbers) or quantiles they want to report. This matters most for metrics which do some form of aggregation client side, like prometheus (e.g. https://github.com/MrLotU/SwiftPrometheus/blob/master/Sources/Prometheus/Prometheus.swift#L210 ).
We should make it possible to use the
SwiftMetrics
API without dropping to raw APIs and still be able to configure bucket sizes etc. These are a common and not specific-backend bound configuration parameter.It could look something like:
Use case discussed with @avolokhov, we'll iterate on the exact needs and shapes of this soon
The text was updated successfully, but these errors were encountered: