API for setting list of dimensions on record, rather than on Metric creation

[avolokhov]

Current metrics api only supports setting dimensions on metric creation. I think to fully leverage dimensions and express in code that certain set of metrics report into a same metric label, we need to allow setting dimensions on record as well as in constructor. E.g. imagine we're trying to instrument an http server. With custom dimensions we could do something like this:

let requestDurationTimer = Timer(label: "request_duration", dimensions: [("instance": "myInstance")])
func handle(request: Request) {
  var statusCodeClass: String = "2XX"
  let startTime = DispatchTime.now()
  defer { 
    requestDurationTimer.recordNanoseconds(since: startTime, 
                                           dimensions: [("statusCode": statusCodeClass, 
                                                         "uri": request.uri)])
  }
  do {
    try handleRequest(request)
  } catch let error as ServerError {
    // recovery routine
    statusCodeClass = "5XX"
  } catch let error as UserError {
    statusCodeClass = "4XX"
  }
}

We won't have to create a separate requestDurationTimer for each "uri" as well as a separate timer for each uri/error class combination. This also applies to more complex state machines instrumentation when we may want to express certain event belongs to a certain state/command.

[MrLotU]

The idea behind the swift-metrics metric types is that they should be "throw away" types. So create one when you need it, increment it, and then drop it again. In your example this would mean creating the timer inside of the defer clause and immediately incrementing it.

func handle(request: Request) {
  var statusCodeClass: String = "2XX"
  let startTime = DispatchTime.now()
  defer { 
    Timer(label: "request_duration", dimensions: [("statusCode": statusCodeClass, "uri": request.uri)])
        .recordNanoseconds(since: startTime)
  }
  do {
    try handleRequest(request)
  } catch let error as ServerError {
    // recovery routine
    statusCodeClass = "5XX"
  } catch let error as UserError {
    statusCodeClass = "4XX"
  }
}

I hope this helps 😄

[avolokhov]

Thanks for the feedback @MrLotU!
I see several downsides to using swift-metrics types as "throw away" types:

1. There's a backend with a registry behind it. Matching an element with one in a registry is a heavy and lock protected operation (registry lookup on a globally shared list/map). With a loaded enough service it can become a bottleneck and may substantially hurt overall performance.
2. There's a pattern when you physically separate your instrumentation from your code - it makes it much easier to learn what metrics are published and keeps an explicit "registry" of what you currently instrument. E.g. see Instrument the instance and shell using Swift Metrics #56 swift-cluster-membership#70
   I believe this pattern may make code more readable/maintainable and is worth supporting.

[avolokhov]

cc @ktoso, do you think this API would make sense in context of you swift-cluster-membership instrumentation?

[ktoso]

Yes, sorry I didn't chime in -- your points capture it exactly @avolokhov 👍

Metrics are not to be assumed to be free to keep creating ad hoc every single time exactly because they may hit a registry and touch locks which may become contended then. If possible keeping a metric object around is a good pattern. Of course this depends on specific backends, but in many that's the case (as is the case with the prom impl actually).

[avolokhov]

So by adding this to the API we'd actually make backend work more difficult - if we allow dimensions on record we'll have to do some ad-hoc matching, while in an old implementation MetricType is immutable and can be fully matched against the registry at the creation time. This may make backend impl substantially more difficult (nested map to get everything by label on init and then dynamically match dimensions on record?)
I'm not 100% sold on this change because of the above thought, but it might make things more concise and unified in case when you can't (or it's just lots of boilerplate) enumerate all your metrics in advance.

[toffaletti]

I too ran into a lot of confusion with the current API and what the expectation is here. I think #139 is another example of this.

In addition to all the great points @avolokhov makes, I think the current Swift Metrics API deviates from patterns in other languages enough to make it very unclear how to use the API for recording multi-dimensional metrics. I think the documentation needs to confront this difference by calling it out and providing examples so confused folks like me aren't driven to the issue tracker.