Define distribution metrics for timers in `registry.timer(metric)` call

[manolama]

Right now, we have to define an association with a metric ID and distribution bucket definition in the reporter builder. This breaks the current API flow wherein metrics are declared at runtime via the calls like registry.timer(metric). Most users will have their metric strings spread across multiple classes and possibly even modules so pre-registering the metrics and distributions are difficult and anti-pattern. Now I have to write a config file with the metrics I want to record histograms from every class, package and module I'm tracking from, read the config at start up and register them with the reporter. This is much more prone to error since I have to remember to add new metrics to this config and the spelling must match exactly.

Instead we should override the timer method with the distribution so the programmer can choose, at run time, what metrics should be histograms and what the buckets should be. E.g. registry.timer(final String metricId, final DistributionBucket distribution); In the event the distribution signature changes (which may be why the reporter registration was chosen) we can handle it in two ways: 1) Throw an error saying the distribution changed. 2) Reset the metric with the new distribution and start recording from there.

E.g.

private static final String HISTO_ID = "query.latency.histo";
private static final String LATENCY_ID = "query.latency";
private static final DistributionBucket DISTRIBUTION = new DistributionBucket(new long[] {0, 10, 50, 100});
private static final String[] TAGS = new String[] { "method", "someMethod", "status", "200" };

public void someMethod() {
  final Timer histoTimer = metricRegistry.timer(HISTO_ID, DISTRIBUTION);
  final Timer latencyTimer = metricRegistry.timer(LATENCY_ID);

  final long histoStart = histoTimer.start(); // NOTE: Why can't we optionally store the long in the timer itself?
  final long latencyStart = latencyTimer.start();

  // fire off a query
  int status = 200;

  final String[] tags;
  if (status == 200) {
    tags = TAGS;
  } else {
    tags = Arrays.copyof(TAGS, TAGS.length);
    tags[TAGS.length - 1] = Integer.toString(status); // dynamic status code
  }
  histoTimer.stop(histoStart, tags);
  latencyTimer.stop(latencyStart, tags);
}

[arungupta]

I had the same observation with the current implementation. While it's possible to workaround, it's non-ideal.

[mmannerm]

The proposal will break independence of reporters and instrumentation and disallow using different configurations in different reporters.

There was earlier design with default value in meter that would be overridable by reporter, why was that design rejected @smrutilal2

[smrutilal2]

My initial implementation had the option to pass the distribution bucket while creating the metric. But, it got rejected during the review. I guess, we just wanted to hear it from the real users before bring it back.

[mmannerm]

This issue should show side by side proposal

[smrutilal2]

Users from mail-jedi are also asking for the feature. Me and @mmannerm had a discussion about this last week. We are thinking of adding a support where is user can provide a generic Config hint for the metric at the instrumentation side. Which can be overridden at the reporter end. To be future proof instead of adding a specific config like DistributionBucket we will support multiple config of a generic type say MetricConfig for a given metric. The possible specific config types are DistributionBuckets, DataSketches etc.