Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
amondnet opened this issue Oct 11, 2023 · 6 comments

Comments

@amondnet
Copy link

amondnet commented Oct 11, 2023

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

  • Alibaba RSocket Broker version: 1.1.5
  • Operating System version: Mac
  • Java version: 17

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.

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
Copy link

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

@amondnet
Copy link
Author

@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..

    @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
Copy link

Sorry for last reply. But it works for me.

Envirements:

  • SpringBoot 3.1.4
  • Alibaba RSocket Broker version: 1.1.5
  • Operating System version: Mac
  • Java version: 17

Code:

rsocket:
  brokers:
    - tcp://127.0.0.1:9999
    - tcp://127.0.0.1:9999
  jwt-token: None

@shareisall
Copy link

Array indentation should be 2 space in your yaml file.

@amondnet
Copy link
Author

@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

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

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"}}
  1. 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
Copy link

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);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants