Closed mandy-chessell closed 3 years ago
Actually, the port messages are coming out ok - so that part is not a problem :)
I am happy to create PR for this. Obviously, issues may be fixed in many ways.
The issue is: We are packaging in server-chassis at least 2 classes with main methods. Default spring-boot-one and the one from open-metadata-types module in OpenMetadataTypesArchiveWriter. As we want to be able to run the OpenMetadataTypesArchiveWriter main outside the server-chassis and silent logging, we added configuration file in the resource file. But this configuration file affects server-chassis as it is added in the classpath.
Solution 1 We keep main method and the writer in the open-metadata-types but rename configuration file ( ex: logback-om.xml ) and loaded on runtime only for this main, and is going to be ignored by server-chassis. Also allowing override it in command line.
Solution 2 Remove the main method and logback.xml from open-metadata-types module and create a dedicated module, for example egeria-cli where we add a main for write the archive as json and maybe other utility main functions.
I'll go for the sake of time with solution 1 and if we decide that solution 2 is better, I will refactor it. Both are easy implementations.
As a good practice we shouldn't add logback.xml files in modules. Especially if we want the modules to be hosted in other applications. If the host application is using logback for logging, our jar could mess up host logging as happened with server-chassis which in this case is the host for open-metadata-types.
logback.xml suppose to be where the java process starts. That was the problem. But we have lot more logback.xml files spread over. They should be removed if we want to be good citizens.
In #3392 the observation of an overriding logback.xml was made and the proposal there was to do the split as @bogdan-sava proposes - but it wasn't actioned at that point.
I provided an explanation there .. and the proposal was item 2 - However I don't see any big issue with going with 1 as an interim measure as it will prevent the log xml 'contamination' - though in many ways it is more complex (code). The root cause was having a module with dual roles - both that of a library & an executable, since as mentioned above, we should only include logging implementation when building 'executables' and not libraries (which imo is an anti pattern)
That being said, the logback configuration (now renamed) in the open metadata types module is setting log level to OFF - so would not explain the 'INFO' messages we see in the original posted problem
As such either a) Some additional local configuration (like the env var LOGGING_LEVEL_ROOT) is set to include info b) The configuration is being picked up from another logback.xml (or another slf4j logging implementation)
So whilst the fix does help prevent future errors, a) we should go further as split role can cause other issues and more crucially b) I don't see how it fixes the original issue (but I may have missed something)
But we have lot more logback.xml files spread over. They should be removed if we want to be good citizens. They should only be included in application modules (ie where we have a main).
Currently I see:
jonesn:egeria/ (master) $ find . -name 'logback.xml' | grep src [9:13:43]
./open-metadata-resources/open-metadata-archives/open-connector-archives/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-archives/open-metadata-types/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-archives/design-model-archives/glossary-canonical-model/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-archives/design-model-archives/cloud-information-model/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-samples/access-services-samples/subject-area-client-samples/subject-area-definition-sample/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-samples/access-services-samples/asset-management-samples/asset-reader-avro-sample/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-samples/access-services-samples/asset-management-samples/asset-create-avro-sample/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-samples/access-services-samples/asset-management-samples/asset-create-csv-sample/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-samples/access-services-samples/asset-management-samples/asset-reader-csv-sample/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-samples/access-services-samples/governance-program-client-samples/governance-leadership-sample/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-samples/access-services-samples/governance-program-client-samples/governance-zone-create-sample/src/main/resources/logback.xml
./open-metadata-resources/open-metadata-samples/admin-services-samples/admin-services-config-metadata-server-sample/src/main/resources/logback.xml
./open-metadata-conformance-suite/open-metadata-conformance-suite-client/src/main/resources/logback.xml
./open-metadata-implementation/server-chassis/server-chassis-spring/src/main/resources/logback.xml
which I think are just either utilities (ie CIM, glossary models), samples (with a main), our server chassis, CTS client - apart from the module referred to above
The actual cause of the INFO files is the change in server-chassis-spring. Under src/main/resources we now have a logback.xml which includes:
<configuration>
<include resource="org/springframework/boot/logging/logback/base.xml"/>
</configuration>
this uses the default config as at https://github.com/spring-projects/spring-boot/blob/master/spring-boot-project/spring-boot/src/main/resources/org/springframework/boot/logging/logback/base.xml which we note has:
<root level="INFO">
<appender-ref ref="CONSOLE" />
<appender-ref ref="FILE" />
</root>
So THAT is the cause of the change - NOT the component mentioned above (though there was POTENTIAL for mess ups there, and the change to server chassis was prompted by that underlaying problem)
This was introduced in https://github.com/odpi/egeria/commit/3e71bc9e898ee4713f0498d9b64abd0cfb35cdb6.
Sticking with default format seems appropriate, but it had the inadvertant effect of resetting the logging levels cc: @lpalashevski
The change was discussed and captured under #4162 point (1), adding logback.xml to server-chassis-spring was the first step to revert to default spring logging on application level and compensate for the fact that some of the modules/libaries override this cofiguration.
For long term solution we also agreed that we want to fix this across the other modules (one example is open-metadata-types) and follow the agreed best practice not to have logging configuration embedded in libraries but leave this as reposnibility of the application/container/environment (sprit roles like Nigel called it)
I aggree with both solutions suggested by Bogdan but for now since I do not know the impact and how much work will be to achieve Soulution 2 as interim I would go with Solution 1.
Adding a logback.xml in the spring-boot app was an workaround because logging was messed up by the lognback.xml in the module. As a matter of fact, I can remove it as the part of PR.
So:
As part of the solution in PR:
Done! I removed logback.xml from server-chassis and works just fine, because default are not override by module.
And again: it happened with server-chassis but it will happen with all host applications that are using logback for logging and include a module with another one in it.
By default the developer logging (SLF4J) is turned off so that debug logging dows not swamp messages to end user. We allowed the 2 log messages from the web server to document the port being used but nothing else.
However, just recently I see a lot of superflous log entries coming out ... and the port entries seem to have gone. Has something changed? How do we put it back to where it was.