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

Logback <if> blocks cannot read spring properties #33028

Closed
matthenry87 opened this issue Nov 6, 2022 · 3 comments
Closed

Logback <if> blocks cannot read spring properties #33028

matthenry87 opened this issue Nov 6, 2022 · 3 comments
Assignees
Labels
type: regression A regression from a previous release
Milestone

Comments

@matthenry87
Copy link

I built and maintain a logging starter for my company, and am in the process of updating it for Spring Boot 3.0. I've found that Janino fails to leverage Spring properties while evaluating conditions. Here is my demo project that reproduces the issue: https://github.com/matthenry87/logback-janino-bug

While debugging, I can see the property in the logging context's property map:
image
(breakpoint at line 191 of LogbackLoggingSystem.java)

But you can see in the statuses variable that the condition is evaluating to false when it should be true:
image

The logging configuration itself can use the property for element values just fine, it just seems that Janino isn't properly receiving them.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 6, 2022
@philwebb
Copy link
Member

philwebb commented Nov 7, 2022

This looks like a filter ordering problem. Our SpringPropertyModelHandler is being added behind a DenyAll filter so it doesn't run early enough.

Changing the order addModelHandlerAssociations in SpringBootJoranConfigurator to the following appears to fix things:

	@Override
	protected void addModelHandlerAssociations(DefaultProcessor defaultProcessor) {
		defaultProcessor.addHandler(SpringPropertyModel.class,
				(handlerContext, handlerMic) -> new SpringPropertyModelHandler(this.context,
						this.initializationContext.getEnvironment()));
		defaultProcessor.addHandler(SpringProfileModel.class,
				(handlerContext, handlerMic) -> new SpringProfileModelHandler(this.context,
						this.initializationContext.getEnvironment()));
		super.addModelHandlerAssociations(defaultProcessor);
	}

I'd like to add some tests before making that change.

@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 7, 2022
@philwebb philwebb added this to the 2.6.x milestone Nov 7, 2022
@philwebb
Copy link
Member

philwebb commented Nov 7, 2022

@matthenry87 have you found the issue only occurs with Spring Boot 3? Are early versions of Spring Boot OK?

@matthenry87
Copy link
Author

@matthenry87 have you found the issue only occurs with Spring Boot 3? Are early versions of Spring Boot OK?

My Janino conditionals in logback-spring.xml work fine prior to migrating to 3 (we're on 2.7.x, and the starter has existed for a couple years now).

@wilkinsona wilkinsona modified the milestones: 2.6.x, 3.0.x Nov 7, 2022
@wilkinsona wilkinsona added type: regression A regression from a previous release and removed type: bug A general bug labels Nov 7, 2022
@philwebb philwebb changed the title Spring Boot 3.0 + Logback + Janino: Janino Fails to Leverage Spring Properties in Conditions Logback <if> blocks cannot read spring properties Nov 7, 2022
@philwebb philwebb modified the milestones: 3.0.x, 3.0.0-RC2 Nov 7, 2022
@philwebb philwebb self-assigned this Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: regression A regression from a previous release
Projects
None yet
Development

No branches or pull requests

4 participants