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

Added DefaultCpuHealthChecker #4673

Merged
merged 41 commits into from
Mar 28, 2024
Merged
Changes from 11 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
f08cb8b
Added DefaultCpuHealthChecker
Bue-von-hon Feb 14, 2023
08c706e
Fixed javadoc and lint based on code review
Bue-von-hon Feb 28, 2023
551263a
Fix Lint
Bue-von-hon Mar 1, 2023
afd1843
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Def…
Bue-von-hon Mar 11, 2023
5e5069e
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Def…
Bue-von-hon Mar 11, 2023
e4132b4
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Def…
Bue-von-hon Mar 11, 2023
35da7bc
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Def…
Bue-von-hon Mar 11, 2023
a0c04bb
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Def…
Bue-von-hon Mar 11, 2023
dd68de9
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Def…
Bue-von-hon Mar 11, 2023
8934066
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Def…
Bue-von-hon Mar 11, 2023
32fd4f7
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Def…
Bue-von-hon Mar 11, 2023
0a0f815
Fixed javadoc, access modifier, invoke method's algorithm and Added T…
Bue-von-hon Mar 18, 2023
3b1770b
Fix Lint, Construct method's access level
Bue-von-hon Apr 5, 2023
c8c1664
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Def…
Bue-von-hon Jun 8, 2023
eda64c4
Fixed base on code review
Bue-von-hon Jun 9, 2023
a4733f9
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Jun 24, 2023
8be0675
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Jul 2, 2023
07c4f02
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Jul 24, 2023
d5f162f
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Aug 8, 2023
760496c
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Aug 28, 2023
a1df8fa
Resolves code review
Bue-von-hon Aug 28, 2023
3f4a5fc
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Sep 7, 2023
0b14883
Fix lint
Bue-von-hon Sep 7, 2023
667a0bf
Merge remote-tracking branch 'Bue-von-hon/cpu-based-health-checker' i…
Bue-von-hon Sep 7, 2023
2dae89c
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Oct 1, 2023
9d7cd92
Optimize CPU Health Check Implementations
Bue-von-hon Nov 15, 2023
69df576
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Nov 21, 2023
8f2b4a1
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Nov 22, 2023
f6ef458
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Cpu…
Bue-von-hon Jan 30, 2024
a766919
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Cpu…
Bue-von-hon Jan 30, 2024
f469ac7
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Feb 26, 2024
fad1155
Resolves code review
Bue-von-hon Feb 26, 2024
29048d3
Rewrite the test correctly
Bue-von-hon Feb 26, 2024
20e3188
Merge branch 'main' into cpu-based-health-checker
Bue-von-hon Mar 21, 2024
5535d6a
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Cpu…
Bue-von-hon Mar 22, 2024
759763b
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Cpu…
Bue-von-hon Mar 22, 2024
fb59ec4
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Cpu…
Bue-von-hon Mar 22, 2024
d1a06e3
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Cpu…
Bue-von-hon Mar 22, 2024
1dda317
Update core/src/main/java/com/linecorp/armeria/server/healthcheck/Cpu…
Bue-von-hon Mar 22, 2024
828e51a
Add a factory method to an HealthChecker interface
Bue-von-hon Mar 22, 2024
d619208
supplement javadocs
jrhee17 Mar 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package com.linecorp.armeria.server.healthcheck;

import static java.util.Objects.requireNonNull;

import java.lang.management.ManagementFactory;
import java.lang.management.OperatingSystemMXBean;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List;

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also leave a link where this class was forked from?

Suggested change
// Forked from https://github.com/micrometer-metrics/micrometer/blob/8339d57bef8689beb8d7a18b429a166f6595f2af/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/system/ProcessorMetrics.java

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i added!

import com.linecorp.armeria.common.annotation.Nullable;

/**
* Forked from <a href="https://github.com/micrometer-metrics/micrometer/blob/8339d57bef8689beb8d7a18b429a166f6595f2af/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/system/ProcessorMetrics.java">ProcessorMetrics.java</a> in the micrometer core.
*/
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be part of Javadoc. Could you use // instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Fixed!

public class DefaultCpuHealthChecker implements HealthChecker {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we hide the implementation from the public API, we can get more freedom on changes.
Should we add a factory method of this class to HealthChecker and make this package-private?

// The simplest one.
HealthChecker.ofCpu(cpuUsage);
// A complex configuation.
HealthChecker.ofCpu(cpuUsage, cpuIdle...);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha, so let's make this class pakage-private and add a factory method to HealthChecker!


private static final List<String> OPERATING_SYSTEM_BEAN_CLASS_NAMES = ImmutableList.of(
"com.ibm.lang.management.OperatingSystemMXBean", // J9
"com.sun.management.OperatingSystemMXBean" // HotSpot
);

private final OperatingSystemMXBean operatingSystemBean;

private final Class<?> operatingSystemBeanClass;

@Nullable
private final Method systemCpuUsage;

private final double targetCpuUsage;

@Nullable
private final Method processCpuUsage;

private final double targetProcessCpuLoad;

/**
* Instantiates a new Default cpu health checker.
*
* @param cpuUsage the cpu usage
* @param cpuIdle the cpu idle
* @param processCpuUsage the process cpu usage
* @param processCpuIdle the process cpu idle
*/
public DefaultCpuHealthChecker(int cpuUsage, int cpuIdle,
final int processCpuUsage, final int processCpuIdle) {
this.targetCpuUsage = (double) cpuUsage / cpuIdle;
this.targetProcessCpuLoad = (double) processCpuUsage / processCpuIdle;
Copy link
Contributor

Choose a reason for hiding this comment

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

Question) I'm unsure why we're dividing usage by idleness. Can't we just receive two doubles targetCpuUsage, targetProcessCpuLoad as parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using two is more readable. fixed it!

this.operatingSystemBean = ManagementFactory.getOperatingSystemMXBean();
this.operatingSystemBeanClass = requireNonNull(getFirstClassFound(OPERATING_SYSTEM_BEAN_CLASS_NAMES));
this.systemCpuUsage = detectMethod("getSystemCpuLoad");
this.processCpuUsage = detectMethod("getProcessCpuLoad");
}

@Override
public boolean isHealthy() {
final double currentSystemCpuUsage = invoke(systemCpuUsage);
final double currentProcessCpuUsage = invoke(processCpuUsage);
return currentSystemCpuUsage <= targetCpuUsage && currentProcessCpuUsage <= targetProcessCpuLoad;
}

private double invoke(final Method method) {
try {
return (double) method.invoke(operatingSystemBean);
Copy link
Contributor

Choose a reason for hiding this comment

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

This method will be frequently invoked whenever HealthCheckService receives a request, hence MethodHandles might be a better choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the lookup#unreflect method, extracted the MethodHandle object from the Method class!

} catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
return Double.NaN;
}
}

@Nullable
private Method detectMethod(final String name) {
jrhee17 marked this conversation as resolved.
Show resolved Hide resolved
try {
// ensure the Bean we have is actually an instance of the interface
requireNonNull(operatingSystemBeanClass.cast(operatingSystemBean));
return operatingSystemBeanClass.getMethod(name);
} catch (ClassCastException | NoSuchMethodException | SecurityException e) {
return null;
}
}

@Nullable
private static Class<?> getFirstClassFound(final List<String> classNames) {
for (String className : classNames) {
try {
return Class.forName(className, false, getClass().getClassLoader());
} catch (ClassNotFoundException ignore) {
}
}
return null;
}
}