-
Notifications
You must be signed in to change notification settings - Fork 78
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
[WFLY-19846] Migrate MP Tel 2.0 from PREVIEW to DEFAULT #624
base: main
Are you sure you want to change the base?
Conversation
|
||
=== Related Issues | ||
|
||
- https://issues.redhat.com/browse/EAP7-2227[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't recall if there was a decision to stop adding these links in the analysis doc. @darranl, @bstansberry or @jmesnil do you recall?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, these are useless for community. They are just convenience for us so we don't have to open the main issue and then click the linked EAP7 issue.
|
||
- https://issues.redhat.com/browse/WFLY-19846[] | ||
|
||
=== Related Issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that telemetry PR adds grpc dependency here. Is this telemetry RFE blocked by grpc RFE? (https://issues.redhat.com/browse/EAP7-1819 / https://issues.redhat.com/browse/WFLY-15495 ) If yes, I believe that it should be stressed out here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marekkopecky No, it doesn't, this is just an internal dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your feedback! I'm not sure whether I understand. Is this dependency in module.xml not used in module.xml in built WF?
|
||
=== User Stories | ||
|
||
User wants to deploy an application using MicroProfile Telemetry 2.0 from MicroProfile Platform 7.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please elaborate how users can benefit from new features brought by MP Telemetry 2.0? https://microprofile.io/specifications/telemetry/2-0/ describes new features against MP Telemetry 1.1 that we have earlier
=== Affected Projects or Components | ||
|
||
- MicroProfile Fault Tolerance | ||
- AMQP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How this WF feature affects AMQP or Kafka?
|
||
== Backwards Compatibility | ||
|
||
* There are no known breaking changes. The upstream OpenTelemetry project made a number of breaking changes to the metrics semantic conventions (which could affect user dashboards), but metrics support will be new with this release, so those changes will not affect existing installations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MP Telemetry main page explicitelly mentioned two backward incompatible changes against MP telemetry 1.1 that we used earlier. Can you please double-check this?
https://microprofile.io/specifications/telemetry/2-0/
It could require additional MTA work ...
|
||
=== Deployments | ||
|
||
Deployment behavior will be unaffected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that any work/checks for some env variables needs to be changed, see "Incompatible changes" section in https://microprofile.io/specifications/telemetry/2-0/
[[test_plan]] | ||
== Test Plan | ||
|
||
There are a number of tests already in WildFly and various downstream repositories. The WildFly test suite will be modified to add test coverage for the newly added observability signals, metrics and logging. Those tests reside in `testsuite/integration/microprofile`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you going to add tests for observability signals and logging to https://github.com/wildfly/wildfly/pull/18305/files ? Or is there some another tracker for this?
This change will have no effect on security. As stated in the MicroProfile Telemetry 1.0, there is no baked in notion of security with OpenTelemetry, so systems administrators will need to make sure that the endpoint for the push publication is properly configured, and that the receiving system -- whatever it may, e.g. and OpenTelemetry Collector -- is secured as desired and appropriate for that system. Such a configuration is outside the scope of the work here. | ||
|
||
[[test_plan]] | ||
== Test Plan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add "Manual tests", "Miscellaneous checks" and "Compatibility tests" sections here according to the template?
wildfly-proposals/design-doc-template.adoc
Line 145 in 790b15b
*** Manual tests: briefly describe checks to be performed during one-time exploratory testing. The purpose of this testing is to check corner cases and other cases that are not worth implementing as automated tests. Typical checks are: bad configurations are easy to reveal, attribute descriptions and error messages are clear, names are descriptive and consistent with similar resources, default values are reasonable. |
@jasondlee Thank you for this analysis! I left a few comments here and there, I would appreciate if you can provide some feedback on them. |
Add analysis document
Resolves #619