Support for MP Metrics 2.0 #992

tjquinno · 2019-09-05T18:00:38Z

Resolves #833 (MP Metrics 2.0) and #939 (verify new metrics with FT).

These changes add support for MP metrics 2.0, including a Helidon bundle for MP 3.0.

The following new modules support the new version of metrics:

metrics2
    metrics2
    prometheus2
microprofile
   metrics2

Their original counterparts support the previous metrics version 1.1, as before:

metrics
   metrics
   prometheus
microprofile
   metrics

This PR updates examples, tests, and TCK drivers to use the newer metrics version with these exceptions:

MP static content example
MP security example
MP IDCS security example
zipkin integration test

Because MP Metrics 2.0 is incompatible with 1.1, Helidon allows developers with existing metrics-based apps to use the latest Helidon release with the older MP metrics version by depending on the helidon-microprofile-2.2 bundle.

Helidon SE apps that use metrics can continue to depend on the metrics/metrics artifact to use the older metrics implementation. Or, to use the newer SE metrics implementation, they can depend on metrics2/metrics2.

Helidon contains the following internal metrics clients:
fault tolerance
grpc metrics

These have been changed so they can work with either version of metrics.

To accomplish this a new metrics abstraction layer exists in the new module common/metrics, a separate module because the abstraction layer is used by more than one module. Therefore its key contents have to be public which is unfortunate. This abstraction layer is not intended for developers using Helidon so its public classes and interfaces have Internal in their names to discourage developers from using them.

…roduced here. A lot of TODO's added for now. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

…try. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

…sistent.

… copyright issues. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

…ities in Registry about passing null to varargs argument

…he registry

…name, not a single metric of that name with no tags

… was not replaced, and tag qualifiers were added to the metric name instead of to each value name within the metric (when the metric had multiple values such as max, min, count)

…mentation of concurrent gauges. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

tjquinno · 2019-09-05T19:15:39Z

BTW, the wercker/build failure is due to wercker timing out, not due to errors in the build.

…e helidon-microprofile-2.2 bundle to use MP metrics 1.1 (instead of the new, backward-incompatible 2.0) API *without* having to do anything else to use the correct MP metrics API

tomas-langer · 2019-09-08T14:05:55Z

Currently the build error is because of failing TCK tests of metrics 2.0

tjquinno · 2019-09-08T15:31:18Z

The TCK failures are known. I've been working on some things and have changed the title to WIP to reflect that.

…tted the fix for Metered

…etricsCdiExtension

…Impl

@Metric

…d metadata more separately; revise metadata consistency checking during re-registration; revise reuse enforcement during metric look-up; suppress CDI-triggered registration of metrics for subclasses due to annotations on superclass methods; implement name-based lookup (rather than complete metadata consistency checking) of registrations due to @Metric annotations

tjquinno · 2019-09-09T03:57:16Z

Removed WIP from the title. TCKs passed locally for me so they should in the pipeline now also.

spericas · 2019-09-09T17:17:22Z

Just to clarify, the TCKs were passing, then an issue was found and some fixes cause the TCKs to fail temporarily. After Tim's latest commit all TCKs are passing again like at the time the PR was submitted.

… old 2.2 bundle in Helidon. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

tomas-langer · 2019-09-10T11:43:24Z

metrics/metrics/src/main/java/io/helidon/metrics/FinalRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019 Oracle and/or its affiliates. All rights reserved.


Seems that only copyright was changed

There were intervening changes along the way that turned out to be unnecessary. Leaving the copyright changed was an oversight. We'll revert this and other similar files.

Working on this. Still haven't found a way to restore a version with the correct copyright that also passes the copyright checker.

I have heard from others who have wrestled with this same problem in other projects. They have also not found a way to solve this. So it seems we might have to live with copyrights that are later than the actual changes in the 8 (I think) affected files. This is unfortunate but not easily avoided at this point.

tomas-langer · 2019-09-10T11:43:46Z

metrics/metrics/src/main/java/io/helidon/metrics/HelidonCounter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019 Oracle and/or its affiliates. All rights reserved.


Seems that only copyright was changed.

Seems we need to live with this.

tomas-langer · 2019-09-10T11:43:55Z

metrics/metrics/src/main/java/io/helidon/metrics/HelidonGauge.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019 Oracle and/or its affiliates. All rights reserved.


Seems that only copyright was changed.

Seems we need to live with this.

tomas-langer · 2019-09-10T11:45:39Z

metrics/metrics/src/test/java/io/helidon/metrics/HelidonCounterTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019 Oracle and/or its affiliates. All rights reserved.


Seems that only copyright was changed.

Seems we need to live with this.

tomas-langer · 2019-09-10T11:46:03Z

metrics/metrics/src/test/java/io/helidon/metrics/HelidonHistogramTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019 Oracle and/or its affiliates. All rights reserved.


Seems that only copyright was changed.
There are many cases like this, please revert all.

Seems we need to live with this. See the extensive notes on the first comment about copyrights above.

metrics2/metrics2/src/main/java/io/helidon/metrics/ExponentiallyDecayingReservoir.java

metrics2/metrics2/src/main/java/io/helidon/metrics/EWMA.java

tomas-langer · 2019-09-10T12:29:22Z

metrics2/metrics2/src/main/java/io/helidon/metrics/InternalMetricIDImpl.java

+        if (!Objects.equals(this.delegate, other.delegate)) {
+            return false;
+        }
+        return true;


Last four lines can be replaced with
return Objects.equals(this.delegate, other.delegate)

Fixed in a pending commit.

Pushed. Similar change also in the metrics/metrics impl class of the same name.

tomas-langer · 2019-09-10T12:35:59Z

metrics2/prometheus2/pom.xml

@@ -0,0 +1,64 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+


Why is there a Prometheus 2 - this does not depend on MP metrics at all, only on prometheus itself.
This module should not have been duplicated.

See the earlier comment about ease of removing the older metrics and microprofile/metrics modules later.

… classes

…-api artifact from example poms

tomas-langer · 2019-09-10T16:18:20Z

metrics2/pom.xml

@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--


This module should not exist.
Please move metrics2 submodule to $root/metrics
The resulting structure should be:
metrics/metrics
metrics/metrics2
metrics/prometheus

The groupId is already following this, so the directory structure should as well.
Also Prometheus should not be touched by this change at all.

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

tomas-langer

LGTM

spericas and others added 30 commits August 9, 2019 10:08

Switching to Metrics 2.0.1. Lots of backward incompatible changes int…

1387be0

…roduced here. A lot of TODO's added for now. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Updates to Registry class for 2.0.

373022a

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Initial support for concurrent gauges.

b512348

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Fixed a few problems related to unit tests. All tests are passing now.

a2f07ff

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Update JSON format, including underlying changes

8390246

Fix import order offense

bede467

Change new static method jsonFullKey to non-public

fb89577

Fixed problem with concurrent gauges and created some unit tests.

a3b5513

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Test changes related to Metrics 2.0 API.

8cff9a9

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Test changes related to Metrics 2.0 API.

dd61e93

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Completed implementation of FinalRegistry. Some minor tweaks to Regis…

622ad42

…try. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Escape special characters in JSON output

c6ed971

Incorporate parallel changes in metrics-2.0

288b95a

Merge FinalRegistry changes; update copyrights

46c796e

Update to OpenMetrics (Prometheus) format

167c013

Metrics created via CDI should not be reusable. Make log messages con…

99d9996

…sistent.

Restore use of : in a Prometheus name in tests

babc12e

Changes to Registry implementation based on 2.0 semantics. Fixed some…

aeb3db2

… copyright issues. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Merge with metrics-2.0

ee9dbe7

Disable prom. format test until we resolve tags issue

e02c8a9

New implementation of concurrent gauge.

53bdb7e

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Set default unit to MetricUnits.NONE in constructor.

5556562

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Use concurrency in test for ConcurrentGauge.

982d719

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

Add tags-related logic to metrics CDI extension; correct a few ambigu…

0964210

…ities in Registry about passing null to varargs argument

Missed some null varargs instances

45baf5c

Remove strict checking of unit when adding/searching for metrics in t…

013f52f

…he registry

registry search by name string needs to return all metrics with that …

cbfe782

…name, not a single metric of that name with no tags

Handle tags in the various produceXXX methods

99b5e7b

Fix formatting errors in JSON output; semi-colon in tag name or value…

dd92eaf

… was not replaced, and tag qualifiers were added to the metric name instead of to each value name within the metric (when the metric had multiple values such as max, min, count)

Support for @ConcurrentGauge annotation and some changes to the imple…

d286120

…mentation of concurrent gauges. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

tjquinno self-assigned this Sep 5, 2019

Fix checkstyle violation in example

47cb608

tjquinno added 2 commits September 5, 2019 19:00

Allow developers to use the upcoming Helidon release but depend on th…

4f275f9

…e helidon-microprofile-2.2 bundle to use MP metrics 1.1 (instead of the new, backward-incompatible 2.0) API *without* having to do anything else to use the correct MP metrics API

Merge with concurrent changes to main repo

e3b9cb9

tjquinno added 2 commits September 6, 2019 18:32

Fix reusability bugs and add tests for metrics2; metrics1 to follow

a531102

Improve metric reusability violation test

b5e44cf

tjquinno changed the title ~~Support for MP Metrics 2.0~~ WIP: Support for MP Metrics 2.0 Sep 8, 2019

tjquinno added 4 commits September 8, 2019 17:39

Earlier fix of incorrectly ignoring reusable value on annotations omi…

28fad50

…tted the fix for Metered

Add missing setting of metadata reusability from annotations in 1.1 M…

355dcfc

…etricsCdiExtension

Set reusability based on metadata passed to constructor in 1.1 Metric…

119e056

…Impl

tjquinno changed the title ~~WIP: Support for MP Metrics 2.0~~ Support for MP Metrics 2.0 Sep 9, 2019

tjquinno mentioned this pull request Sep 9, 2019

The @Counted metric is always reusable #1001

Closed

FT TCKs must run as a MP 2.2 application. Changed pom to point to the…

e99cb72

… old 2.2 bundle in Helidon. Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

spericas mentioned this pull request Sep 9, 2019

Verify compatibility of new Metrics implementation with FT #939

Closed

tomas-langer requested changes Sep 10, 2019

View reviewed changes

tjquinno added 2 commits September 10, 2019 09:08

Review-inspired changes to equals method in both InternalMetricIDImpl…

0e3e55a

… classes

Remove no-longer-needed explicit dependencies on the newer MP metrics…

9bc1b79

…-api artifact from example poms

tomas-langer reviewed Sep 10, 2019

View reviewed changes

Removed duplicated module metrics2/prometheus2.

811cf89

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

tomas-langer approved these changes Sep 10, 2019

View reviewed changes

spericas approved these changes Sep 10, 2019

View reviewed changes

tjquinno merged commit 7708105 into helidon-io:master Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for MP Metrics 2.0 #992

Support for MP Metrics 2.0 #992

tjquinno commented Sep 5, 2019

tjquinno commented Sep 5, 2019

tomas-langer commented Sep 8, 2019

tjquinno commented Sep 8, 2019

tjquinno commented Sep 9, 2019

spericas commented Sep 9, 2019

tomas-langer Sep 10, 2019

tjquinno Sep 10, 2019

tjquinno Sep 10, 2019

tjquinno Sep 10, 2019

tomas-langer Sep 10, 2019

tjquinno Sep 10, 2019

tomas-langer Sep 10, 2019

tjquinno Sep 10, 2019

tomas-langer Sep 10, 2019

tjquinno Sep 10, 2019

tomas-langer Sep 10, 2019

tjquinno Sep 10, 2019

tomas-langer Sep 10, 2019

tjquinno Sep 10, 2019

tjquinno Sep 10, 2019

tomas-langer Sep 10, 2019

tjquinno Sep 10, 2019

tomas-langer Sep 10, 2019

tomas-langer left a comment

Support for MP Metrics 2.0 #992

Support for MP Metrics 2.0 #992

Conversation

tjquinno commented Sep 5, 2019

tjquinno commented Sep 5, 2019

tomas-langer commented Sep 8, 2019

tjquinno commented Sep 8, 2019

tjquinno commented Sep 9, 2019

spericas commented Sep 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomas-langer left a comment

Choose a reason for hiding this comment