Skip to content

Commit 8fb377b

Browse files
Swiddisclaude
andauthored
Improve resource monitor errors (#5129)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 94649fb commit 8fb377b

12 files changed

Lines changed: 679 additions & 26 deletions

File tree

core/src/main/java/org/opensearch/sql/monitor/AlwaysHealthyMonitor.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ public class AlwaysHealthyMonitor extends ResourceMonitor {
1111

1212
/** always healthy. */
1313
@Override
14-
public boolean isHealthy() {
14+
protected boolean isHealthyImpl() {
1515
return true;
1616
}
17+
18+
@Override
19+
public ResourceStatus getStatus() {
20+
return ResourceStatus.healthy(ResourceStatus.ResourceType.OTHER);
21+
}
1722
}

core/src/main/java/org/opensearch/sql/monitor/ResourceMonitor.java

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,43 @@ public abstract class ResourceMonitor {
1414
* Is the resource healthy.
1515
*
1616
* @return true for healthy, otherwise false.
17+
* @throws UnsupportedOperationException if the subclass doesn't override getStatus() or
18+
* isHealthyImpl()
1719
*/
18-
public abstract boolean isHealthy();
20+
public boolean isHealthy() {
21+
return getStatus().isHealthy();
22+
}
23+
24+
/**
25+
* Get detailed resource status including context for error messages. Subclasses should override
26+
* this method to provide rich status information.
27+
*
28+
* @return ResourceStatus with health state and detailed context
29+
* @throws UnsupportedOperationException if the subclass doesn't override getStatus() or
30+
* isHealthyImpl()
31+
*/
32+
public ResourceStatus getStatus() {
33+
// Default implementation for backwards compatibility
34+
// Subclasses should override this to provide detailed status
35+
boolean healthy = isHealthyImpl();
36+
return healthy
37+
? ResourceStatus.healthy(ResourceStatus.ResourceType.OTHER)
38+
: ResourceStatus.builder()
39+
.healthy(false)
40+
.type(ResourceStatus.ResourceType.OTHER)
41+
.description("Resource monitor reports unhealthy status")
42+
.build();
43+
}
44+
45+
/**
46+
* Internal implementation for health check. Subclasses that don't override getStatus() should
47+
* override this instead.
48+
*
49+
* @return true for healthy, otherwise false.
50+
*/
51+
protected boolean isHealthyImpl() {
52+
// Subclass must override either getStatus() or isHealthyImpl()
53+
throw new UnsupportedOperationException(
54+
"Subclass must override either getStatus() or isHealthyImpl()");
55+
}
1956
}
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.monitor;
7+
8+
import lombok.Builder;
9+
import lombok.Getter;
10+
11+
/**
12+
* Represents the health status of a resource with detailed context information. This wrapper allows
13+
* error messages to include actionable details about resource exhaustion instead of just boolean
14+
* health checks.
15+
*/
16+
@Getter
17+
@Builder
18+
public class ResourceStatus {
19+
/** Type of resource being monitored. */
20+
public enum ResourceType {
21+
MEMORY,
22+
CPU,
23+
DISK,
24+
OTHER
25+
}
26+
27+
/** Whether the resource is healthy (within limits). */
28+
private final boolean healthy;
29+
30+
/** Type of resource (memory, CPU, etc.). */
31+
private final ResourceType type;
32+
33+
/** Human-readable description of resource state. */
34+
private final String description;
35+
36+
/** Current resource usage value (optional, for metrics). */
37+
private final Long currentUsage;
38+
39+
/** Maximum allowed resource value (optional, for metrics). */
40+
private final Long maxLimit;
41+
42+
/** Additional contextual information (optional). */
43+
private final String additionalContext;
44+
45+
/**
46+
* Creates a healthy status with minimal information.
47+
*
48+
* @param type Resource type
49+
* @return Healthy ResourceStatus
50+
*/
51+
public static ResourceStatus healthy(ResourceType type) {
52+
return ResourceStatus.builder()
53+
.healthy(true)
54+
.type(type)
55+
.description(type + " resources are healthy")
56+
.build();
57+
}
58+
59+
/**
60+
* Creates an unhealthy status with detailed context.
61+
*
62+
* @param type Resource type
63+
* @param currentUsage Current usage value
64+
* @param maxLimit Maximum allowed limit
65+
* @param description Human-readable description
66+
* @return Unhealthy ResourceStatus with context
67+
*/
68+
public static ResourceStatus unhealthy(
69+
ResourceType type, long currentUsage, long maxLimit, String description) {
70+
return ResourceStatus.builder()
71+
.healthy(false)
72+
.type(type)
73+
.currentUsage(currentUsage)
74+
.maxLimit(maxLimit)
75+
.description(description)
76+
.build();
77+
}
78+
79+
/**
80+
* Gets a formatted description including usage metrics if available.
81+
*
82+
* @return Formatted description string
83+
*/
84+
public String getFormattedDescription() {
85+
if (currentUsage != null && maxLimit != null) {
86+
if (maxLimit <= 0) {
87+
// Treat invalid limit as 0, don't compute percentage
88+
return String.format(
89+
"%s (current: %s, limit: %s)", description, formatBytes(currentUsage), formatBytes(0));
90+
}
91+
double percentage = (double) currentUsage / maxLimit * 100;
92+
return String.format(
93+
"%s (current: %s, limit: %s, usage: %.1f%%)",
94+
description, formatBytes(currentUsage), formatBytes(maxLimit), percentage);
95+
}
96+
return description;
97+
}
98+
99+
/**
100+
* Formats byte values into human-readable format (KB, MB, GB).
101+
*
102+
* @param bytes Byte value
103+
* @return Formatted string
104+
*/
105+
private String formatBytes(long bytes) {
106+
if (bytes < 1024) {
107+
return bytes + "B";
108+
} else if (bytes < 1024 * 1024) {
109+
return String.format("%.1fKB", bytes / 1024.0);
110+
} else if (bytes < 1024 * 1024 * 1024) {
111+
return String.format("%.1fMB", bytes / (1024.0 * 1024));
112+
} else {
113+
return String.format("%.1fGB", bytes / (1024.0 * 1024 * 1024));
114+
}
115+
}
116+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.monitor;
7+
8+
import static org.junit.jupiter.api.Assertions.assertThrows;
9+
10+
import org.junit.jupiter.api.Test;
11+
12+
class ResourceMonitorTest {
13+
14+
@Test
15+
void testDefaultImplementationThrowsException() {
16+
// Create a minimal subclass that doesn't override getStatus() or isHealthyImpl()
17+
ResourceMonitor monitor = new ResourceMonitor() {
18+
// Intentionally empty - doesn't override anything
19+
};
20+
21+
// Attempting to use the default path should throw UnsupportedOperationException
22+
assertThrows(UnsupportedOperationException.class, monitor::isHealthy);
23+
}
24+
}

0 commit comments

Comments
 (0)