Additional metrics changes

[EdColeman]

• Provide a metrics SPI to enable metric provider extensions by users

  • Adds the MeterRegistryFactory to ..core.spi.metrics and deprecates .core.metrics.MeterRegistryFactory
  • Modified the GENERAL_MICROMETER_FACTORY to be a list of class names. This allows multiple providers to be used. For example, logging and statsd.
  • Allows passing parameters using standard general.custom.metrics.opts prefix to allow for user runtime customization
  • Provides a sample metrics implementation using a LoggingProvider

• Adds metric initialization to ServiceContext

  • provides consistent use of tags and service metrics initialization so service name and ip address are applied as common tags.

• Removes prop store metrics

• Completes removal of static MetricsUtils class

This supersedes and includes the changes and comments from PR #4244

[dlmarion]

It appears there is a common pattern being used by the server processes:

addServiceTags();
addMetricsProducers();
init()

I'm wondering if this could be collapsed to one method that initializes the MetricsInfo object and makes it ready for use.

[EdColeman]

The pattern is common, but in TimedProcessor there is an example where a producer is added, but init() is not directly called. With TServerUtils it was necessary to allow producers to be added, but init() deferred until service start.

I kept the methods separate because it is not required that they are always called back-to-back. There could be an confgAndInit() method that would collapse the pattern, but addProducers() would still be necessary and seemed it could be more confusing to understand when to use one or the other, and the discrete methods made it more explicit,

[dlmarion]

So you have a multi-step build process where some of the steps are optional. This seems like a good place for a Builder then. It seems that once init is called, then the configuration is fixed and ready for use, like a build() method that creates an un-modifiable object. I think the implementation here would be that ServerContext would have a reference to a MetricsInfoBuilder, it would be passed around as needed to configure all the things, then the server process would call build() on it which would be much like the init method. Just a thought.

[EdColeman]

That is a good observation and essentially correct. I'd need to think about a builder and maybe do it as a follow-on. I didn't consider adding a builder to server context and passing that around.

The key thing is that common tags must be bound to the registry before creating / binding meters.

From the micrometer tag documentation:

Common tags generally have to be added to the registry before any (possibly autoconfigured) meter binders. Depending on your environment, there are different ways to achieve this.

That's what prompted this whole set of changes - to gather the common tags (some of which are run-time dependent (i.e. port number)) and then apply them to the registry and then perform the registration of the producers - sealing the common tags with the init() call.

[keith-turner]

Why is the last part of the logger name all caps?

[EdColeman]

I did not want it to look like a class name and it makes it easy to grep. Specifically, if the output is not defeated by logging configuration, the metrics will appear in the standard logs.

[dlmarion]

I wonder if the interface declaration should be:

public interface MeterRegistryFactory<T extends MeterRegistry>

and then this method could be

T create();

[EdColeman]

The use of ClassLoaderUtil.loadClass to dynamically load the factories seems to complicate the use of a generic. Using the generic T gets flagged as a raw parameter usage when loading the classes in MetricsInfoImpl - and it is not obvious to me how to craft the signature.

[ddanielr]

Overall I think the changes are good!
I'd like to get these in so we can start testing with them in the other branches