search

alibaba / alibaba-rsocket-broker

Alibaba RSocket Broker: Mesh, Streaming & IoT
https://alibroker.info
Apache License 2.0
761 stars 166 forks source link

In the yaml, if rsocket.brokers is list, `RSocketBrokerHealthIndicator` is not registered. #239

Open amondnet opened 1 year ago

amondnet commented 1 year ago

Describe the bug

In the yaml, if rsocket.brokers is a list, then RSocketBrokerHealthIndicator is not registered.

    rsocket:
      brokers: 
      - tcp://localhost:9999

If it is a string, then RSocketBrokerHealthIndicator is registered.

    rsocket:
      brokers: tcp://localhost:9999

Environment

Steps to reproduce this issue

  1. set rsocket.brokers to list 
    rsocket:
  brokers: 
  - tcp://localhost:9999
management:
  endpoint:
    health:
      show-details: always
  2. bootRun
  3. open http://localhost:8080/actuator/health

Pls. provide GitHub address to reproduce this issue.

https://github.com/alibaba/alibaba-rsocket-broker/blob/fdd39b328b6ed69d7b2e320d8384f29bdc189569/alibaba-rsocket-spring-boot-starter/src/main/java/com/alibaba/spring/boot/rsocket/RSocketAutoConfiguration.java#L166

Expected Result

{
  "status": "DOWN",
  "components": {
   "mongo": {
      "status": "UP",
      "details": {
        "maxWireVersion": 17
      }
    },
    "ping": {
      "status": "UP"
    },
    "r2dbc": {
      "status": "UP",
      "details": {
        "database": "MariaDB",
        "validationQuery": "validate(REMOTE)"
      }
    },
    "rsocketBrokerHealth": {
      "status": "DOWN",
      "details": {
        "brokers": "tcp://localhost:9999"
      }
    }
  }
}

Actual Result

{
  "status": "UP",
  "components": {
    "mongo": {
      "status": "UP",
      "details": {
        "maxWireVersion": 17
      }
    },
    "ping": {
      "status": "UP"
    },
    "r2dbc": {
      "status": "UP",
      "details": {
        "database": "MariaDB",
        "validationQuery": "validate(REMOTE)"
      }
    }
  }
}
shareisall commented 1 year ago

rsocket.broker.topology=gossip rsocket.broker.seeds=192.168.1.2,192.168.1.3,192.168.1.4

amondnet commented 1 year ago

@shareisall

rsocket.broker.topology=gossip rsocket.broker.seeds=192.168.1.2,192.168.1.3,192.168.1.4

The above properties are used by the alibaba-rsocket-server. This issue occurs in alibaba-rsocket-spring-boot-starter..

https://github.com/alibaba/alibaba-rsocket-broker/blob/fdd39b328b6ed69d7b2e320d8384f29bdc189569/alibaba-rsocket-spring-boot-starter/src/main/java/com/alibaba/spring/boot/rsocket/RSocketAutoConfiguration.java#L166

    @Bean
    @ConditionalOnProperty("rsocket.brokers")
    public RSocketBrokerHealthIndicator rsocketBrokerHealth(RSocketEndpoint rsocketEndpoint, UpstreamManager upstreamManager, @Value("${rsocket.brokers}") String brokers) {
        return new RSocketBrokerHealthIndicator(rsocketEndpoint, upstreamManager, brokers);
    }

If you use application.properties or application.yaml + string, the RSocketBrokerHealthIndicator will be registered without any problems.

rsocket.brokers=tcp://localhost:9999
rsocket:
  brokers: tcp://localhost:9999

However, if you use an array of strings, it won't register.

rsocket:
  brokers:
  - tcp://localhost:9999

This is fine for practical use, but kubernetes scheduler can't perform health checks properly because the health indicator is not registered.

shareisall commented 1 year ago

Sorry for last reply. But it works for me.

Envirements:

Code:

rsocket:
  brokers:
    - tcp://127.0.0.1:9999
    - tcp://127.0.0.1:9999
  jwt-token: None
shareisall commented 1 year ago

Array indentation should be 2 space in your yaml file.

amondnet commented 1 year ago

@shareisall

I've been using alibaba-rsocket-broker in production for over a year. I'm not talking about the rsocket app not working, I'm talking about the health indicator not registering.

https://github.com/alibaba/alibaba-rsocket-broker/blob/master/alibaba-rsocket-spring-boot-starter/src/main/java/com/alibaba/spring/boot/rsocket/RSocketBrokerHealthIndicator.java

https://github.com/alibaba/alibaba-rsocket-broker/blob/fdd39b328b6ed69d7b2e320d8384f29bdc189569/alibaba-rsocket-spring-boot-starter/src/main/java/com/alibaba/spring/boot/rsocket/RSocketAutoConfiguration.java#L167

In the yaml, if rsocket.brokers is a list, then RSocketBrokerHealthIndicator is not registered.

  1. use string

    rsocket:
brokers: tcp://127.0.0.1:9999
jwt-token: None
management:
endpoint:
  health:
    show-details: always
    curl http://localhost:8080/actuator/health/rsocketBrokerHealth
    {"status":"UP","details":{"brokers":"tcp://127.0.0.1:9999","localServiceStatus":"Serving"}}

  2. use array

    rsocket:
brokers: 
  - tcp://127.0.0.1:9999
jwt-token: None
management:
endpoint:
  health:
    show-details: always
    curl http://localhost:8080/actuator/health/rsocketBrokerHealth

    -->

shareisall commented 1 year ago

You are right, there's a bug in the method rsocketBrokerHealth of class com.alibaba.spring.boot.rsocket.RSocketAutoConfiguration.

CODE:

@Bean
@ConditionalOnProperty("rsocket.brokers")
public RSocketBrokerHealthIndicator rsocketBrokerHealth(
    RSocketEndpoint rsocketEndpoint,
    UpstreamManager upstreamManager,
    @Value("${rsocket.brokers}") String brokers
) {
        return new RSocketBrokerHealthIndicator(rsocketEndpoint, upstreamManager, brokers);
}