-
Notifications
You must be signed in to change notification settings - Fork 80
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-17510] Convert MicroProfile OpenTracing subsystem to Admin-Only Mode #512
Conversation
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.
Thanks for this proposal @jasondlee!
Just a few minor comments, let me know what you think.
|
||
=== Default Configuration | ||
|
||
The default configuration for MicroProfile configurations will be altered to remove the MicroProfile OpenTracing subsystem. Likewise, the relevant Galleon layers will also have the extension removed from their definitions. |
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.
Is it Galleon layers or feature packs? I am not sure though, just asking.
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 think, both actually. There are layer-spec.xml
files, as well as feature pack definitions. @bstansberry can chime in if I'm off base 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.
It's layers.
The feature pack still incorporates MP OT as a FP incorporates everything that can get provisioned, and MP OT can get provisioned. The galleon content used for the default configs and the layers are what drive whether MP OT shows up in the configuration files. That's what we want to change.
That said, the words "the relevant Galleon layers" should be replaced with an explicit list of the layers that are going to change.
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'll update that text to include those layers.
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.
@bstansberry @fabiobrz Edited the text. Is that better?
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.
Yep, I think it solves Brian's concerns.
Analysis doc for MP OpenTracing Removal
Add layer information to the proposal
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.
Thanks for addressing my comments @jasondlee - the proposal looks good from the QE perspective, approving.
Great. Thanks for all the help @fabiobrz! |
https://issues.redhat.com/browse/WFLY-17510
Analysis doc for MP OpenTracing Removal