TS-40806 replace the artifical delayed logger with a delayed appender #614
Conversation
|
The one failing test should be fixed once merged to master. |
| /** | ||
| * An appender that collects log statements in its buffer until it's asked to empty it into a given | ||
| * {@link LoggerContext}. It can be used to log log-messages before an actual logging appender has been configured using | ||
| * the options passed to the agent. | ||
| * */ |
There was a problem hiding this comment.
We should also document here how logback picks up this appender. I guess this will feel a bit to magical without further logback knowledge.
| <appender name="Delayed" class="shadow.com.teamscale.jacoco.agent.logging.DelayedLogAppender"> | ||
| </appender> | ||
|
|
||
| <root level="INFO"> |
There was a problem hiding this comment.
Does this mean we drop any TRACE/DEBUG messages that are logged while the usual logger is not yet attached? This might be unexpected I guess.
| // StatusPrinter will handle this | ||
| } | ||
| StatusPrinter.printInCaseOfErrorsOrWarnings(loggerContext); | ||
| DelayedLogAppender.logTo(loggerContext); |
There was a problem hiding this comment.
Here we dump the already collected logs into the new context, but do we still continue to collect logs in the DelayedLogAppender afterwards? If so this would be I problem I guess considering that we keep all of those in memory and never discard them anymore.
| TeamscaleCredentials credentials = TeamscalePropertiesUtils.parseCredentials(); | ||
| if (credentials == null) { | ||
| delayedLogger.warn("Did not find a teamscale.properties file!"); | ||
| LOGGER.warn("Did not find a teamscale.properties file!"); |
There was a problem hiding this comment.
When I run the sample-app on master with logging-config=./tmp/teamscale-jacoco-agent/logging/logback.rolling-file.xml the console output is
Picked up JAVA_TOOL_OPTIONS: -javaagent:tmp/teamscale-jacoco-agent/lib/teamscale-jacoco-agent.jar=config-file=./log-test.properties
on this branch it is
Picked up JAVA_TOOL_OPTIONS: -javaagent:tmp/teamscale-jacoco-agent/lib/teamscale-jacoco-agent.jar=config-file=./log-test.properties
16:00:35.529 [main] WARN com.teamscale.jacoco.agent.PreMain -- Did not find a teamscale.properties file!
16:00:35.531 [main] DEBUG com.teamscale.jacoco.agent.options.AgentOptionsParser -- Parsing options: config-file=./log-test.properties
which I would not have expected as the log output is expected to be written to the file system. Not 100% sure though where those logs are written to the console though.
There was a problem hiding this comment.
I think this was caused by the fact that the delayed appender wasn't actually used at all - I think for some reason a console logger was picked up somewhere instead? And I mistook its output for the output of the console appender that was supposed to be used after initializing the correct logger... At least in my current implementation it is not actually used when calling LOGGER.error(...) 🤔 I'm working on it
|
Addresses issue TS-XXXXX
Please respect the vote of the Teamscale bot or flag irrelevant findings as tolerated or false positives. If you feel that the Teamscale config needs adjustment, please state so in a comment and discuss this with your reviewer.