I'm registering a custom BeanPostProcessor instance that adds some filters to WebClient.Builder instances. I need my filters to be added after the tracing ones, which are added by a third party tracing library (opentracing-spring-web). The main problem is that this library doesn't specify any order on the PostProcessor they register to assign the Trace data. This becomes a problem cause PostProcessorRegistrationDelegate sorts all post-processor putting all sorted ones before the unsorted ones

// First, register the BeanPostProcessors that implement PriorityOrdered.
sortPostProcessors(priorityOrderedPostProcessors, beanFactory);
registerBeanPostProcessors(beanFactory, priorityOrderedPostProcessors);

// Next, register the BeanPostProcessors that implement Ordered.
List<BeanPostProcessor> orderedPostProcessors = new ArrayList<>();
for (String ppName : orderedPostProcessorNames) {
    BeanPostProcessor pp = beanFactory.getBean(ppName, BeanPostProcessor.class);
    orderedPostProcessors.add(pp);
    if (pp instanceof MergedBeanDefinitionPostProcessor) {
        internalPostProcessors.add(pp);
    }
}
sortPostProcessors(orderedPostProcessors, beanFactory);
registerBeanPostProcessors(beanFactory, orderedPostProcessors);

// Now, register all regular BeanPostProcessors.
List<BeanPostProcessor> nonOrderedPostProcessors = new ArrayList<>();
for (String ppName : nonOrderedPostProcessorNames) {
    BeanPostProcessor pp = beanFactory.getBean(ppName, BeanPostProcessor.class);
    nonOrderedPostProcessors.add(pp);
    if (pp instanceof MergedBeanDefinitionPostProcessor) {
        internalPostProcessors.add(pp);
    }
}
registerBeanPostProcessors(beanFactory, nonOrderedPostProcessors);

// Finally, re-register all internal BeanPostProcessors.
sortPostProcessors(internalPostProcessors, beanFactory);
registerBeanPostProcessors(beanFactory, internalPostProcessors);

// Re-register post-processor for detecting inner beans as ApplicationListeners,
// moving it to the end of the processor chain (for picking up proxies etc).
beanFactory.addBeanPostProcessor(new ApplicationListenerDetector(applicationContext));

I have three options:

  1. Register mine as sorted: doesn't work cause it gets registered before
  2. Register mine as unsorted: doesn't work for me either cause I don't seem to find a way of predicting the order
  3. Register mine as MergedBeanDefinitionPostProcessor: it works as it gets registered at the end, but it doesn't really make sense as it doesn't implement the extra methods

I can also register it as a WebClientCustomizer but also gets processed before.

In my opinion, the way the sorting is done isn't right because it leads to this kind of situation where we need to operate after something else we have no control over, and we just don't have a proper mechanism of controlling it.

My suggestion would be to assign a default priority to the unsorted postprocessors so they get sorted along with the sorted ones instead of before/after. This way the order would be:

  1. priorityOrderedPostProcessors
  2. orderedPostProcessos and nonOrderedPostProcessors (sorted treating nonOrderedPostProcessors as if they had order 0)
  3. internalPostProcessors (the ones that extend MergedBeanDefinitionPostProcessor)

Of course, this is based on my own experience and what makes sense to me. I know I'm missing many things on the big picture so feel free to let me know if it can cause other issue.

Comment From: rubasace

Any feedback on this issue?

Comment From: rstoyanchev

Making a change along those lines is guaranteed to cause subtle issues for applications that rely no the present behavior, whether intentionally or not. Therefore a change along those lines is pretty much not an option.

Have you considered asking OpenTracing to make it possible to set the order so you can register your filters relative to that?

Or otherwise you have to defer the registration of your filters until a later point in the lifecycle. WebClient.Builder provides a method with a Consumer<List<ExchangeFilterFunction>> that lets you inspect, change, or re-order the list of filters. You can also mutate a WebClient instance which drops you back into the Builder.

Comment From: spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Comment From: rubasace

@rstoyanchev I haven't contacted Opentracing as I preferred to explore the options with what I consider to be the root of the issue.

The solutions you propose to me don't work, cause I'm doing those manipulations from a generic library, so won't be able to mutate a WebClient and at the moment I'm accessing the Builder the other PostProcessors haven't been called yet. Sure I could wait for the Context to be ready and do the manipulations on the Builder there but in that case, I think I prefer to stick to MergedBeanDefinitionPostProcessor.

Regarding the impact, I get it, it's a risky change so my last question would be: is a change to LOWEST_PRECEDENCE - 1 an option? that would still offer an option to add things behind while keeping the impact to the minimum.

Comment From: rstoyanchev

I am puzzled by your view on the root cause. It is recommended in the docs for a BPP to implement Ordered:

You can configure multiple BeanPostProcessor instances, and you can control
the order in which these BeanPostProcessor instances execute by setting the
order property. You can set this property only if the BeanPostProcessor implements
the Ordered interface. If you write your own BeanPostProcessor, you should
consider implementing the Ordered interface, too.

The Javadoc for OrderComparator clearly indicates the rules for ordered vs non-ordered objects.

Even a seemingly small change like defaulting to LOWEST_PRECEDENCE - 1 is a change in the contract, which IMO is unreasonable, and would justifiably be a regression from the perspective of those affected by it and I have no doubt there would be such cases.

I will defer to @jhoeller for further comments.

Comment From: rubasace

@rstoyanchev I understand that the issue comes from the fact that there's a BPP not following the recommendations, not from Spring. I think we are on the same page on that, sorry if I haven't stated it properly.

What I think is that the decision that had been taken for defaulting the OrderComparator creates an issue if we don't have control over third-party BPPs that don't follow the recommendations.

I'm just asking about the LOWEST_PRECEDENCE -1 cause my understanding is that, in case of two Beans with the same order, the final sorting isn't predictable. If that assumption is true, I don't think that change in the default value should affect anything that cannot already misbehave with the current implementation. I could be totally wrong on that, of course.

LOWEST_PRECEDENCE - 1 is a change in the contract, which IMO is unreasonable

That's totally understandable and if that's the final position feel free to close this issue

Comment From: ttddyy

Hi @rubasace

For workaround in your case, you can create a custom BeanPostProcessor that simply inherit/compose the one from open-tracing with ordering in place.

For example, assuming that what you are using is TracingWebClientBeanPostProcessor

public class MyTracingWebClientBeanPostProcessor extends TracingWebClientBeanPostProcessor implements Ordered {
  ...
}

If you are using spring-boot auto-configuration, to exclude the BPP bean coming from auto-configuration, either exclude the WebClientTracingAutoConfiguration itself, or if you really want to disable the TracingWebClientBeanPostProcessor bean registered by auto-configuration, you can remove the bean definition for the TracingWebClientBeanPostProcessor.

e.g.:

public class UnregisterBeanDefinitionBeanFactoryPostProcessor implements BeanFactoryPostProcessor, Ordered {

    @Override
    public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
        BeanDefinitionRegistry registry = (BeanDefinitionRegistry) beanFactory;
        for (String beanName : this.beanNames) {
          if(beanName.equals("tracingWebClientBeanPostProcessor")) {
                registry.removeBeanDefinition(beanName);
            }
        }
    }

    ...
}

This way, even though auto-configuration registers the BPP from open-tracing, it removes from beanFactory and use the one you have specified. (https://github.com/spring-projects/spring-boot/issues/18228)

I have a java library that uses open-tracing and we have a case that we need to replace the one defined in open-tracing library. If it helps for your workaround.

Comment From: rubasace

Thanks @ttddyy , that definitely did the job. I managed to solve both the BeanPostProcessor for WebClient as well as the RestTemplateCustomizer for RestTemplate.

Comment From: rubasace

@rstoyanchev what about adding a configuration property to override the default order? like that, the issue can be solved at risk of whoever touches it, but backwards compatibility will be ensured.

Comment From: khatkarrohit

I am also in a similar situation. I wanted to add a BeanFactoryPostProcessor after all unordered BeanFactoryPostProcessors but for some reason the BeanFactoryPostProcessor from my project source folder gets picked up first in nonOrderedPostProcessors and library ones are added later.

If a natural sort order is added to nonOrderedPostProcessors based on name than I can rename my custom BeanFactoryPostProcessor to start with 'Z' and fix such issues.

Comment From: rstoyanchev

Superseded by #24844.