Skip to content

WsConfigurer can trigger early initialization that prevents other post processors to run successfully #1435

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

Closed
N4zroth opened this issue Oct 15, 2024 · 11 comments · Fixed by #1443
Assignees
Labels
type: bug A general bug
Milestone

Comments

@N4zroth
Copy link

N4zroth commented Oct 15, 2024

Hi,
as said in the title if I have a Configuration that extends WsConfigurerAdapter with a direct or transitive dependency to ObservationRegistry, ObservationRegistryPostProcessor.postProcessAfterInitialization is never called with ObservationRegistry as argument, thus bricking tracing for the whole project. I'm not sure what's the cause, my best guess is that some initialization order gets messed up.
You can see a small sample project here https://github.com/N4zroth/ws-tracing , both tests should work but only one is green. This seems similar to spring-projects/spring-security#15658 but isn't fixed even with Spring Boot 3.3.4 and spring-ws 6.1.13.

@corneil corneil self-assigned this Nov 4, 2024
@corneil corneil added status: waiting-for-triage An issue we've not yet triaged status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 4, 2024
@N4zroth
Copy link
Author

N4zroth commented Nov 4, 2024

@corneil just updated the project with newest spring boot dependency and it still fails.

@corneil
Copy link
Contributor

corneil commented Nov 5, 2024

@N4zroth Thanks for the reproducer. I am looking into the root cause.

@corneil corneil added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 5, 2024
@corneil
Copy link
Contributor

corneil commented Nov 6, 2024

@N4zroth Can you provide more information as to what you would like to do with the ObservationRegistry within the WsConfigurerAdapter?

@N4zroth
Copy link
Author

N4zroth commented Nov 6, 2024 via email

@corneil
Copy link
Contributor

corneil commented Nov 7, 2024

If you move the bean configuration of feign client to a different configuration class you should not encounter this problem.

@N4zroth
Copy link
Author

N4zroth commented Nov 7, 2024

The configuration of the Feign Bean is already a different Configuration class. This class only has a dependency on the generated Feign Bean.

@N4zroth
Copy link
Author

N4zroth commented Nov 7, 2024

I updated the example with an example including Feign

@corneil
Copy link
Contributor

corneil commented Nov 7, 2024

By injecting the FeignClient into the WsConfig you have the same condition. Forcing the ObservationRegistry to be created before the Web Server is causing the problem.

You can make a change so that WsConfig implements ApplicationListener<ContextRefreshedEvent> and add a method like:

@Override
    public void onApplicationEvent(ContextRefreshedEvent event) {
        configureFeignClient(event.getApplicationContext().getBean(FeignClientExample.class));
    }

Assuming that configureFeignClient will do what is required using the provided parameter.

@N4zroth
Copy link
Author

N4zroth commented Nov 7, 2024

But why is this the case? Shouldn't it be possible to do it 'the normal way'? It looks similar to spring-projects/spring-security#15658 which was fixed some time ago.

@corneil
Copy link
Contributor

corneil commented Nov 8, 2024

If ObservationRegistry is created before the web container we have a problem.
Even if you use an ObjectProvider to delay creation, when you use the object in WsConfig before the Web container is initialized then it doesn't properly initialize the ObservationRegistry.

The best is to hook it in after the context is refreshed. Then you can also deal with dynamic changes in configuration like when using Spring Cloud Config.

@hpoettker
Copy link

Isn't this bug a consequence of #1425 and #1391?

The WsConfigurerAdapter and its sub-classes are not eligible to be postprocessed by all bean post-processors like the ObservationRegistryPostProcessor.

Resolving #1425 should also fix this issue, if I understand correctly.

corneil pushed a commit to corneil/spring-ws that referenced this issue Dec 17, 2024
Change endpoint mapping from BeanPostProcessor to SmartInitializingSingleton
Added Test and updated test.sh to invoke with separate profile.
Fixes spring-projects#1435
corneil pushed a commit to corneil/spring-ws that referenced this issue Dec 17, 2024
Change endpoint mapping from BeanPostProcessor to SmartInitializingSingleton
Added Test and updated test.sh to invoke with separate profile.
Fixes spring-projects#1435
corneil pushed a commit that referenced this issue Dec 18, 2024
Change endpoint mapping from BeanPostProcessor to SmartInitializingSingleton
Added Test and updated test.sh to invoke with separate profile.
Fixes #1435
@snicoll snicoll added in: core and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 18, 2025
@snicoll snicoll added this to the 4.0.12 milestone Feb 18, 2025
@snicoll snicoll changed the title Presence of WsConfigurerAdapter with a dependency to ObservationRegistry prevents ObservationRegistry's post processing and bricks tracing WsConfigurer can trigger early initialization that prevents other post processors to run successfully Feb 18, 2025
snicoll added a commit that referenced this issue Feb 18, 2025
This commit reverts the test that has been introduced to validate the
change in fe43f88. This test requires Spring Boot to operate and we
cannot rely on that.

See gh-1435
snicoll added a commit that referenced this issue Feb 18, 2025
This commit reverts the test that has been introduced to validate the
change in fe43f88. This test requires Spring Boot to operate and we
cannot rely on that.

See gh-1435
@snicoll snicoll marked this as a duplicate of #1391 Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants