[WFLY-17137] MicroProfile with Jakarta EE10 #499
Conversation
| |4.0 | ||
| |n | ||
|
|
||
| |JWT |
There was a problem hiding this comment.
@kabir I took a look if there are any tests we should add to our testsuites for new functionality and changes introduced by JWT 2.1 and OpenAPI 3.1.
I looked at the release notes for the JWT 2.1 and checked if each new functionality is used anywhere in their TCK tests folder, which it is.
For OpenAPI 3.1, they are implementing tests for new functionality according to the test strategy listed here and it does contain test for each of the changes. So seems that there is nothing needed to be added to our testsuites for this now
There was a problem hiding this comment.
Regarding JWT, the new properties mp.jwt.verify.token.age and mp.jwt.verify.clock.skew properties do change the behavior of the server, right? In such a case I am wondering whether we're happy with just the TCKs being run for this specific case or we'd want to cover an actual integration scenario.
Just checking here, I don't have a strong opinion yet.
On a different side, the addition is being being covered by TCKs but authors state that the mp.jwt.verify.clock.skew property isn't tested, and by looking at the code I assume this is because the default is used.
What about us? Does the leeway behavior require for additional test coverage?
There was a problem hiding this comment.
@fabiobrz @fjuma the mp.jwt.verify.token.age is tested in TokenAgeTest. It has 2 tests, 1 that uses Thread.sleep to invalidate token and checks if the connection is unauthorized, and second test doesn't set timeout and checks that the connection is successful. To me these 2 tests seem sufficient for us. But Fabio is right that the mp.jwt.verify.clock.skew is not being tested, we should add a test for it then
There was a problem hiding this comment.
+1 on the fact the first property is being tested properly already.
the mp.jwt.verify.clock.skew is not being tested, we should add a test for it then
@Skyllarr, which JIRA issue will be tracking the test implementation?
There was a problem hiding this comment.
There was a problem hiding this comment.
This seems fine from REST Client perspective. I've upgraded the implementation to use Jakarta EE 10 dependencies even though the current MP specification does not. However, MP 6 says it targets the Jakarta EE 10 specification so it seemed best to do that. I can do a release, Beta to start, for the client implementation when needed.
| |Health | ||
| |4.0 | ||
| |4.0 | ||
| |4.0 |
There was a problem hiding this comment.
except for minor (no functional change) upgrade if needed nothing changed
| |- | ||
| |- | ||
| |1.0 | ||
| |y - |
There was a problem hiding this comment.
The versions/plans for my components are all correct.
| :idseparator: - | ||
|
|
||
| == Overview | ||
| This issue is about upgrading to most parts of MicroProfile Platform 6, which is aligned with Jakarta EE 10. 'most' is used since a decision has been made to not include the new MicroProfile Metrics version, as the inclusion of Micrometer tracked by https://issues.redhat.com/browse/EAP7-1686[EAP7-1686] provides our observability strategy. |
There was a problem hiding this comment.
Please use a WFLY JIRA here.
Micrometer is only in WildFly Preview right now so @jasondlee can tell you the JIRA that tracks inclusion in standard WildFly.
There was a problem hiding this comment.
The JIRA to move Micrometer to standard WF is here https://issues.redhat.com/browse/WFLY-17144
| There will be a separate RFEs for: | ||
|
|
||
| * Removal of MicroProfile Metrics (https://issues.redhat.com/browse/WFLY-17138[WFLY-17138]) | ||
| * Addition of MicroProfile Telementry (https://issues.redhat.com/browse/WFLY-17156[WFLY-17156]) |
There was a problem hiding this comment.
s/Telementry/Telemetry
There was a problem hiding this comment.
@kabir @jasondlee If one doesn't exist we need a WFLY for removing MP OT, and discussion of it here similar to what is done re: MP Metrics.
There was a problem hiding this comment.
I don't think either of those JIRAs exist, so I'll file them now.
There was a problem hiding this comment.
|
|
||
| === Non-Requirements | ||
|
|
||
| * MP Metrics will no longer be included, and thus not upgraded be to MP Metrics 5.0 |
|
|
||
| == Backwards Compatibility | ||
|
|
||
| * Users will no longer be able to use MicroProfile Metrics APIs and annotations |
|
|
||
| === Default Configuration | ||
|
|
||
| * MicroProfile Metrics will be removed from shipped configurations that currently include it |
|
|
||
| === Importing Existing Configuration | ||
|
|
||
| * A user will not be able to use a configuration containing MP Metrics |
There was a problem hiding this comment.
'except in an admin-only server.'
There was a problem hiding this comment.
A user will not be able to use a configuration containing MP OpenTracing except in an admin-only server or a domain-mode Domain Controller that manages secondary Host Controllers running previous releases.
|
|
||
| === Deployments | ||
|
|
||
| * Deployments will no longer expose data via MicroProfile Metrics |
There was a problem hiding this comment.
Deployments will no longer be able to configure tracing spans via MP OpenTracing.
| //// | ||
| == Release Note Content | ||
|
|
||
| MicroProfile specifications have been updated to the versions that are part of MicroProfile Platform 6, with the exception of MicroProfile Metrics which has been droppped. MicroProfile Metrics has been dropped in favour of integration with Micrometer, which offers improved observability functionality. |
| |3.0 | ||
| |3.0 | ||
| |3.1 | ||
| |TODO: ?? |
There was a problem hiding this comment.
This is a trivial upgrade.
| |Fault Tolerance | ||
| |4.0 | ||
| |4.0 | ||
| |4.0 | ||
| |n |
There was a problem hiding this comment.
I believe this is correct; i.e. MP FT has had no changes since MicroProfile 5.0.
kabir
force-pushed
the
WFLY-17137-jakarta-ee10
branch
from
088545a to
b067beb
Compare
| === Testing By | ||
| // Put an x in the relevant field to indicate if testing will be done by Engineering or QE. | ||
| // Discuss with QE during the Kickoff state to decide this | ||
| * [x] Engineering |
There was a problem hiding this comment.
Hi @kabir - Is it actually going to be tested by ENG?
There was a problem hiding this comment.
Hi @kabir - I've dropped some comments behind.
My goal here is to understand if/how changes in the specs, implementation and those coming as a consequences of specs removal or addition will affect the server behavior.
Then I'd say we also need to understand how to deal with the MicroProfile test suite executions, since that is currently used to validate Wildfly and EAP behavior, even in case we decide that we'll run it just for covering regressions and that new functionality is covered by the TCKs.
WDYT?
| |OpenAPI | ||
| |3.0 | ||
| |3.0 | ||
| |3.1 |
There was a problem hiding this comment.
I've got a question, here.
There's https://issues.redhat.com/browse/WFLY-17496 which is a blocker since it marks a regression in behavior when compared to previous versions.
The component issue has been fixed, and it was initially queued in the 3.1.3 milestone, which would have met the requirements defined above, i.e. the expected minor to be 3.1.
Now I can see the issue has been queued for the 3.2.0 milestone (actually it looks like the milestone was renamed), so I am wondering whether we should first understand if something is going to be released soon, and which minor version it will be, e.g.: the main version is still being set to 3.1.3-SNAPSHOT.
Then we could understand whether an update is actually needed here.
WDYT @pferraro, CC @kabir ?
There was a problem hiding this comment.
As discussed with @fabiobrz in chat, the versions in this table are MP spec versions and not SmallRye implementation versions.
Fabio will work with the SmallRye Open API team to get a SmallRye Open API release for MP OpenAPI with the mentioned blocker resolved. (My guess is via a backport of the fix to their 3.1.x branch, but I am not familiar with their versioning)
There was a problem hiding this comment.
I just checked and there's no 3.1.x SR OpenAPI branch, hence it seems we cannot backport the fix unless a 3.1.3 branch is generated.
3.2.0 was released recently that include support for MP OpenAPI 3.1, so we could evaluate upgrading to 3.2.0, see wildfly/wildfly#16555
There was a problem hiding this comment.
wildfly/wildfly#16555 has been merged into WildFly. It bumps SR OpenAPI to 3.2.0, which supports MP OpenAPI 3.1, matching this proposal expectations.
It also upgrades jackson-dataformat-yaml to 2.14.2.
| |4.0 | ||
| |n | ||
|
|
||
| |JWT |
There was a problem hiding this comment.
Regarding JWT, the new properties mp.jwt.verify.token.age and mp.jwt.verify.clock.skew properties do change the behavior of the server, right? In such a case I am wondering whether we're happy with just the TCKs being run for this specific case or we'd want to cover an actual integration scenario.
Just checking here, I don't have a strong opinion yet.
On a different side, the addition is being being covered by TCKs but authors state that the mp.jwt.verify.clock.skew property isn't tested, and by looking at the code I assume this is because the default is used.
What about us? Does the leeway behavior require for additional test coverage?
| |y | ||
|
|
||
| |+++<s>Metrics</s>+++ + | ||
| Will not include |
There was a problem hiding this comment.
Wildfly MicroProfile specs integration is currently being tested through TCKs execution and through the MicroProfile test suite execution.
The latter includes tests for the MicroProfile Metrics spec integration, and those will have to be removed from the test suite itself.
This mean we'll dump the complex cases test coverage that we currently have. How do we replace them according to the new specs and implementation? How will the server behavior change, for instance when it comes to the endpoint definitions etc., with the new integration?
Besides, we are replacing the MicroProfile Metrics specs with one portion of the Observability implementation, i.e. Micrometer, see https://issues.redhat.com/browse/WFLY-14947.
Is the test coverage here expected to be based on just TCKs? Which TCKs are being used? The analysis document for the Micrometer integration is not clear enough here, although it is required for users to know.
CC @jasondlee
| |y (trivial) | ||
|
|
||
| |OpenTracing + | ||
| Dropped in MP 6 |
There was a problem hiding this comment.
Wildfly MicroProfile specs integration is currently being tested through TCKs execution and through the MicroProfile test suite execution.
The latter includes tests for the MicroProfile Opentracing spec integration, and those will have to be removed from the test suite itself. Given that it must be updated, then I'd say a mention to it should be added to the Test plan section.
How will the server behavior change from the user point of view?
BTW - This also mean that we'll dump the complex cases test coverage that we currently have. How do we replace them?
CC @jasondlee
| |n | ||
|
|
||
| |Telemetry + | ||
| New in MP 6 |
There was a problem hiding this comment.
How is the Telemetry spec integration going to map to the functionality provided by the MicroProfile OpenTracing spec integration from the user point of view?
I see the analysis document is not expecting for complex test cases to be covered by the Microprofile test suite, is this correct?
CC @jasondlee
|
|
||
| === Issue | ||
|
|
||
| * https://issues.redhat.com/browse/WFCORE[WFCORE-XXXX] |
There was a problem hiding this comment.
Is this some kind of placeholder or can we remove it?
| === Non-Requirements | ||
|
|
||
| * MP Metrics will no longer be included, and thus not upgraded be to MP Metrics 5.0 (https://issues.redhat.com/browse/WFLY-17138[WFLY-17138]) | ||
| * As MicroProfile OpenTracing is remove from MicroProfile Platform 6, it will be dropped (https://issues.redhat.com/browse/WFLY-17510[WFLY-17510]) |
|
|
||
| == Community Documentation | ||
|
|
||
| * Community documentation for the subsystems in question will be updated to reflect the new versions |
There was a problem hiding this comment.
... and any potential differences in behavior, I'd say. WDYT?
kabir
force-pushed
the
WFLY-17137-jakarta-ee10
branch
from
8e2b335 to
a00f4a0
Compare
|
@Skyllarr @jasondlee @fjuma I have updated the analysis document from @fabiobrz 's comments. Since this is in your areas, please take a look. CC @bstansberry |
| * mailto:{email}[{author}] | ||
|
|
||
| === QE Contacts | ||
|
|
There was a problem hiding this comment.
Hi @kabir - I think we can add my name in here as the QE contact.
| * https://issues.redhat.com/browse/WFLY-17156[WFLY-17156] - Implement Support for MicroProfile Telemetry | ||
| * https://issues.redhat.com/browse/WFLY-17510/[WFLY-17510] - Removal of MicroProfile OpenTracing | ||
| * https://issues.redhat.com/browse/WFLY-14947[WFLY-14947] - Addition of Micrometer | ||
| * |
There was a problem hiding this comment.
Shouldn't https://issues.redhat.com/browse/WFLY-17144 be listed in here too?
Hi @jamezp , do we have any trackers for this work that you've been doing? Is it related to RESTEasy? |
| // to that kind of WildFly installation | ||
| * [x] Traditional standalone server (unzipped or provisioned by Galleon) | ||
|
|
||
| * [x] Managed domain |
There was a problem hiding this comment.
@bstansberry @darranl I can never remember. Should managed domain be checked or not for MicroProfile/XP stuff?
There was a problem hiding this comment.
@kabir If history tells us anything, then leave unchecked.
microprofile/WFLY-14798-upgrade-reactive-messaging-2.0.adoc:* [ ] Managed domain
microprofile/WFLY-14932_mp_reactive_messaging_kafka_api.adoc:* [ ] Managed domain
microprofile/WFLY-14940_upgrade-microprofile-health-3.1.adoc:* [ ] Managed domain
microprofile/WFLY-14987_reactive_messaging-ssl-context-kafka-connector.adoc:* [ ] Managed domain
microprofile/WFLY-15832-microconfig-config-root-folder.adoc:* [x] Managed domain
@fabiobrz This was done in https://issues.redhat.com/browse/WFLY-17664 and https://github.com/resteasy/resteasy-microprofile. There weren't really many changes short of component upgrades to the actual source files. |
There was a problem hiding this comment.
Hi @kabir - it seems we're pretty much done with this proposal.
IMO all the relevant information is provided, and the related issue trackers linked.
I've been discussing a bit with @jamezp about the WFLY trackers for updating components' version in WildFly, i.e.:
- JWT (@Skyllarr do we have one? Yep -> https://issues.redhat.com/browse/WFLY-17594)
- OpenAPI (https://issues.redhat.com/browse/WFLY-17601)
- REST client (https://issues.redhat.com/browse/WFLY-17664)
and we were wondering whether we should add them as well to the document. WDYT?
Apart from that, the analysis LGTM, and will approve as soon as we solve this last question.
|
@fabiobrz +1 to adding the upgrade trackers. Though, just to fix the above, the upgrade Jira for OpenAPI is actually https://issues.redhat.com/browse/WFLY-17601 |
Oh, thanks for catching it @rhusar! I'll fix that comment of mine :) |
kabir
force-pushed
the
WFLY-17137-jakarta-ee10
branch
from
f68c69d to
17e1c63
Compare
Mention that MicroMeter and MP Telemetry need to replicate tests removed as part of removal of MP Metrics and MP OpenTracing.
kabir
force-pushed
the
WFLY-17137-jakarta-ee10
branch
from
17e1c63 to
dc9c356
Compare
kabir
force-pushed
the
WFLY-17137-jakarta-ee10
branch
from
dc9c356 to
a778ab0
Compare
https://issues.redhat.com/browse/WFLY-17137
This is the original proposal document, and targets MP Platform 6.0. #537 contains adjustments, in case we are in a position to support MP Platform 6.1.