Moved workbench-metrics from rawls

[helgridly]

PR checklist

• For each library you've modified here, decide whether it requires a major, minor, or no version bump. (Click here for guidance)

If you're doing a major or minor version bump to any libraries:

• Bump the version in project/Settings.scala createVersion()
• Update CHANGELOG.md for those libraries
• I promise I used @deprecated instead of deleting code where possible

In all cases:

• Get two thumbsworth of PR review
• Verify all tests go green (CI and coverage tests)
• Squash commits and merge to develop
• Delete branch after merge

After merging, even if you haven't bumped the version:

• Update README.md and the CHANGELOG.md for any libs you changed with the new dependency string. The git hash is the same short version you see in GitHub (i.e. seven characters). You can commit these changes straight to develop.

[helgridly]

note this used to be m.toString.toLowerCase but that results in httpMethod(get) in akka-http, which is ugly

[rtitle]

👍

[helgridly]

requestInstance -> extractRequest and mapHttpResponse -> mapResponse for spray -> akka-http

it seems to work, according to tests.

[rtitle]

Actually the code in this module is agnostic to the backend reporter -- our code is just concerned with writing to the in-memory MetricRegistry (provided by metrics-scala/dropwizard). Then we happen to use metrics-scala to pull off the MetricsRegistry and write to statsd, but we could conceivably use something else besides statsd.

As I'm thinking about this -- the workbench-metrics includes the metrics-statsd dependency but it's only used in Rawls here. So technically we could remove the metrics-statsd dependency from workbench-metrics. But since we're standardizing on statsd, it might be better to leave it in this module for convenience sake.

Sorry for the long-winded way of saying "this is fine".

[helgridly]

Funnily enough I was wondering if that block of code in Rawls should be moved up into this library. We should decide one way or another if this library should be "drop in and you get statsd reporting" or "drop in and you get metrics but wire up your own backend", and do whichever the right thing is based on that.

Opinions? I lean toward the former.

[rtitle]

Good point - I also like the idea of moving

def startStatsDReporter(host: String, port: Int, period: java.time.Duration): Unit = {
    logger.info(s"Starting statsd reporter writing to [$host:$port] with period [${period.getSeconds} seconds]")
    val reporter = StatsDReporter.forRegistry(SharedMetricRegistries.getOrCreate("default"))
      .convertRatesTo(TimeUnit.SECONDS)
      .convertDurationsTo(TimeUnit.MILLISECONDS)
      .build(host, port)
    reporter.start(period.getSeconds, period.getSeconds, TimeUnit.SECONDS)
  }

into this library somewhere. It would be a convenience for other modules depending on this library (they wouldn't need to wire up statsd themselves).

[helgridly]

Anything else that needs to be yanked over?

[rtitle]

I think that's it in terms of existing code.

One other thought, Rawls has this config:

metrics {
  prefix = "{{$environment}}.firecloud.rawls"
  includeHostname = false
  reporters {
    # statsd commented out until it's available in the docker container
    statsd {
      host = "statsd"
      port = 8126
      period = 1m
    }
  }
}

https://github.com/broadinstitute/firecloud-develop/blob/dev/configs/rawls/rawls.conf.ctmpl#L134-L145

Currently it's all parsed in Boot.scala, but maybe we could make a MetricsConfig case class and put it in workbench-metrics. Maybe even add logic for parsing it from a .conf file, though I'm not sure if we should assume all modules are using typesafe-config.

[helgridly]

I think it's reasonable to let users fill in startStatsDReporter themselves. Similarly providing prefix etc.

[rtitle]

All looks good to me -- cool that the spray->akka-http conversion didn't require too many changes besides renaming stuff.