-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Health Check 스케줄러 추가 #38
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
Conversation
WalkthroughA new class named Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as HealthCheckScheduler
participant Spring as Spring Scheduler
participant Server as Local Server (/actuator/health)
participant Logger as Logger
Spring->>Scheduler: Trigger performHealthCheck() every 60s
Scheduler->>Server: GET /actuator/health
alt Success
Server-->>Scheduler: Health status response
else Exception
Scheduler->>Logger: Log error with exception details
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/java/com/example/Jinus/scheduler/HealthCheckScheduler.java (4)
16-17: Consider using a bean for RestTemplate.Creating a RestTemplate instance directly is functional but not ideal for a Spring application. Consider defining it as a bean in a configuration class for better manageability, reusability, and testability.
- private final RestTemplate restTemplate = new RestTemplate(); + @Autowired + private RestTemplate restTemplate;And add in a configuration class:
@Configuration public class WebClientConfig { @Bean public RestTemplate restTemplate() { return new RestTemplate(); } }
18-19: Consider using cron expression for more control.The
fixedRateapproach works, but using a cron expression might give you more control over when the health check runs.- @Scheduled(fixedRate = 60000) // 60초마다 실행 + @Scheduled(cron = "0 */1 * * * *") // Run every minute
23-25: Consider more specific exception handling.Catching all exceptions can mask specific issues. Consider handling different types of exceptions separately for better diagnostics.
try { restTemplate.getForEntity(url, String.class); - } catch (Exception e) { - logger.error("Health check failed: {}", e.getMessage(), e); + } catch (ResourceAccessException e) { + logger.error("Health check connection failed: {}", e.getMessage(), e); + } catch (HttpClientErrorException e) { + logger.error("Health check client error: {} - {}", e.getStatusCode(), e.getMessage(), e); + } catch (HttpServerErrorException e) { + logger.error("Health check server error: {} - {}", e.getStatusCode(), e.getMessage(), e); + } catch (Exception e) { + logger.error("Health check unexpected error: {}", e.getMessage(), e); }
9-27: Add configurability to the health check URL.The health check URL is hardcoded to localhost. Consider making it configurable to allow checking different services or environments.
@Value("${server.port:8080}") private int serverPort; + @Value("${health.check.path:/actuator/health}") + private String healthCheckPath; + + @Value("${health.check.host:localhost}") + private String healthCheckHost; private final RestTemplate restTemplate = new RestTemplate(); @Scheduled(fixedRate = 60000) // 60초마다 실행 public void performHealthCheck() { - String url = "http://localhost:" + serverPort + "/actuator/health"; + String url = "http://" + healthCheckHost + ":" + serverPort + healthCheckPath; try { restTemplate.getForEntity(url, String.class); } catch (Exception e) { logger.error("Health check failed: {}", e.getMessage(), e); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/example/Jinus/scheduler/HealthCheckScheduler.java(1 hunks)
🔇 Additional comments (3)
src/main/java/com/example/Jinus/scheduler/HealthCheckScheduler.java (3)
1-8: Imports look good.All necessary imports for Spring scheduling, REST client, component scanning, and logging are properly included.
13-15: Good use of property injection with fallback.Using
@Value("${server.port:8080}")with a default value is a good practice as it ensures the application will work even if the property is not explicitly defined.
9-10:Details
✅ Verification successful
Verify that scheduling is enabled in your configuration.
The
@Componentannotation is correctly applied, but ensure that@EnableSchedulingis configured in a configuration class for the scheduler to function properly.
🏁 Script executed:
#!/bin/bash # Check if @EnableScheduling is configured in the application rg -l "@EnableScheduling" --type javaLength of output: 91
Scheduling Configuration Verified
@EnableSchedulingis present insrc/main/java/com/example/Jinus/JinusApplication.java, so your scheduled tasks will run.@ComponentonHealthCheckScheduleris correctly applied.No further changes are needed.
| String url = "http://localhost:" + serverPort + "/actuator/health"; | ||
| try { | ||
| restTemplate.getForEntity(url, String.class); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add response handling to verify health status.
The current implementation only confirms that the endpoint is reachable but doesn't verify the actual health status. Spring Boot's actuator returns JSON with a status field that should be checked.
public void performHealthCheck() {
String url = "http://localhost:" + serverPort + "/actuator/health";
try {
- restTemplate.getForEntity(url, String.class);
+ ResponseEntity<Map> response = restTemplate.getForEntity(url, Map.class);
+ String status = (String) ((Map) response.getBody()).get("status");
+ if (!"UP".equals(status)) {
+ logger.warn("Application health check returned status: {}", status);
+ } else {
+ logger.debug("Health check successful: status UP");
+ }
} catch (Exception e) {
logger.error("Health check failed: {}", e.getMessage(), e);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String url = "http://localhost:" + serverPort + "/actuator/health"; | |
| try { | |
| restTemplate.getForEntity(url, String.class); | |
| } catch (Exception e) { | |
| public void performHealthCheck() { | |
| String url = "http://localhost:" + serverPort + "/actuator/health"; | |
| try { | |
| ResponseEntity<Map> response = restTemplate.getForEntity(url, Map.class); | |
| String status = (String) ((Map) response.getBody()).get("status"); | |
| if (!"UP".equals(status)) { | |
| logger.warn("Application health check returned status: {}", status); | |
| } else { | |
| logger.debug("Health check successful: status UP"); | |
| } | |
| } catch (Exception e) { | |
| logger.error("Health check failed: {}", e.getMessage(), e); | |
| } | |
| } |
Summary by CodeRabbit