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

IllegalArgumentException when message includes {} #401

Open
Zack-Freedman-Thoughtworks opened this issue Feb 24, 2024 · 5 comments
Open

Comments

@Zack-Freedman-Thoughtworks

In the following example, the first error leads to the failure:

WARN StatusConsoleListener io.github.oshai.kotlinlogging.slf4j.internal.LocationAwareKLogger caught java.lang.IllegalArgumentException logging ReusableParameterizedMessage: Hey this fails {}
 java.lang.IllegalArgumentException: found 1 argument placeholders, but provided 0 for pattern `Hey this fails {}`
	at org.apache.logging.log4j.message.ParameterFormatter.formatMessage(ParameterFormatter.java:238)
logger.atInfo {
	message = "Hey this fails {}"
	payload = mapOf(
		"first" to "second"
	)
}

logger.atInfo {
	message = "Hey this doesn't fail {}"
}

Configuration:

	implementation("org.springframework.boot:spring-boot-starter-log4j2")
	implementation("io.github.oshai:kotlin-logging-jvm:6.0.3")
	implementation("org.springframework.boot:spring-boot-starter-actuator")

It doesn't seem like the issue happens with Spring Boots default logger spring-boot-starter-logging (vs log4j2).

I am aware this is an error arising from log4j but I'm wondering if there is something we can do for these methods to always avoid parameterized messaging. As far as I understand, we couldn't use atInfo to parameterize messages anyways.

Example can be found here. Let me know if there's a better way to provide examples.

@Zack-Freedman-Thoughtworks
Copy link
Author

When I first noticed this issue, the error presented as a JsonMappingException which is likely related to using JsonLayout

@oshai
Copy link
Owner

oshai commented Feb 25, 2024

Yes, this is really something related to the specific log4j impl. I don't have a good idea how to fix that (looking for {} placeholder in strings feels an overkill, as anyway we will just throw an exception).

@Zack-Freedman-Thoughtworks
Copy link
Author

Is there any workaround for this or should we stay away from the version upgrade? I'd like to avoid breaking some logs for an upgrade.

@oshai
Copy link
Owner

oshai commented Apr 14, 2024

I am not sure what you're trying to upgrade: from which version to which and how does the log call changes. I suspect using a payload is what causing this, so I expect that change to be breaking anyway, and need to be tested.

Can you please provide so more details?

@Zack-Freedman-Thoughtworks
Copy link
Author

Hi, as you guessed, we were experimenting with a major version increment where we weren't using payloads in the past. I suppose we can still upgrade as long as we don't allow payloads, but that seems hard to enforce.

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

2 participants