Document Sidecar's Properties that override normal eureka instance properties

[tkvangorder]

We are using Brixton.M5 and I noticed that the port being registered with Eureka is incorrect. The SidecarConfiguration registers its own copy of the EurekaInstanceConfigBean.

The server port is injected as a value into the configuration class but never used. Instead it only consults the "sidecar" properties.

This should be a pretty easy fix: If the sidecar properties are not defined, fallback to logic that is similar to that found in EurekaClientAutoConfiguration.eurekaInstanceConfigBean.

@spencergibb Let me know if you would like help with this one, I can put a pull request together.

[spencergibb]

@tkvangorder Don't you want the sidecar properties port to be the one registered with eureka? If server.port is used it will register the port of the sidecar, which doesn't make sense to me.

[tkvangorder]

Yeah, It definitely makes sense to use the parent application's port. I guess what was surprising is that you can already define the eureka instance information via the eureka.instance.* fields.

The sidecar properties introduce confusion because they override what is being set in the instance fields. Maybe a better approach would be to drop the sidecar properties and highlight in the documentation that the instance information should point to the parent application's port?

As it is now:

We actually use the actuator on the sidecar and have a health indicator check that pings the parent application.

We were setting up the health, and status values in eureka.instance.* :

eureka:
  # The instance information is used to register this service with the discovery service. We include a version with the metadata
  # and this version is used as a sanity check between the clients and the servers. This must be set BEFORE the spring boot application
  # is initialized. We also register the actuator's endpoints with the agent as well.
  instance:
    health-check-url-path: ${server.contextPath:}/health
    status-page-url-path: ${server.contextPath:}/info

Without setting the sidecar properties for the node app listening on port 3000 (and the sidecar listening on port 8082), our metadata ends up looking like this:

<instance>
  .
  .
  <port enabled="true">0</port>
  <securePort enabled="false">443</securePort>
  .
  .
  <homePageUrl>/info</homePageUrl>
  <statusPageUrl>http://TVANGO1001034.Build.Internal:8080/info</statusPageUrl>
  <healthCheckUrl>http://TVANGO1001034.Build.Internal:8080/health</healthCheckUrl>

</instance>

It's easy enough to fix but it was behavior that changed between Brixton.M3 and Brixton.M5

[spencergibb]

What changed in the behavior?

I've added constraints on the port here a185755.

@Max(65535)
@Min(1)

[tkvangorder]

OK, I think I tracked down the change to the health and status pages:

in M3, the server port was used to construct the URI:

        @Value("${server.port:${SERVER_PORT:${PORT:8080}}}")
        private int serverPort = 8080;

.
.
.
            String scheme = config.getSecurePortEnabled() ? "https" : "http";
            config.setStatusPageUrl(scheme + "://" + config.getHostname() + ":"
                    + this.serverPort + config.getStatusPageUrlPath());
            config.setHealthCheckUrl(scheme + "://" + config.getHostname() + ":"
                    + this.serverPort + config.getHealthCheckUrlPath());

in M5, the management port was used to construct the URI and the server port is not used.

        @Value("${server.port:${SERVER_PORT:${PORT:8080}}}")
        private int serverPort = 8080;

        @Value("${management.port:${MANAGEMENT_PORT:${PORT:8080}}}")
        private int managementPort = 8080;

.
.
            config.setStatusPageUrl(scheme + "://" + config.getHostname() + ":"
                    + this.managementPort + config.getStatusPageUrlPath());
            config.setHealthCheckUrl(scheme + "://" + config.getHostname() + ":"
                    + this.managementPort + config.getHealthCheckUrlPath());

In EurekaClientAutoConfig, the management port falls back to the server port, before defaulting to 8080

    @Value("${management.port:${MANAGEMENT_PORT:${server.port:${SERVER_PORT:${PORT:8080}}}}}")

Regardless, trying to figure out which sidecar properties override the normal eureka.instance properties is rough.

It might be better to just drop the configuration of the EurekanstanceConfigBean from the SidecarConfiguration and instead document how to use the "StatusPageUrlPath", "HealthCheckUrlPath", management port, and server port to correctly define things.

Or make the instance exposed in the SidecarConfiguration conditional and allow a sidecar to provide its own? I guess you could do this today with a @primary annotation.

[spencergibb]

I've updated managementPort to behave like the one in EurekaClientAutoConfig. A @Primary EurekanstanceConfigBean should work. More docs are needed for sure.

[yankhonskikendaxa]

@tkvangorder Don't you want the sidecar properties port to be the one registered with eureka? If server.port is used it will register the port of the sidecar, which doesn't make sense to me.

I had the same issue. I have spring gateway and spring registry. I have a python-service and java sidecar for it. I want registry to see only sidecar, and to know nothing about the python app.

I want nobody to know about python app, but the only sidecar (which has zuul proxy) will forward request to the sidecar.

[spencergibb]

Sidecar doesn't work that way. It does not proxy requests to the service. It only registers the port of the external process (in your case python).