spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.21k stars 40.69k forks source link

Reactive observation auto-configuration does not declare order for WebFilter #33444

Closed codependent closed 1 year ago

codependent commented 1 year ago

Affects: 6.0.2

When using micrometer-tracing to correlate requests in Webflux applications we are able to access the underlying Observation withing the Reactor context as seen in this spring-boot issue: https://github.com/spring-projects/spring-boot/issues/33372

However, currently there's no way to access that information within a WebFilter, even though we set it up with the lowest precedence to trigger ServerHttpObservationFilter before it:

@Component
@Order(Ordered.LOWEST_PRECEDENCE)
class RequestLoggerPropagationFilter: WebFilter {

    private val logger = LoggerFactory.getLogger(javaClass)
    override fun filter(exchange: ServerWebExchange, chain: WebFilterChain): Mono<Void> {
        return Mono.just(Unit)
            .handle { t: Unit, u: SynchronousSink<Any> ->
                logger.info("beginning request")
                u.next(t)
            }.flatMap {
                chain.filter(exchange)
            }
    }
}

ServerHttpObservationFilter, which doesn't have any @Order, is always invoked last, so there's no way the traceId/spanId can be accessed within our filters.

Would you consider adding an @ Order(X) to ServerHttpObservationFilter. This way we will be able to place our application WebFilters after it to propertly access correlation/observation info.

bclozel commented 1 year ago

I don't think we should hard code the order on the filter class itself, but rather configure this in Spring Boot at the bean method level. This is done for the MVC counterpart but was never done for the metrics webflux variant in the past.

We should update the WebFluxObservationAutoConfiguration and also ensure that the order is properly documented. As for the ordering value itself, it should probably be aligned with the MVC variant to be set ahead of the security filter.