Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fully enforce reusability in MP Metrics 1.1. support #1048

Merged
merged 15 commits into from
Sep 23, 2019
Merged

Fully enforce reusability in MP Metrics 1.1. support #1048

merged 15 commits into from
Sep 23, 2019

Conversation

tjquinno
Copy link
Member

The code that supports MP Metrics 1.1 does not fully enforce reusability. These changes correct that.

(This PR also addresses some unnecessary references to the internal bridging infrastructure which these classes did not need to use; they can and should depend directly on the MP metrics 1.1 artifacts.)

See issue #1046

@tjquinno tjquinno self-assigned this Sep 20, 2019
@tjquinno
Copy link
Member Author

The (tentative) changes to FT to make the automatically-created metrics reusable are because automatically-derived metric names come from the class name and method name of the annotated method. In the FT unit tests, the CircuitBreakerBean uses FT on two overloaded variants of exerciseBreaker. Both methods yield the same automatic metric name, so if the corresponding metric is not set up as reusable then, now that the registry code is enforcing the reusability, FT tries to reregister the same metric twice which fails.

…ounter(String) method should retrieve an existing metric, regardless of reusability set in previously-stored metadata. The original test code should work; we need to work on the registry methods that accept a name so they do a look-up before trying to reregister the metric.
@tjquinno
Copy link
Member Author

tjquinno commented Sep 23, 2019

Note that the preceding changes to MetricsSupport register the vendor metrics as reusable. This is needed because the TCK starts the server multiple times, which causes the MP services to be loaded multiple times, but there is no life cycle support in the server for services to respond to server shutdown so they can clean up. With the fixes to reusability enforcement, and with reusability being false by default in metrics 1.1, this caused failures in the TCK as the vendor metrics were reregistered. Specifying these metrics as reusable in 1.1 is consistent with our 2.0 implementation in which programmatically-created metrics (as these are) are reusable by default.

@tjquinno tjquinno changed the title WIP: on-the-shelf Fix for incomplete reusability enforcement in MP Metrics 1.1. support Fully enforce reusability in MP Metrics 1.1. support Sep 23, 2019
@tjquinno tjquinno added bug Something isn't working metrics MP P1 SE labels Sep 23, 2019
@tjquinno tjquinno requested a review from spericas September 23, 2019 15:19
@tjquinno tjquinno merged commit 719aa16 into helidon-io:master Sep 23, 2019
@tjquinno tjquinno deleted the fix-1.1-reusability branch September 30, 2019 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working metrics MP P1 SE
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants