Skip to content
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

Support for 2.4.0+ / Log4J2 #175

Closed
ibacher opened this issue Feb 2, 2022 · 3 comments · Fixed by #181
Closed

Support for 2.4.0+ / Log4J2 #175

ibacher opened this issue Feb 2, 2022 · 3 comments · Fixed by #181
Assignees

Comments

@ibacher
Copy link
Member

ibacher commented Feb 2, 2022

Currently, Iniz hard-codes setting up a logger in the Activator here.

This will fail on versions of OpenMRS that use Log4J2 (2.4.0+ and now 2.1.5+) because the class org.apache.log4j.FileAppender is no longer on the classpath. (Log4J2 provides a Log4J1 "bridge" that maps many Log4J1 class, but reimplements them on Log4J2's infrastructure).

We should refactor things so it's possible to load Iniz on versions of OpenMRS using Log4J2 while maintaining the current functionality on versions that use Log4J1.

@rbuisson
Copy link
Member

rbuisson commented Feb 3, 2022

@ibacher , would you have a work estimate on this?

@ibacher
Copy link
Member Author

ibacher commented Feb 3, 2022

@rbuisson Simply refactoring things so the logging is only setup where Log4J1 is available is probably quite quick, e.g., 5-10 hrs of work and testing. This would get things so it loaded on 2.4.0, but we wouldn't have an initializer.log file generated.

Refactoring things so that we actually create a similar initializer.log file when using Log4J2 is more complicated and I'd need to figure out what the design should be.

There are a few options:

  1. We leave creating an initializer.log file out of scope for Iniz itself and instead take advantage of the feature in core 2.4.3+ and 2.5.1+ that allows users to be albe define their own logging configuration. That would mean just updating the README with a "suggested configuration".
  2. The hack: we create the logger and appender directly in Core's logging configuration. This is also easy to implement but, well, a hack and maybe hard to justify.
  3. Adding functionality (outlined below) to core to allow OpenMRS modules to define and run Log4J2 configuration plugins. This is, from a technical perspective, the cleanest, but honestly in the range of 20-30 hours to figure out and implement.

The challenge is that Log4J2 allows programmatic reconfiguration but it's tied to Java's SPI interface. SPI will automatically recognise properly registered classes, but it only scans the classpath once and then assumes all classes have been found. Because (currently) modules are only loaded well after the logging system will have been initialised any classes in modules that try to define Log4J2 configuration will simply not be executed. We can work around this the same way we work around similar limitations on servlets, i.e., by defining a proxy configuration class that's aware of OpenMRS's loading mechanism and executes any classes defined in modules. That way, modules will be able to define Log4J2 configuration classes and have them executed correctly. The higher development time for this option is around doing this sanely and efficiently.

Pragmatically, option 1 is probably the best way to go, though I do enjoy the sort of challenges posed by option 3.

@mseaton
Copy link
Collaborator

mseaton commented Feb 3, 2022

@rbuisson and @ibacher - whatever we end up doing here, it would be great if we could ensure it also solves the issues raised in #157

ibacher added a commit to openmrs/openmrs-distro-referenceapplication that referenced this issue Feb 8, 2022
@ibacher ibacher self-assigned this Feb 28, 2022
@ibacher ibacher mentioned this issue Feb 28, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants