make it easy to enable/disable specific metrics through config

[donbourne]

Spring has concept of enabling metrics through config: ​​https://docs.spring.io/spring-boot/docs/2.0.x/reference/html/production-ready-metrics.html#_per_meter_properties . Seems worthwhile to have a way to do this (both Oracle and Liberty MicroProfile Metrics impls have concepts for this we've had to impl outside of the basic requirements of the spec.

we should be able to use MP config to enable/disable certain metrics

[tjquinno]

IIUC the Spring implementation disables the transmission of the data for affected meters. Here is their description of the meaning of xxx.enabled:

Whether to deny meters from emitting any metrics.

(emphasis mine)

I have not looked at the source, but I presume a "disabled" meter still resides in a MeterRegistry and still can be found in the registry, updated, and queried.

We should be very clear what MP metrics means by a "disabled" MP metric. Is it merely "silent" with respect to output via /metrics or is it something more than that?

[Channyboy]

@donbourne @tjquinno
It would seem that the "disabling" for springBoot implies preventing registration into the meterRegistry through micrometer meter filters. It does return a no-op.

So for MP Metrics, we would similarly say that the metric will be denied registration (will not exist in the MP MetricRegistry at all) and a no-op will be returned. For annotations, no registrations and nothing will happen from the annotation.

[tjquinno]

One concern about this approach:

An app might be written to query the registry for a given metric (using getCounter for example), having registered it earlier. If an app end-user disables the metric using configuration, then with the SpringBoot model the metric would never be registered and getCounter will return null.

One could argue that the code should be written to tolerate this change in behavior, but that would complicate the application code at every attempted retrieval of a metric from the registry.

Ultimately, this all comes down to what we actually want to mean--and being very clear about it--by "disabled."

[Channyboy]

For 5.1 if we mention that that "disabled" implies non-registration or rather denied registration and that it would not exist in the MP Metric registry, would that not be a complete explanation?

[tjquinno]

Yes, it would be, but I want us to be sure of what behavior we want before we work on the phrasing to describe it.

To me, the denied registration approach seems less developer-friendly as I tried to illustrate earlier.

Consider this code:

Counter c = metricRegistry.counter("myCounter");
Counter retrievedC = metricRegistry.getCounter("myCounter");

If we adopt the denied-registration behavior, then either:
1 . the code above must be prepared to handle the null returned by line 2, even though the code thinks it has just registered that counter, or
2. the metric registry must return a second no-op Counter in line 2.

In choice 1, the developer needs to anticipate that the metric could be disabled and, therefore, with the denied registration approach, be able to deal with a null return value from getCounter. That makes for awkward and confusing code, given that the app had just registered in line 1 the very metric it tries to retrieve in line 2. In a more practical example the retrieval of the metric would likely be separated more than this from the metric's creation, but the awkward and confusing nature of the code around the retrieval (checking for null) would remain.

In choice 2, to honor the contract of getCounter (return null if the requested metric had not previously been registered), the metric registry would need to remember that the just-preceding registration had occurred so it could return a no-op counter instead of a null. Remembering the registration seems to directly oppose the denied registration implementation of disabling metrics. Or, we relax that contract in the case where a requested metric is disabled and always return a no-op counter in line 2 (even without it having previously been registered), which (to me) seems weird and would be surprising to a developer.

Another approach would be for the registry implementation to be fully functional (registrations, retrievals, removals all work) but the metrics returned would be no-ops when disabled. This allows the code to be written according to the existing API contracts. Put another way, only the disabled metrics themselves would be disabled; the registry(ies) themselves would be fully functional and conform to the contracts in the API.

We might conclude that these types of scenarios--an app registration and then retrieving the same metric from the registry later--are rare enough to be less important than compatibility with SpringBoot's behavior.

I just want us to be sure we're going through that thought process and choosing carefully the behavior we want.

[tjquinno]

To add to the conversation...

As @donbourne mentioned earlier, using a Micrometer filter to implement disabling has the effect of denying registration. And that's a simple (and fairly natural) implementation in a Micrometer environment, but the spec needs to prescribe consistent behavior regardless of the underlying implementation choices.

With that in mind, if we adopt this approach we should be sure that the JavaDoc for each "get-or-create" method on MetricRegistry is explicit that, if the user has disabled that metric (or all metrics) then

1. the method returns a no-op metric, so updates will not be reflected in the metric's value, and
2. no metric is ever added to the registry, so attempts to retrieve any metric--implied by an annotation or explicitly registered via the MetricRegistry API--will not find it and will create a new no-op metric.