Micrometer: StringToDurationConverter conflicts with our own Converter

[robinst]

In our app, we have our own Converter<String, Duration> so that we can use nice readable values such as "500ms" or "10s" in our configuration (highly recommended btw!).

The StringToDurationConverter that is included in micrometer implements a different conversion (just Duration.parse). It seems to have higher priority than our own converter, so it wins but is then unable to convert our inputs such as "500ms", which causes our app to fail to start.

I'm not that familiar with spring and all the spring-boot magic that is going on, but is there a way to scope converters so that they don't conflict? Or any other ideas?

(This is currently blocking my progress for integrating micrometer.)

[robinst]

If we could somehow change the order in which the converters are registered so that our own converter comes first, we could probably add support for the format that Duration.parse expects. So that would be an acceptable solution.

[jkschneider]

I'll bring this up with the Boot team and see what we can do.

[jkschneider]

@wilkinsona Any thoughts on how to scope or prioritize Converters?

[wilkinsona]

It's a bit tricky. It appears that DefaultConversionService will use the last converter that's added that supports a particular conversion. That makes it a little tricky as the default ordering for a bean is lowest precedence so you can't order something explicitly and guarantee that it will go after something that's unordered.

@robinst You could work around the problem by using a BeanPostProcessor to replace the StringToDurationConverter with your own implementation.

@jkschneider A better longer-term solution could be to order our StringToDurationConverter with 0, allowing a user-provided converter to have lower precedence. We'd need to double-check that it works as I believe it will. The conversion service certainly seems to behave as I've described.

[jkschneider]

Thanks for the suggestions @wilkinsona. I think we'll do two things:

1. Add @Ordered to our StringToDurationConverter, assuming this works as expected.
2. Extend our converter to also parse more readable forms like "500ms", because it's just nicer. If you've got an exhaustive listing of what your converter supports or could just share the code with us, that would be helpful @robinst.

[robinst]

Yeah, I can arrange to share the code.

I'm wondering if micrometer is the correct place for providing such a generic converter though. Imagine if it provided a Converter<String, BigDecimal>. Is there a chance of putting it in spring/spring-boot itself somewhere? That would be the ideal outcome of this, and I think it would be a really nice thing to have for all users of spring anyway.

[wilkinsona]

This was ringing a bell and I finally remembered why. The TL;DR is that this works (limited to Duration.parse(String)) without an additional converter:

package com.example.demo;

import java.time.Duration;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;

@SpringBootApplication
@EnableConfigurationProperties(Jsr310Properties.class)
public class DurationConversionApplication {

    public static void main(String[] args) {
        System.out.println(
            SpringApplication.run(DurationConversionApplication.class, "--jsr310.duration=PT10H")
                .getBean(Jsr310Properties.class).getDuration().getSeconds());
    }
}

@ConfigurationProperties(prefix="jsr310")
class Jsr310Properties {

    private Duration duration;

    public Duration getDuration() {
        return duration;
    }

    public void setDuration(Duration duration) {
        this.duration = duration;
    }

}

So I think that StringToDurationConverter can be dropped and that what's being discussed here is a possible enhancement to Spring Framework's existing DurationFormatter.

[jkschneider]

@wilkinsona So if I understand, this is OK in Boot 2 but not in 1.5.x right?

[wilkinsona]

Right. Sorry, I should have been clearer above.

[robinst]

@jkschneider looks like StringToDurationConverter can now parse things like "10s" as well?: https://github.com/micrometer-metrics/micrometer/blob/d2beebf8ba7203638c7310a013acc2fa834a953c/micrometer-spring-legacy/src/main/java/io/micrometer/spring/autoconfigure/export/StringToDurationConverter.java#L52-L57

[jkschneider]

Indeed it can. Leaving this open for now because we haven't yet addressed Boot 2. Also, we should still make this converter @Ordered so yours can take precedence.

[philwebb]

https://github.com/spring-projects/spring-boot/issues/11078 has provided first class support for Duration parsing for all binders. We've also reworked many of our properties classes to use Duration rather than int.

[jkschneider]

StringToDurationConverter has @Order(0) in 1.0.0-rc.5, so you can override it with your own in our legacy support for Boot 1.5.x.