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

Logging of percent character #457

Open
c-classen opened this issue Aug 11, 2023 · 6 comments
Open

Logging of percent character #457

c-classen opened this issue Aug 11, 2023 · 6 comments
Assignees

Comments

@c-classen
Copy link

The % character seems to be handled differently by the default Quarkus standard output logging and this library. This line:

logger.info("Progress: 10%")

leads to the following output:

2023-08-11 13:19:21,924 INFO  [REDACTED] (Quarkus Main Thread) Progress: 10%
java.util.logging.ErrorManager: 1: Nested handler publication threw an exception
java.util.UnknownFormatConversionException: Conversion = '%'
	at java.base/java.util.Formatter.parse(Formatter.java:2826)
	at java.base/java.util.Formatter.format(Formatter.java:2763)
	at java.base/java.util.Formatter.format(Formatter.java:2717)
	at java.base/java.lang.String.format(String.java:4155)
	at io.quarkiverse.logging.cloudwatch.LoggingCloudWatchHandler.publish(LoggingCloudWatchHandler.java:82)
	at org.jboss.logmanager.ExtHandler.publishToNestedHandlers(ExtHandler.java:97)
	at io.quarkus.bootstrap.logging.QuarkusDelayedHandler.doPublish(QuarkusDelayedHandler.java:81)
	at org.jboss.logmanager.ExtHandler.publish(ExtHandler.java:66)
	at org.jboss.logmanager.LoggerNode.publish(LoggerNode.java:327)
	at org.jboss.logmanager.LoggerNode.publish(LoggerNode.java:334)
	at org.jboss.logmanager.LoggerNode.publish(LoggerNode.java:334)
	at org.jboss.logmanager.LoggerNode.publish(LoggerNode.java:334)
	at org.jboss.logmanager.LoggerNode.publish(LoggerNode.java:334)
	at org.jboss.logmanager.LoggerNode.publish(LoggerNode.java:334)
	at org.jboss.logmanager.Logger.logRaw(Logger.java:750)
	at org.slf4j.impl.Slf4jLogger.log(Slf4jLogger.java:574)
	at org.slf4j.impl.Slf4jLogger.info(Slf4jLogger.java:288)
	[REDACTED]
	at io.quarkus.runtime.ApplicationLifecycleManager.run(ApplicationLifecycleManager.java:132)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:71)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:44)
	at io.quarkus.runner.GeneratedMain.main(Unknown Source)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
	at io.quarkus.runner.bootstrap.StartupActionImpl$1.run(StartupActionImpl.java:104)

While the line is logged as requested by Quarkus, it is not sent to Cloudwatch. Instead, an exception is printed to standard out.

Considering Log4Shell, I'd probably prefer if the string I pass to the logging functions was printed without any processing. However, if some sort of conversion is enabled, it would be nice if it was documented and the error message and stack trace should also be sent to Cloudwatch, so people relying on that log know that there was some logging that failed.

@bennetelli bennetelli self-assigned this Sep 13, 2023
@bennetelli
Copy link
Contributor

bennetelli commented Oct 5, 2023

Hey @c-classen I just released 6.6.0. It should be fixed with this version. Please let me know if it works for you. It's a bit shitty to test this by an integration test. Let me know if this works for you.

@vkrizan
Copy link

vkrizan commented Oct 19, 2023

@bennetelli which commits were fixing the issue? We need to backport them to version 5 to support Quarkus 2. Thanks.

@vkrizan
Copy link

vkrizan commented Oct 23, 2023

@gwenneg @josejulio can you please help with this? Thank you!

@bennetelli
Copy link
Contributor

bennetelli commented Oct 23, 2023

@vkrizan sorry, I was in vacation for some weeks.

It's part of the commit with revision number 02b4305 and two commits later (revision c5a8ea9) I also fixed some formatting issues coming from quarkiverse restrictions. So I guess you just want to pick the one containing the fix only? 02b4305

@vkrizan
Copy link

vkrizan commented Oct 23, 2023

@bennetelli if those later commits are related and required, can you please also include them as well? Thank you!

PS: I'd recommend to branch out the version 5 and open PR with the cherry-picks against it.

@vkrizan
Copy link

vkrizan commented Nov 23, 2023

@bennetelli @gwenneg can you please help with it? Thank you!

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

No branches or pull requests

3 participants