JAVA-2694: Add Metrics to the extension (reimplement metrics)

[tomekl007]

This is the version with a reimplemented driver metrics to use microprofile metrics used in the quarkus. To see the alternative approach look at this PR: https://github.com/datastax/cassandra-quarkus/pull/2

I see two cons of this solution:

• I think that the overall complexity of this version is higher than wrapping (look at lines of code, it is 2x and I didn't add tests for implemented metrics yet).
• the timer metrics that we are using in the driver codahale metrics is a custom HdrReservoir that is a com.codahale.metrics.Reservoir, so it has a dependency on the codahale. We are not able to use it when excluding dependency. I don’t think that re-implementing HdrReservoir is a good idea. Even worse than that, I don’t see a way for org.eclipse.microprofile.metrics.MetricRegistry to provide custom Timer implementation (like we do for HdrReservoir) so we would need to strip that functionality and ignore properties as such:

  DefaultDriverOption.METRICS_SESSION_CQL_REQUESTS_HIGHEST,
  DefaultDriverOption.METRICS_SESSION_CQL_REQUESTS_DIGITS,
  DefaultDriverOption.METRICS_SESSION_CQL_REQUESTS_INTERVAL);

Only one con is the ability to exclude the <artifactId>metrics-core</artifactId>: https://github.com/datastax/cassandra-quarkus/pull/3/files#diff-48c560bbb8f61d82574c2887b65b498fR68-R71

[absurdfarce]

I'm kinda conflicted here. I agree there is more complexity here, although the changes in response to @olim7t 's points have reduced that considerably. In exchange for that complexity, though, we get some non-trivial benefits.

The first thing that struck me is that the conversion to MicroProfile isn't really tied to Quarkus. We could (semi-)easily put all this in java-driver and allow any user to use MicroProfile instead of DropWizard for any application. That may not have been a feature we would've developed in isolation but when we already have it perhaps we should consider it.

A side effect of the above: Quarkus is largely ignorant of this impl: more specifically there aren't any MetricBuildItems getting passed around here. That in turn means we aren't iterating over all the supported metrics and generating a MetricBuildItem for them. This seems like a pretty big goodness. There is some metric-specific stuff in MicroProfileXMetricUpdater but that's not really different from DropwizardXMetricUpdater in java-driver.

Another observation: we were discussing doing the wrapped impl now and moving to the more robust implementation at some later date. But if we have the more robust implementation now... is there an argument for waiting?

I am somewhat concerned about effectively maintaining two metrics impls but I'm not sure that's enough to outweigh the benefits described above.

[olim7t]

The first thing that struck me is that the conversion to MicroProfile isn't really tied to Quarkus. We could (semi-)easily put all this in java-driver and allow any user to use MicroProfile instead of DropWizard for any application.

If the driver was still in beta that would definitely be something to investigate. But we're going to hit backward compatibility issues if we do it now, I don't think it's worth the hassle.

At least this PR confirms that the driver's internal design works as intended: you can switch metric frameworks with very little surface changes (namely, session.getMetrics() is empty and you have to find another way to expose the metrics if you want programmatic access to them).

A side effect of the above: Quarkus is largely ignorant of this impl: more specifically there aren't any MetricBuildItems getting passed around here.

Yes, I like that too. The only code that needs to be aware of the actual list of driver metrics are the custom *MetricsUpdater components. And as @tomekl007 noted, another huge win is that dynamic node metrics work out of the box.

Another observation: we were discussing doing the wrapped impl now and moving to the more robust implementation at some later date. But if we have the more robust implementation now... is there an argument for waiting?

I think we can move to this approach over wrapping right now.

I am somewhat concerned about effectively maintaining two metrics impls

If you're talking about dropwizard in the driver core vs. microprofile here, yes there is some duplication and we'll have to remember to update this extension when the driver adds new metrics.

[absurdfarce]

If you're talking about dropwizard in the driver core vs. microprofile here, yes there is some duplication and we'll have to remember to update this extension when the driver adds new metrics.

Yeah, this is what I'm concerned about. I'm not super-enthusiastic about maintaining two distinct metrics implementations let alone two impls in completely different repos. That was one of my reasons for making the observation about possibly moving this to java-driver; at least there we'd have everything under one roof.

Real as that concern is I think I'm 👍 to @olim7t 's comments earlier... it seems like the approach in this PR offers too many benefits to ignore and should be the approach we take here.

[absurdfarce]

Somewhat off-topic, and I don't want to make too big a deal out of it... but we could support programmatic access to metrics with a custom Session type here, right? It's not at all clear to me that it's worth it, more musing out loud.

[tomekl007]

Somewhat off-topic, and I don't want to make too big a deal out of it... but we could support programmatic access to metrics with a custom Session type here, right? It's not at all clear to me that it's worth it, more musing out loud.

There is programmatic access to metrics via MetricRegistry:

 @Inject
  @RegistryType(type = MetricRegistry.Type.VENDOR)
  MetricRegistry registry;