Allow override of the order of ServerHttpObservationFilter in WebFluxObservationAutoConfiguration

[davidmelia]

Hi,

Migrating from Spring Cloud Sleuth to Spring Boot 3 there used to be a property to override the TraceFilter:

spring:
  sleuth:
    web:
      filter-order: -111

which is quite important for us as we have some filters which we need to run before any observation. I have noticed in Spring Boot 3 the order is hardcoded:

public class WebFluxObservationAutoConfiguration {

...

	@Bean
	@ConditionalOnMissingBean
	@Order(Ordered.HIGHEST_PRECEDENCE + 1)
	public ServerHttpObservationFilter webfluxObservationFilter(ObservationRegistry registry,

Is it possible to override ServerHttpObservationFilter order (specifically the reactive version)?

Thanks

[wilkinsona]

we have some filters which we need to run before any observation

Any filters with highest precedence should run before ServerHttpObservationFilter.

Is it possible to override ServerHttpObservationFilter order

There's no property-based support for this at the moment. Without that support you'd have to define your own ServerHttpObservationFilter bean with an appropriate @Order annotation on the @Bean method.

[davidmelia]

Issue I have is ServerHttpObservationFilter is ordered as Ordered.HIGHEST_PRECEDENCE + 1 and I have three other filters. This gives me the only option to use Ordered.HIGHEST_PRECEDENCE which will randomly be ordered. :-(

I can define my own ServerHttpObservationFilter no problem but a property would be cleaner.

Thanks

[mhalbritter]

The property is named management.observations.http.server.filter.order.

[wilkinsona]

We need to consider the servlet side of things as well. The property should either be applied to the auto-configured webMvcObservationFilter or its name should indicate that it's specific to reactive. Probably the former.

[mhalbritter]

Thanks Andy, i missed that.

[wilkinsona]

I've just learned about https://github.com/spring-projects/spring-framework/commit/96a429a5613c1fee2b011e42d4a401ab3506a588. We need to review these changes in the context of that deprecation and what it means for the requirement raised here.

/cc @bclozel

[mhalbritter]

We need to set the ObservationRegistry on the WebHttpHandlerBuilder, as described here. We can remove ServerHttpObservationFilter. For servlet applications, nothing changes.

[wilkinsona]

We need to set the ObservationRegistry on the WebHttpHandlerBuilder, as described here

How would that work for the original requirement where "we have some filters which we need to run before any observation"?

[bclozel]

Sorry for the late feedback.

The Servlet Filter abstraction works well in general for Spring MVC, although some people find that limited and prefer a custom Tomcat Valve for observations in order to observe the entire chain, servlet container error processing included. To be accurate, the observation filter should be as early as possible in order to measure most of the processing time. I don't think we have similar properties for other filters in Spring Boot and the general feedback so far was to configure your own filter instance. I'm not against this change, but I guess this would mean that we implement the same thing for other filters.

On the WebFlux side, the WebFilter approach was too limited according to our community. In WebFlux we don't rely on Servlet contracts and there are dedicated SPIs for this. The DispatcherHandler handles HTTP exchanges and delegates to WebFilter and WebExceptionHandler. This means that error handling in WebExceptionHandler happens outside of the scope of WebFilter observations and error logs did not contain the trace context as expected. We fixed that in https://github.com/spring-projects/spring-framework/issues/30013 with a change of infrastructure. Given that WebFlux manages the HTTP contracts, this was the only way to provide the feature to our users.

With the new infrastructure, the observation starts before any webfilter is involved. @davidmelia can you share more about the use case? Maybe there is something we can do to help with that without relying on extra filters?

With that in mind, if we want to provide configuration properties for servlet filters, we probably need a servlet and a reactive namespace to separate concerns are they don't align 100%.

[mhalbritter]

How would that work for the original requirement where "we have some filters which we need to run before any observation"?

I'm afraid it doesn't, like Brian said. It only gets rid of the deprecation.

The changes to get rid of the deprecation are in this branch. However, there are 3 tests in WebFluxObservationAutoConfigurationTests failing. I think this is because the WebClient does something funky or we missed setting up important parts of the WebFlux infrastructure in the test, because it's working in a standalone application.

[wilkinsona]

I think we should revert the changes that have been made thus far for this issue. The requirement hasn't been raised for servlet apps and, until we hear from @davidmelia, we don't know what, if anything, we might need to do for reactive apps.

[davidmelia]

Hi @bclozel @wilkinsona - in terms of my use case we have a few custom filters which need to happen to set up the observation context - this includes extracting the IP address from a custom header and extracting other sources of data. All of this data is integral to our logging context information (N.B I have a custom logging.pattern.level)

Therefore, while I understand the assumption that ServerHttpObservationFilter should go first (well Ordered.HIGHEST_PRECEDENCE + 1) I have a few pre filters that need to run to set up the observation filter.

Thanks

N.B My only use case is reactive.

[mhalbritter]

I think we should revert the changes that have been made thus far for this issue. The requirement hasn't been raised for servlet apps

I reverted the commits in the meantime.

[bclozel]

@davidmelia I'm not really sure I understand the use case here.

If you are trying to have additional metrics/traces tags contributed to the observation, you don't need to set things up before the observation starts. Writing your own ObservationConvention implementation is enough to augment the observations. You can lookup request headers in the observation context.

If your goal is to customize the context propagation, I would need more information.

Maybe you are trying to get ahead of the web observation to pre-populate the MDC with custom values? In this case, I believe you can achieve this by contributing a HttpHandlerDecoratorFactory bean that would do just that. Those are ordered as well and are all processed before the observation starts.

[davidmelia]

@bclozel I have tried HttpHandlerDecoratorFactory and it works great :-) . This actually removes my need to override this filter order.

[wilkinsona]

That's excellent news. Thanks for letting us know, @davidmelia.

I.m going to close this one as we no longer have a need to make the filter order configurable, for either web stack.

[edigu]

@bclozel I have tried HttpHandlerDecoratorFactory and it works great :-) . This actually removes my need to override this filter order.

@davidmelia it would be great if you could share your solution involving HttpHandlerDecoratorFactory to shed some light for others.

I have a similar use case: extracting a special header value from the the request (using a very high order once per request filter - request has to be rejected in case of an invalid value) where the value is vital for the rest of the lifecycle. I have been setting the value in MDC there to make it available in logs, and also wanted to make it globally available for all metrics.

I read the thread and learned that very high precedence is intentional here.

what would you recommend for this use case? How HttpHandlerDecoratorFactory helped?

[bclozel]

@edigu your use case might be a bit different actually.

1. Is the application reactive or Servlet based?
2. Which Spring Boot version are you using?
3. If the request is rejected because of an invalid value, should it still record an observation (metrics/tracing) for this?

Maybe you can share those details in a StackOverflow question and point us to it? A closed issue isn't the right place to get help on a different matter.

[edigu]

@bclozel

1. It is Servlet based
2. Using boot 3.1.x
3. No I track invalid values in a different way. Observation is not a must.

Yes you are right, will try SO in worst case. Wanted to get feedback since the use case is quite similar.

[davidmelia]

@edigu my use-case is to set up the log context which i used to do using web filters without realising I could simply use HttpHandlerDecoratorFactory. Below is an example. I can have multiple factories for the different log context concerns

public class MyHttpHandlerDecoratorFactory implements HttpHandlerDecoratorFactory {

  @Override
  public HttpHandler apply(HttpHandler httpHandler) {
    return (request, response) -> {
      log.debug("enter filter()");
      // extract stuff and decide how you want to add this to the log context
      log.debug("end filter()");
      return httpHandler.handle(mutatedRequest, response);
    };
  }

}

@AutoConfiguration
public class HttpHandlerDecorationAuthConfiguration {

  @Bean
  MyHttpHandlerDecoratorFactory myHttpHandlerDecoratorFactory() {
    MyHttpHandlerDecoratorFactory httpHandlerDecorator = new MyHttpHandlerDecoratorFactory();
    return httpHandlerDecorator;
  }
}