[WFCORE-4336] Add the ability for a user to add custom filters for loggers and log handlers #3908
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wildfly-ci
added
the
deps-ok
bstansberry
added
the
missing-reqs
bstansberry
removed
the
missing-reqs
bstansberry
reviewed
Aug 30, 2019
| FilterConfiguration configuration = logContextConfiguration.getFilterConfiguration(name); | ||
| final String className = CLASS.resolveModelAttribute(context, model).asString(); | ||
| final ModelNode moduleNameNode = MODULE.resolveModelAttribute(context, model); | ||
| final String moduleName = moduleNameNode.isDefined() ? moduleNameNode.asString() : null; |
There was a problem hiding this comment.
No need to change this but in the future you can do
final String moduleName = MODULE.resolveModelAttribute(context, model).asStringOrNull();
There was a problem hiding this comment.
I saw that on a PR the other day and was quite surprised I didn't know about it :)
bstansberry
reviewed
Aug 30, 2019
| package org.jboss.as.logging.filters; | ||
|
|
||
| import static org.jboss.as.logging.CommonAttributes.CLASS; | ||
| import static org.jboss.as.logging.CommonAttributes.MODULE; |
There was a problem hiding this comment.
@jamezp While reviewing this I noticed the definitions of these is incorrect, at least for this use case and the other ones I could see. So I filed https://issues.jboss.org/projects/WFCORE/issues/WFCORE-4631. Not necessary to correct in this PR as it is a pre-existing issue.
There was a problem hiding this comment.
That seems correct. I'll get a fix for that probably today.
bstansberry
approved these changes
Aug 30, 2019
bstansberry
added
the
ready-for-merge
jamezp
added a commit
to jamezp/wildfly-core
that referenced
this pull request
Aug 30, 2019
[WFCORE-4336] Add the ability for a user to add custom filters for loggers and log handlers.
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA: https://issues.jboss.org/browse/WFCORE-4336
Model Upgrade JIRA: https://issues.jboss.org/browse/WFCORE-4591
Feature JIRA: https://issues.jboss.org/browse/EAP7-1199
For testing with the subsystem tests I added some modules to the subsystem test resources. I'm not sure whether or not this was a good idea as these will require maintenance. However there is a potential to catch issues with unit tests which seems like a decent trade-off for now.
For the integration test I also add a module. I didn't see, or I'm unaware of, a standard way of doing this. If I missed that please let me know and I'll fix it.
I also went with using two attributes
constructor-propertiesandpropertiesfor configuring the filter. I had consideredpropertieswith a boolean to indicate whether or not this was a constructor property, however this seemed less user friendly as the stand key/value pair properties are already widely used and likely understood.