APPSERV-59 Updates to MicroProfile Metrics 2.3

[jbee]

Summary

Updates Payara to conform to the MicroProfile Metrics 2.3 standard.

New Features and Additions:

• Implements the added SimpleTimer metric type and @SinplyTimed annotation
• Metadata now has uses Optionals to distinguish between not set values for unit, description or display name from intentionally set values including the empty string. This requires to change how Metadata is build from annotations.
• MetricRegistry now uses a Clock so the Metrics it creates that use a Clock use the one passed to the registry so it becomes unit testable
• ConcurrentGaugeImpl now uses a Clock for deciding when to roll over the min/max state so this becomes unit testable without waiting a minute.
• TimerImpl can now be created with a custom Clock for unit testing
• Replaced the use of LongAdded with AtomicLong in some places. While it is true that the adder has better throughput due to smaller risk of thread contention it is only used on paths that will also use at least one but often several other atomic primitives so if there ever were a thread contention it would simply occur on one of those CAS instead. With that in mind AtomicLong seems the better choice as it is more lightweight in both memory and CPU usage. Also should there be a hot loop with many threads involved using a metric (e.g. a counter) the bottleneck of thread contention on an atomic CAS operation will be the least of our problems. There are many other (in comparison) heavy weight operations in the path that should pop up as problem before the CAS.
• A race condition in the MinMax updating of ConcurrentGaugeImpl was fixed.
• MetricsWriter now uses MetricRegistry.Type for the scope (which was misunderstood as registryName before (Scope only allows base, vendor, application whereas the registry name is base, vendor or that of a particular application).
• gauges resolved via CDI are injected as a proxy in case no such gauge is currently known to the registry assuming that the gauge will be made known later during discovery and bootstrapping. As long as proxy cannot get hold of the actual gauge it will return null. This behaviour is not yet standard but suggested to be (see Fixes GaugeInjectionBeanTest setup eclipse/microprofile-metrics#563 (comment))
• NoSuchRegistryException made a RuntimeException (otherwise cause ugly try-catch cascades for no reason)
• fixes MP config ordering of sources that should fall back on name order in case of same ordinal

Related Issues

• Fixes GaugeInjectionBeanTest setup eclipse/microprofile-metrics#563
• NPE in MethodParameterInjectionPoint arquillian/arquillian-core#235

Testing

Tests Added

The Metrics module was sparsely tested. Since the existing code was hard to follow, at times seemed duplicated and inconsistent I decided to change code so that it became unit-testable and easier to follow so I could write test to make sure we are complaint with the specification.

All extracting data from annotations and annotated member is replaced by the AnnotationReader class which is tested with a coverage > 88%. This replaces different utilities that would each have their own inconsistent logic (which isn't correct).

The export to JSON is done by JsonExporter (coverage > 97%), the export to OpenMetrics is done by OpenMetricsExporter (coverage > 98%). These operate on the level of individual metrics which in a case for multiple metrics for the same name must be exported following one another. Extracting the data for export from one or more registries and feeding individual metrics in right grouping into the exporter is done by the MetricsWriterImpl (tests need to be created).

Unit and component tests were added to cover the important and complex parts.

This includes:

• Reading effective metrics IDs and Metadata from annotations
• General interceptor logic
• REST API JSON and OpenMetrics formatting

The formatting is both tested with examples found in the spec. These (and some more) are added as text files (OpenMetrics format) and JSON files (JSON format). Some of these examples contained inconsistencies with the specification which were corrected in eclipse/microprofile-metrics#555

The added tests only take a few seconds to run so they are no problem for the build process.

Tests Performed

Besides the over 150 new unit tests manual testing was done using the endpoints

http://localhost:8080/metrics/
http://localhost:8080/metrics/base
http://localhost:8080/metrics/vendor
http://localhost:8080/metrics/application
http://localhost:8080/metrics/base/gc.total

To test both JSON and OpenMetrics some browser addon can be used to set the Accept header to application/json and test both GET and OPTIONS for the above URLs.

Running the TCK with updates in payara/MicroProfile-TCK-Runners#103 against local server instance using:

 mvn clean install "-Ppayara-server-remote" "-Dpayara.version=5.202"

Remember that MP_METRICS_TAGS needs to be set to tier=integration when running against remote server using

export MP_METRICS_TAGS=tier=integration

in the shell that is used to start the server.

[jbee]

jenkins test please

[jbee]

jenkins test please

[jbee]

jenkins test please

[Pandrex247]

Some discussion points

[Pandrex247]

How much effort is it to do this?

[jbee]

I opted to not open that box because of the change of behaviour it might bring. I don't think it is much work but it is this ugly inconsistency in CDI where you cannot ask for the Bean that is intercepted. Weld has a non standard way around it which allows to access the CDI level abstraction which has the correct picture. To be clear: this is just the difference between dynamic adds/removes of annotations on CDI level considered or not.

[jbee]

jenkins test please

[Pandrex247]

Points discussed.