rohityadavcloud commented on PR #7131: URL: https://github.com/apache/cloudstack/pull/7131#issuecomment-1488671229
@JoaoJandre thanks for the reply. We use dependencies from other opensource communities and organisations all the time - any organisation behind a dependency shouldn't be the only criteria for choosing it. [Reload4j](https://reload4j.qos.ch/) is backed by supporters and used by at least half-a-dozen other Apache projects and appears to be maintained and released (I see a new release last week, that reminds me we need to upgrade the reload4j dependency for 4.18.1.0 milestone). Thinking about our community and users, if we're undertaking such a big task - I would have explored an agnostic approach like a logging framework where the underlying library is easily swappable (sl4j comes to my mind, or even our own logging manager). I appreciate all the hard work you've put in, however, I wasn't aware of this until Daan and Wei discussed this PR on the dev@ ML. I don't have any objections if the community wants to go this route but I suggest we consider the following concerns and suggestions: - We need to validate packaging and smoketests across all hypervisor (and permutations on zone types, storage etc), was there any done? Does changing logging library has side-effects for existing codebase? - I think we would also need to do upgrade tests for all supported hypervisors. Have we done any testing on this, could you share all such details of testing you may have done? - I see some 120+ PRs, would merging this cause conflicts on them? Is there a way to minimise conflicts and side-effects on rest of the community PRs? - Do we need an advisory for users, on how this would affect their current logging config, affect their plugins and any other integrations? (a lot of plugins aren't in the upstream repo but managed/shipped by users in the community) - If we're considering this, should we also look at upgrading related dependencies such as gson? - Speculating here - if only the parent classes has logger, has anybody tested if logging by a child-class prints the right class name in the logs (for example, [c.c.c.ClusterManagerImpl] in the log file)? Could you also discuss your initiative on the dev@ and users@ mailing lists, it's possible my list above isn't exhaustive. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
