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

health actuator for solace binder #55

Closed
wants to merge 1 commit into from
Closed

health actuator for solace binder #55

wants to merge 1 commit into from

Conversation

GreenRover
Copy link
Contributor

Spring cloud stream - spring health integration

Scenario starting up

Try starting the application once with only main_session in application.yaml.
To do that remove all other sessions, except main_session part and specify main_session on second binder.
The application should start up without problem. Health page goes up.

Then do it again with a binder which has a connection but no working credentials. The application will not come up crash and health page is never on "up".

Scenario user enabled changed / deleted while running

Use i.e. third session and create a user and put its credentials into that session.
Start application, it should come up normally. The health page shows "up".

If the health indicator is implemented, that health page is showing more information about the sessions:
http://localhost:9003/actuator/health shows:

{
  "status": "UP",
  "components": {
    "binders": {
      "status": "UP",
      "components": {
        "main_session": {
          "status": "UP"
        },
        "third_session": {
          "status": "UP"
        }
      }
    }
  }
}

Delete the user via solace management or deactivate it. As a result solace stops the connection and jcsmp tries to reconnect.
After the reconnection failed after several retries, the binder stops working entirely. But still the health page shows up.

You get a log error message like that: "403: Client Username Is Shutdown".

The binder is not able to recover from this situation.

Recreate the user or activate it again - the session remains closed and you get the same error message again.

With fixed health page the health page is down as a result like this:
Note: in this scenario only one of the session components is down, third_session.

{
  "status": "DOWN",
  "components": {
    "binders": {
      "status": "DOWN",
      "components": {
        "main_session": {
          "status": "UP"
        },
        "third_session": {
          "status": "DOWN"
        }
      }
    }
  }
}

Scenario broker gets down for a time

The reconnect duration is taking a lot of time but eventually you get "Connection timed out: no further information"
and the health goes to down:

{
  "status": "DOWN",
  "components": {
    "binders": {
      "status": "DOWN",
      "components": {
        "main_session": {
          "status": "DOWN"
        },
        "third_session": {
          "status": "DOWN"
        }
      }
    }
  }
}

Note: in this scenario all sessions are down.

Usage of the health indicator

Those health indicator can be used by kubernetes to kill / restart the pod/application if there is no connection to broker any more (and will not recover).
In this case we can leave the error reporting / disaster handling to kubernetes.

@GreenRover
Copy link
Contributor Author

@Mrc0113 any updates here? How we can make progress?

@Mrc0113
Copy link
Contributor

Mrc0113 commented Apr 22, 2021

Hi @GreenRover, thanks for pinging me on this. I love what you have done and agree we should enable the ability to get the status of binders via actuator.

Here are my thoughts on the details:

  1. Actuator should be an optional dependency that you only include when you want to use actuator. I noticed they have actuator listed at the spring-cloud-stream level so do we actually need to add it here?
  2. We probably need a test or 2. I think the scenarios are great for manual testing, but any ideas for automated testing to ensure this functionality works going forward? Probably just mock the triggering of the different SessionEvent types.
  3. I'm going to do some research internally with our engineering team to make sure we're not overlooking anything with the SessionEvents, when they occur and how we're mapping them to the actuator health status options.

@GreenRover
Copy link
Contributor Author

Hi @Mrc0113 any updates on this issue? Any feedback from your team?

Automated testing could be hard at this point. Especially we only serve a spring boot api that do the real job.

@GreenRover
Copy link
Contributor Author

Hi @Mrc0113 any updates on this issue? Any feedback from your team?

@Nephery Nephery changed the base branch from stage-2.0.0 to stage-2.3.0 November 12, 2021 16:40
@Nephery Nephery changed the base branch from stage-2.3.0 to stage-2.0.0 November 12, 2021 16:41
@@ -50,6 +50,11 @@
<artifactId>spring-cloud-stream</artifactId>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-actuator</artifactId>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The artifact ID must be spring-boot-starter-actuator. Also make this an optional dependency.

@@ -27,15 +30,18 @@

private JCSMPSession jcsmpSession;

private SolaceHealthIndicator solaceHealthIndicator = new SolaceHealthIndicator();
Copy link
Collaborator

@Nephery Nephery Nov 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including the SessionEventHandler into the session needs to be reworked to handle actuator being an optional dependency, where the actuator classes might not be present on the class path.

An idea on how you could achieve this:

  1. @Import the SolaceHealthIndicatorConfiguration to SolaceMessageChannelBinderConfiguration.
  2. Split the SessionEventHandler from SolaceHealthIndicator and make it its own bean. Where the SolaceHealthIndicator bean would depend on the SessionEventHandler bean.
  3. @Autowired(required = false) the SessionEventHandler here and use it when creating the JCSMPSession.

Comment on lines +15 to +21
protected void doHealthCheck(Health.Builder builder) throws Exception {
if(connected.get()) {
builder.up();
} else {
builder.down();
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • If the SessionEventArgs has a non-null getException(), set it as an exception on the Heath.Builder.
  • If the SessionEventArgs has a non-zero getResponseCode(), set it as a detail on the Heath.Builder.
  • If the SessionEventArgs has a non-null getInfo(), set it as a detail on the Heath.Builder.

@Nephery Nephery mentioned this pull request Mar 2, 2022
@Nephery Nephery mentioned this pull request Mar 9, 2022
@Nephery
Copy link
Collaborator

Nephery commented Mar 9, 2022

@GreenRover Thank you for the opening this pull request. We've added support for the health actuator in #125 to be released in the 2.3.0 solace-spring-cloud release.

Closing this pull request as a duplicate of #125.

@Nephery Nephery closed this Mar 9, 2022
@Nephery Nephery added the duplicate This issue or pull request already exists label Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants