ApplicationDiscoveryListener::convert method creates wrong health/management/home URLs

[jvmlet]

I'm developing spring boot service that runs both embedded Tomcat on port 8081 and embedded gRPC server on port 6565 (using grpc-spring-boot-starter).
Tomcat hosts actuator endpoints and gRPC server hosts gRPC services (over HTTP/2 protocol).
My config looks like this :

server:
  port: 8081
grpc:
  port: 6565
eureka:
  instance:
    nonSecurePort: ${grpc.port}
    home-page-url: http://${spring.cloud.client.hostname}:${server.port:8081}
    health-check-url: ${eureka.instance.home-page-url}/health
    status-page-url: ${eureka.instance.home-page-url}/info

The problem is that de.codecentric.boot.admin.discovery.ApplicationDiscoveryListener creates de.codecentric.boot.admin.model.Application with health URL pointing to http://serviceHost:**6565**/health instead of http://serviceHost:**8081**/health
From ApplicationDiscoveryListener::convertmethod :

String serviceUrl = append(instance.getUri().toString(), serviceContextPath);
String managementUrl = append(instance.getUri().toString(), managementContextPath);
String healthUrl = append(managementUrl, healthEndpoint);

I've overcame this problem with my custom EurekaApplicationDiscoveryListener (thanks to DiscoveryClientConfiguration and @ConditionalOnMissingBean)

@Component
public class EurekaApplicationDiscoveryListener extends ApplicationDiscoveryListener {

    @Autowired
    public EurekaApplicationDiscoveryListener(DiscoveryClient discoveryClient, ApplicationRegistry registry) {
        super(discoveryClient, registry);
    }

    @Override
    protected Application convert(ServiceInstance instance) {
        EurekaDiscoveryClient.EurekaServiceInstance eurekaServiceInstance = (EurekaDiscoveryClient.EurekaServiceInstance) instance;
        final Application converted = super.convert(instance);
        return Application.create(converted)
                .withHealthUrl(eurekaServiceInstance.getInstanceInfo().getHealthCheckUrl())
                .withManagementUrl(eurekaServiceInstance.getInstanceInfo().getHomePageUrl())
                .withServiceUrl(eurekaServiceInstance.getInstanceInfo().getHomePageUrl())
                .build();
    }
}

Would you please fix this in your code base ?
Thanks

[jvmlet]

Just noticed this from your reference guide :

Currently the implementation is not specific to any DiscoveryClient implementation. Unfortunately the DiscoveryClient doesn’t expose the health- or management-url, so if you’re not sticking to the defaults and setting spring.boot.admin.discovery.management.context-path doesn’t suffice, you can add a bean extending ApplicationDiscoveryListener to your admin server and provide your own conversion.

Looks like you are going to reject this issue :), another thing that could be done is to check the class name of ServiceInstance and try to do the best.

[joshiste]

Do you wanna submit a PR? Thhe only contstraint I have: It must run without the EurekaServiceInstance.class on the classpath and there should be an default conversation in case it's not a EurekaServiceInstance.class instance.

[joshiste]

@jvmlet so are you working on this?

[jvmlet]

Yes, the implementation itself is not so complex, I want also to include integration tests with and without eureka on the class path and maven is not my daily used tool for this:-)
Will take few more days to finish this feature.

[jvmlet]

Johannes,
I've just realized that Angel.SR4 you are using includes spring-cloud-netflix-core:1.0.4.RELEASE which doesn't expose inner EurekaDiscoveryClient.EurekaServiceInstance class , it has package modifier access level.
I'm on Brixton.M4 (spring-cloud-netflix-core:1.1.0.M4 ) and this class is defined as public + it exposes the underlying com.netflix.appinfo.InstanceInfo which is also missing from Angel.SR4

public static class EurekaServiceInstance implements ServiceInstance {
        private InstanceInfo instance;

        public InstanceInfo getInstanceInfo() {
            return instance;
        }

I have the implementation + integration test of this feature,how do you want to proceed with this ?
Thanks

[joshiste]

@jvmlet Is there a release schedule for Brixton? If the release date isn't that far we can integrate it into the master branch...

[jvmlet]

Can't find any info about Brixton roadmap ...

[joshiste]

I found a comment by Spencer Gibb, that they want to release in Q1...

[jvmlet]

OK, will PR tomorrow

[cforce]

End of this month Brixton gonna released

Furer Alexander notifications@github.com schrieb am Mo., 15. Feb. 2016
6:45 PM:

OK, will PR tomorrow

—
Reply to this email directly or view it on GitHub
#154 (comment)
.

[joshiste]

Currently the management.context-path isn't included in eurekas instanceInfo. Would you prefer to add an entry for it to each instances metadata-map or is it better to have a central property in the admin (as it is today with the current "simple" conversion) ....
I would prefer the first solution, but this would require to reconfigure all the eureka clients in case the management.context-path is set when upgrading. (or forcing the old conversion until you done so)