Copilot commented on code in PR #12967:
URL: https://github.com/apache/cloudstack/pull/12967#discussion_r3052178114
##########
agent/conf/log4j-cloud.xml.in:
##########
@@ -39,7 +39,7 @@ under the License.
<Console name="CONSOLE" target="SYSTEM_OUT">
<ThresholdFilter level="OFF" onMatch="ACCEPT" onMismatch="DENY"/>
- <PatternLayout pattern="%-5p [%c{3}] (%t:%x)
(logid:%X{logcontextid}) %m%ex%n"/>
+ <PatternLayout pattern="%d{DEFAULT} %-5p [%c{3}] (%t:%x)
(logid:%X{logcontextid}) %m%ex%n"/>
</Console>
Review Comment:
Same concern here: if `.err` is sourced from stderr as stated in the PR
description, this console appender targeting `SYSTEM_OUT` won’t influence
`.err` formatting. Consider using `target=\"SYSTEM_ERR\"` (or aligning systemd
routing) to ensure `.err` lines include the new timestamp.
##########
client/conf/log4j-cloud.xml.in:
##########
@@ -70,7 +69,7 @@ under the License.
<Console name="CONSOLE" target="SYSTEM_OUT">
<ThresholdFilter level="OFF" onMatch="ACCEPT" onMismatch="DENY"/>
- <PatternLayout pattern="%-5p [%c{1.}] (%t:%x)
(logid:%X{logcontextid}) %m%ex{filters(${filters})}%n"/>
+ <PatternLayout pattern="%d{DEFAULT} %-5p [%c{1.}] (%t:%x)
(logid:%X{logcontextid}) %m%ex{filters(${filters})}%n"/>
</Console>
Review Comment:
Same as server: the description references stderr->`.err`, but the console
appender is configured for `SYSTEM_OUT`. If `.err` is populated from stderr,
the timestamped pattern won’t be reflected there unless the target is
`SYSTEM_ERR` (or the unit routes stdout to `.err`).
##########
server/conf/log4j-cloud.xml.in:
##########
@@ -68,7 +64,7 @@ under the License.
<Console name="CONSOLE" target="SYSTEM_OUT">
<ThresholdFilter level="OFF" onMatch="ACCEPT" onMismatch="DENY"/>
- <PatternLayout pattern="%-5p [%c{1.}] (%t:%x) %m%ex%n"/>
+ <PatternLayout pattern="%d{DEFAULT} %-5p [%c{1.}] (%t:%x) %m%ex%n"/>
</Console>
Review Comment:
The PR description says the .err logs are fed from console/stderr, but this
Console appender targets `SYSTEM_OUT`. If systemd is capturing stderr into
`.err`, changing the stdout pattern won’t affect `.err`. Consider switching the
console target to `SYSTEM_ERR` (or update the systemd unit routing) so the
timestamped console output actually lands in `.err`.
--
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]
