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

More HealthChecker implementations #1854

Open
trustin opened this issue Jun 25, 2019 · 14 comments · May be fixed by #5781
Open

More HealthChecker implementations #1854

trustin opened this issue Jun 25, 2019 · 14 comments · May be fixed by #5781
Labels
good first issue new feature sprint Issues for OSS Sprint participants

Comments

@trustin
Copy link
Member

trustin commented Jun 25, 2019

  • Healthy when memory usage is lower than a certain threshold (%)
  • Healthy when disk usage is lower than a certain threshold (%)
  • Healthy when there are no dead locks
  • Healthy when ...
@huydx
Copy link
Contributor

huydx commented Jul 6, 2019

@trustin any idea about how upstream server could propagate its state (like memory/disk..) to client side?

@trustin
Copy link
Member Author

trustin commented Jul 6, 2019

You can add more than one health checker when you build a HealthCheckService, then a load balancer or HealthCheckedEndpointGroup will notice when any of the HealthCheckers becomes unhealthy.

@huydx
Copy link
Contributor

huydx commented Jul 7, 2019

ah, seems I mistaken that you mean client side HealthChecker but seems that you talk about server side HealthCheck service, that should make sense

@4whomtbts
Copy link
Contributor

@trustin
When I was working on healthChecker for my project and I thought that It would be good If Armeria has those healthChecker implementations as default :) !

Could I work on this issue?

@trustin
Copy link
Member Author

trustin commented Feb 6, 2020

Sure. Please tell us what kind of health checkers you'd like to implement first, though. :-)

@4whomtbts
Copy link
Contributor

Sure. Please tell us what kind of health checkers you'd like to implement first, though. :-)

@trustin

  1. Memory based healthChecker
    Q :
    Do you want to the healthiness is determined by server's heap memory usage
    or system's memory usage?
    Either that or, provide both of them?

  2. disk usage rate based healthChecker

Plus, what I want to suggest to implement is CPU based healthChecker that the healthiness is determined by average CPU usage which is a result of calculation of average during certain period of time(given by programmer) not by CPU rate at the moment It's asked.

In example,


public final class CpuBasedHealthChecker implements ListenableHealthChecker { 
  ... abbreviate .. 

 public CpuBasedHealthChecker(TimeUnit unit, long interval, double threshold) {}

 @Override
 public isHealthy() {
      return true when average of CPU usage rate in given interval is less than threshold
      return false otherwise.
 }
} 

HealthChecker cpuBasedHealthChecker

           = new CpuHealthChecker(TimeUnit.MINIUTES, 10, 0.8); 

My reservations are that It could be a little bit ridiculous, because It should memorize and calculate periodically to determine whether It's healthy or because It's practically not usable.

What do you think about that?

Is it little bit ridiculous idea for healthChecker or inappropriate to be provided as default implementation?

@Bue-von-hon
Copy link
Contributor

could i continuing this issue?

@ikhoon
Copy link
Contributor

ikhoon commented Nov 25, 2022

Sure, it's all yours.

Bue-von-hon added a commit to Bue-von-hon/armeria that referenced this issue Feb 14, 2023
Motivation:

Resolves part of line#1854

Some users may want to use CPU usage to determine if their instance is healthy.
From now on, Users can perform health checks in the following ways

```
DefaultCpuHealthChecker cpuHealthChecker = new DefaultCpuHealthChecker(20.0);
cpuHealthChecker.isHealthy();
```

Modifications:

- Added DefaultCpuHealthChecker that checks cpu-usage by OperatingSystemMXBean

Result:

A User can health-check with system-cpu-usage

p.s.
Implemented this by referencing ProcessorMetrics.java in the micrometer core.
@Bue-von-hon
Copy link
Contributor

@ikhoon i sent out PR #4673
please take a look 🙏

@trustin trustin added the sprint Issues for OSS Sprint participants label Feb 16, 2023
@lazmond3
Copy link

@minwoox I also submitted a draft Pull request #4711 about this issue.
Please take a look at it when you have a moment.

jrhee17 added a commit that referenced this issue Mar 28, 2024
Motivation:

Resolves part of #1854

Some users may want to use CPU usage to determine if their instance is
healthy.
From now on, Users can perform health checks in the following ways

```java
HealthChecker cpuHealthChecker = HealthChecker.ofCpu(80, 80);
cpuHealthChecker.isHealthy();
```

Modifications:

- Added DefaultCpuHealthChecker that checks cpu-usage by
OperatingSystemMXBean

Result:

A User can health-check with system-cpu-usage

p.s.
Implemented this by referencing
[ProcessorMetrics.java](https://github.com/micrometer-metrics/micrometer/blob/main/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/system/ProcessorMetrics.java)
in the micrometer core.

---------

Co-authored-by: jrhee17 <guins_j@guins.org>
Co-authored-by: Ikhun Um <ih.pert@gmail.com>
Co-authored-by: Trustin Lee <t@motd.kr>
Co-authored-by: Trustin Lee <trustin@linecorp.com>
@Bue-von-hon
Copy link
Contributor

Looks like there was a follow-up.
I'll add more health checkers, such as disk usage based.
If there's still work to do on the memory health checker by the time I merge this, I'll follow up on that as well, @lazmond3. 🙇🏻‍♂️
Thanks.

@Bue-von-hon
Copy link
Contributor

My friend @AnneMayor wants to join, so I recommended this issue.
@AnneMayor will implement a disk usage-based health checker.
Please refer to the link below and implement a health checker based on disk usage.

link: https://github.com/micrometer-metrics/micrometer/blob/8339d57bef8689beb8d7a18b429a166f6595f2af/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/system/DiskSpaceMetrics.java

P.S. There are several options, but this seems like a good place to start.

@AnneMayor
Copy link

AnneMayor commented Jun 9, 2024

Thanks @Bue-von-hon :)
I will look into the issue.
I am going to open PR until June 18

@AnneMayor
Copy link

It might be a little long to open PR. I need more time to review. I will let you know today when I could complete it.

@AnneMayor AnneMayor linked a pull request Jun 22, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue new feature sprint Issues for OSS Sprint participants
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants