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

Mark Metrics 5.0 as optional #292

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dblevins
Copy link
Contributor

As discussed in the latest technical call, a potential PR to mark Metrics 5.0 as optional so Red Hat can implement MP 6.0 and future versions of MP.

This leaves Metrics as included in the umbrella, but marks it as optional. This is in the spirit of how Jakarta EE Platform spec has several optional aspects such as EJB CMP, RMI-IIOP, etc. which were fully required but eventually marked optional to accommodate more implementations.

Ideally, we only put things in the Optional APIs section if 1) they were once required, 2) the majority of impls still will ship it, and 3) we know there are no conflicts such as TCK with other component specs in the umbrella spec.

@edbratt
Copy link

edbratt commented Nov 17, 2022

I believe that this would weaken the customer expectation of MicroProfile -- with this change, customers will need to check every vendor and their compatibility status -- to determine if it contains standardized metrics, tracing or something else. To me, this lowers the confidence around MicroProfile.

This was discussed at length earlier in the release development cycle and those discussions concluded with the release content remaining as originally planned, prior to this proposed change. Vendor participation is crucial to the development of our specifications and it is important for us to work out our different perspectives and that is what we should continue to do though not by such a large, unplanned change on the eve of the release.

Helidon team has identified FaultTolerance TCK tests that have dependencies on Metrics -- these may also be improper tests and our analysis is certainly preliminary but they could present Test Challenge problems related to this change. It also seems we will not likely have a CI that can demonstrate we have sorted all the possible inter-dependencies for an implementation that does NOT include Metrics by the time this release is currently scheduled to go to final release review (in 5 days).

We would prefer that the working group continue trying to resolve these issues in a moving forward fashion -- not by carving out portions of the specification in this fashion. For example, the working group might decide that all the derived specifications (e.g. metrics, tracing, Open API ...) be recast as integration guidelines rather than specifications -- but this is something for careful analysis and consideration -- with time for evaluation. Not on the eve of a major feature release.

We had been in favor of the MicroProfile 6 release up until this proposal, introduced this week. Regretfully, proceeding forward with this change, at this time, for MicroProfile 6, with a release ballot starting by Tuesday, Nov. 22 would not be backed by Oracle.

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Nov 17, 2022

I have thought this more carefully after the discussion and also discussed with other teams. We also concluded this will weaken MicroProfile 6.0 and will not support this either. If we start doing this, gradully more and more specifications will be made optional, then there will have little value to have the compatibility programme nor the releases. The specification should have a high standard to ensure portability and maintain customer satisification. +1 on what Ed said. We were happy with the agreed plan review laid out.

Copy link
Member

@Emily-Jiang Emily-Jiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added my comment and will not approve this change for the reason I explained in my comment.

@donbourne
Copy link
Member

When we decided to proceed on the metrics spec earlier this year, it was with due consideration of the significance of OpenTelemetry metrics and Micrometer in the industry. It wasn't a decision made lightly to move forward, and efforts were made to determine if our users would use a new MP Metrics API backed by Micrometer or OpenTelemetry (for example https://twitter.com/emilyfhjiang/status/1502587551139389440).

The majority of MP members expressed support of our direction as recently as August 24 (the MP Metrics 5.0 plan review). I don't feel that any new information has been added that should make us revisit that at this point.

We've done what we can to make it easy for vendors to implement the spec, including providing a Micrometer-based open source implementation in Smallrye Metrics (https://github.com/smallrye/smallrye-metrics/releases/tag/5.0.0-M5) that has been tested in TomEE and Open Liberty. This implementation lets app developers code to the MP Metrics API while taking advantage of any of the 18 supported Micrometer monitoring backends, with the convenience of MP Config-based configuration for each.

My preference would be that we include MP Metrics 5.0 in MP 6.0 without declaring it optional, and continue to check in with our user community over the next several months to see if they are happy using it in their microservices (as we should with all of our APIs).

@jclingan
Copy link
Contributor

jclingan commented Nov 18, 2022

@Emily-Jiang, Reducing the number of compatible implementations (by three) also weakens MicroProfile 6.0. I really think we should accept this request. We are not aligned on this and I feel like I should approve this request and let it go to a vote.

@radcortez
Copy link
Contributor

When we decided to proceed on the metrics spec earlier this year, it was with due consideration of the significance of OpenTelemetry metrics and Micrometer in the industry. It wasn't a decision made lightly to move forward, and efforts were made to determine if our users would use a new MP Metrics API backed by Micrometer or OpenTelemetry (for example https://twitter.com/emilyfhjiang/status/1502587551139389440).

I'm sorry, but I have to disagree with a personal Twitter pool being used as any evidence to make decisions about the direction of any specification. This was pointed out in the technical calls when discussing MP Metrics.

Red Hat made it clear in an email thread about our position regarding MP Metrics:
https://groups.google.com/g/microprofile/c/k85_2po0jh4/m/JQ44AnnyAQAJ

Technically speaking, it seems odd that for Tracing we allow our users to use the OTel API directly, but for Metrics, we are forcing them into our API. Tracing and Metrics (and Logging) are not isolated; they must be coherent and consistent across the board.

Our stance did not change. We will keep supporting MicroProfile, but unfortunately won't be able to claim platform compatibility with any of our products with the current MicroProfile 6.0 specification.

@m0mus
Copy link
Member

m0mus commented Nov 18, 2022

I think that Metrics should not be moved to optional. Doing it in favor of one vendor plays against vendor neutrality.

@Emily-Jiang
Copy link
Member

It is fine for some vendors not to fully agree on what the specification heads towards. We need to agree on disagreement, which is why we need a ballot. Please note the ballot for MicroProfile Metrics 5.0 and MicroProfile 6.0 were approved successfully.

@edbratt
Copy link

edbratt commented Nov 19, 2022

@Emily-Jiang, Reducing the number of compatible implementations (by three) also weakens MicroProfile 6.0. I really think we should accept this request. We are not aligned on this and I feel like I should approve this request and let it go to a vote.

I would argue the opposite -- proceed with the original proposal. Regardless that outcome, I think we can all agree this issue is important for us to resolve as a community.

A couple of points:

  • MicroProfile 5 was about the migration from javax to jakarta namespace.
  • MicroProfile 6 was intended to codify the use of Jakarta EE Core Profile (and, there was debate on this point but this is where the community settled).
  • Metrics was evolved and Telemetry was added as an incremental adjustment in reaction to concerns raised by vendors.
  • The required API status of Metrics is not different from the previous MicroProfile releases.

To my eye, it seems, from the list of changes proposed for Metrics 5, that implementation of Metrics 5 over Metrics 4 (included in MicroProfile 5) would largely involve the removal of APIs previously required, with only a few additional APIs added. If that observation is accurate I struggle to see how the requirements of MicroProfile 6 are posing a higher bar than was required in MicroProfile 5.

Up until the discussion on Tuesday this week, it seemed to me, we were ready to go to a successful release ballot (though I realize it's never over until it's over). I will reiterate -- we had previously discussed all of this and I thought we had reached a point where we all felt there could be progress. Therefore, it is difficult for me to reconcile this magnitude of change, being required on this day.

Unchanged, this would be the conclusion of the plans we laid out and approved (by community ballots), earlier this year. If the community has decided that these plans are no longer relevant. If we are truly at loggerheads, I suppose the one thing we could do is abandon the proposal to bring this to ballot -- initiate something that allows us to revise the plan -- I guess that would be a progress review -- then once that is settled, proceed with a new revised plan. Unfortunately, that would not happen this calendar year.

This would be disappointing as I had thought we had achieved sufficient consensus and had found an incremental point, from which we could continue to evolve MicroProfile.

@rdebusscher
Copy link
Member

Either Metrics is removed or stays required. Optional goes against the idea that an application compiled against a spec (MP 6.0 umbrella) can be deployed on any compatible/certified product.

@edburns
Copy link

edburns commented Nov 21, 2022

Microsoft supports this proposal. As the only hyperscale cloud
actively participating in MicroProfile it is important that the
hyperscale metrics solution already widely supported on Azure,
OpenTelemetry, is the supported solution in MicroProfile. Microsoft
has no intention of supporting the MicroProfile metrics 5.0.

18:20 EST edit

Microsoft is sympathetic to this proposal, but requests the act of marking Metrics 5.0 as optional be deferred to a future release. As the only hyperscale cloud actively participating in MicroProfile it is important that the hyperscale metrics solution already widely supported on Azure, OpenTelemetry, is the supported solution in MicroProfile. Microsoft has no intention of supporting the MicroProfile metrics 5.0 beyond the existing "Prometheus" endpoint export feature in MP Metrics 5.0

@Emily-Jiang
Copy link
Member

Microsoft supports this proposal. As the only hyperscale cloud actively participating in MicroProfile it is important that the hyperscale metrics solution already widely supported on Azure, OpenTelemetry, is the supported solution in MicroProfile. Microsoft has no intention of supporting the MicroProfile metrics 5.0.

At the moment, there is no OpenTelemetry Metrics available in MicroProfile as yet. We will continue evolving in this space. Until then, MP Metrics is the only solution in MicroProfile. Making MP Metrics optional will put MicroProfile 6.0 plan back to the starting point. I suggest releasing MicroProfile 6.0 as it is and we evolve the technologies in the upcoming releases.

@donbourne
Copy link
Member

@edburns , MP Metrics 5.0 requires implementations to, at minimum, have ability to export metrics through a "Prometheus" endpoint. Cloud providers that support monitoring of metrics using Prometheus (which I believe Azure does) should have no trouble with app servers/runtimes exposing metrics from MP Metrics 5.0.

@starksm64
Copy link
Contributor

It seems to me the simplest resolution is to remove Metrics from the platform for 6.0. Optional is problematic.

@jclingan
Copy link
Contributor

I am open to either removing Metrics from the platform spec or making it optional. Both have merit, IMHO.

@Emily-Jiang Emily-Jiang mentioned this pull request Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants