Skip to content

Commit 2d34120

Browse files
author
Pradeep L
committed
increasing test coverage
Signed-off-by: Pradeep L <spradeel@amazon.com>
1 parent 7cc959b commit 2d34120

2 files changed

Lines changed: 145 additions & 23 deletions

File tree

server/src/main/java/org/opensearch/monitor/os/OsProbe.java

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.opensearch.common.io.PathUtils;
4040
import org.opensearch.monitor.Probes;
4141

42-
import java.io.BufferedReader;
4342
import java.io.IOException;
4443
import java.lang.management.ManagementFactory;
4544
import java.lang.management.OperatingSystemMXBean;
@@ -259,40 +258,47 @@ public long getProcessRssAnon() {
259258
}
260259

261260
/**
262-
* Reads the {@code RssAnon} field from {@code /proc/self/status}. Package-private so tests
263-
* can override it with canned file contents.
261+
* Reads the lines of {@code /proc/self/status}. Package-private so tests can override it
262+
* with canned file contents.
263+
*
264+
* @return the lines of {@code /proc/self/status}
265+
* @throws IOException if the file cannot be opened or read
266+
*/
267+
@SuppressForbidden(reason = "access /proc/self/status")
268+
List<String> readProcSelfStatus() throws IOException {
269+
return Files.readAllLines(PathUtils.get("/proc/self/status"));
270+
}
271+
272+
/**
273+
* Reads the {@code RssAnon} field from {@code /proc/self/status}.
264274
*
265275
* @return the {@code RssAnon} value in bytes, or {@code -1L} when the line is missing or
266276
* the value is unparseable/negative
267277
* @throws IOException if the file cannot be opened or read
268278
*/
269-
@SuppressForbidden(reason = "access /proc/self/status")
270279
long readRssAnonFromProcSelfStatus() throws IOException {
271-
try (BufferedReader reader = Files.newBufferedReader(PathUtils.get("/proc/self/status"))) {
272-
String line;
273-
while ((line = reader.readLine()) != null) {
274-
if (line.startsWith("RssAnon:")) {
275-
// Format: "RssAnon:\t 12345 kB"
276-
String[] parts = line.split("\\s+");
277-
if (parts.length >= 2) {
278-
try {
279-
long kb = Long.parseLong(parts[1]);
280-
if (kb < 0L) {
281-
return -1L;
282-
}
283-
return kb * 1024L;
284-
} catch (NumberFormatException nfe) {
285-
logger.warn("malformed RssAnon value in /proc/self/status", nfe);
280+
for (final String line : readProcSelfStatus()) {
281+
if (line.startsWith("RssAnon:")) {
282+
// Format: "RssAnon:\t 12345 kB"
283+
final String[] parts = line.split("\\s+");
284+
if (parts.length >= 2) {
285+
try {
286+
final long kb = Long.parseLong(parts[1]);
287+
if (kb < 0L) {
286288
return -1L;
287289
}
290+
return kb * 1024L;
291+
} catch (NumberFormatException nfe) {
292+
logger.warn("malformed RssAnon value in /proc/self/status", nfe);
293+
return -1L;
288294
}
289-
logger.warn("RssAnon line has unexpected shape: [{}]", line);
290-
return -1L;
291295
}
296+
logger.warn("RssAnon line has unexpected shape: [{}]", line);
297+
return -1L;
292298
}
293-
logger.warn("RssAnon line not found in /proc/self/status");
294-
return -1L;
295299
}
300+
logger.warn("RssAnon line not found in /proc/self/status");
301+
return -1L;
296302
}
297303

298304
public short getSystemCpuPercent() {

server/src/test/java/org/opensearch/monitor/os/OsProbeTests.java

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@
5656
import static org.hamcrest.Matchers.lessThanOrEqualTo;
5757
import static org.hamcrest.Matchers.notNullValue;
5858
import static org.junit.Assume.assumeThat;
59+
import static org.mockito.ArgumentMatchers.any;
5960
import static org.mockito.ArgumentMatchers.anyString;
61+
import static org.mockito.ArgumentMatchers.eq;
6062
import static org.mockito.Mockito.mock;
6163
import static org.mockito.Mockito.never;
6264
import static org.mockito.Mockito.reset;
@@ -310,6 +312,120 @@ List<String> readSysFsCgroupCpuAcctCpuStat(String controlGroup) throws IOExcepti
310312
verify(logger, never()).warn(anyString());
311313
}
312314

315+
public void testGetProcessRssAnonNotLinuxReturnsNegativeOne() {
316+
assumeFalse("test runs on non-Linux only", Constants.LINUX);
317+
final OsProbe probe = new OsProbe() {
318+
@Override
319+
List<String> readProcSelfStatus() {
320+
throw new AssertionError("readProcSelfStatus should not be called on non-Linux");
321+
}
322+
};
323+
assertThat(probe.getProcessRssAnon(), equalTo(-1L));
324+
}
325+
326+
public void testGetProcessRssAnonReturnsBytes() {
327+
assumeTrue("test runs on Linux only", Constants.LINUX);
328+
final OsProbe probe = new OsProbe() {
329+
@Override
330+
List<String> readProcSelfStatus() {
331+
return Arrays.asList("Name:\topensearch", "VmRSS:\t 98304 kB", "RssAnon:\t 4096 kB", "RssFile:\t 2048 kB");
332+
}
333+
};
334+
assertThat(probe.getProcessRssAnon(), equalTo(4096L * 1024L));
335+
}
336+
337+
public void testGetProcessRssAnonHandlesIOException() {
338+
assumeTrue("test runs on Linux only", Constants.LINUX);
339+
final Logger logger = mock(Logger.class);
340+
final OsProbe probe = new OsProbe(logger) {
341+
@Override
342+
List<String> readProcSelfStatus() throws IOException {
343+
throw new IOException("simulated failure");
344+
}
345+
};
346+
assertThat(probe.getProcessRssAnon(), equalTo(-1L));
347+
verify(logger, times(1)).warn(eq("failed to read /proc/self/status"), any(IOException.class));
348+
}
349+
350+
public void testReadRssAnonFromProcSelfStatusValidValue() throws IOException {
351+
final OsProbe probe = new OsProbe() {
352+
@Override
353+
List<String> readProcSelfStatus() {
354+
return Arrays.asList("Name:\topensearch", "VmPeak:\t 131072 kB", "RssAnon:\t 12345 kB", "RssShmem:\t 64 kB");
355+
}
356+
};
357+
assertThat(probe.readRssAnonFromProcSelfStatus(), equalTo(12345L * 1024L));
358+
}
359+
360+
public void testReadRssAnonFromProcSelfStatusZeroValue() throws IOException {
361+
final OsProbe probe = new OsProbe() {
362+
@Override
363+
List<String> readProcSelfStatus() {
364+
return Collections.singletonList("RssAnon:\t 0 kB");
365+
}
366+
};
367+
assertThat(probe.readRssAnonFromProcSelfStatus(), equalTo(0L));
368+
}
369+
370+
public void testReadRssAnonFromProcSelfStatusNegativeValue() throws IOException {
371+
final OsProbe probe = new OsProbe() {
372+
@Override
373+
List<String> readProcSelfStatus() {
374+
return Collections.singletonList("RssAnon:\t -1 kB");
375+
}
376+
};
377+
assertThat(probe.readRssAnonFromProcSelfStatus(), equalTo(-1L));
378+
}
379+
380+
public void testReadRssAnonFromProcSelfStatusMalformedValue() throws IOException {
381+
final Logger logger = mock(Logger.class);
382+
final OsProbe probe = new OsProbe(logger) {
383+
@Override
384+
List<String> readProcSelfStatus() {
385+
return Collections.singletonList("RssAnon:\tnotanumber kB");
386+
}
387+
};
388+
assertThat(probe.readRssAnonFromProcSelfStatus(), equalTo(-1L));
389+
verify(logger, times(1)).warn(eq("malformed RssAnon value in /proc/self/status"), any(NumberFormatException.class));
390+
}
391+
392+
public void testReadRssAnonFromProcSelfStatusUnexpectedShape() throws IOException {
393+
final Logger logger = mock(Logger.class);
394+
final OsProbe probe = new OsProbe(logger) {
395+
@Override
396+
List<String> readProcSelfStatus() {
397+
// No whitespace after the label, so split produces a single token
398+
return Collections.singletonList("RssAnon:");
399+
}
400+
};
401+
assertThat(probe.readRssAnonFromProcSelfStatus(), equalTo(-1L));
402+
verify(logger, times(1)).warn("RssAnon line has unexpected shape: [{}]", "RssAnon:");
403+
}
404+
405+
public void testReadRssAnonFromProcSelfStatusMissingLine() throws IOException {
406+
final Logger logger = mock(Logger.class);
407+
final OsProbe probe = new OsProbe(logger) {
408+
@Override
409+
List<String> readProcSelfStatus() {
410+
return Arrays.asList("Name:\topensearch", "VmRSS:\t 98304 kB");
411+
}
412+
};
413+
assertThat(probe.readRssAnonFromProcSelfStatus(), equalTo(-1L));
414+
verify(logger, times(1)).warn("RssAnon line not found in /proc/self/status");
415+
}
416+
417+
public void testReadRssAnonFromProcSelfStatusEmptyFile() throws IOException {
418+
final Logger logger = mock(Logger.class);
419+
final OsProbe probe = new OsProbe(logger) {
420+
@Override
421+
List<String> readProcSelfStatus() {
422+
return Collections.emptyList();
423+
}
424+
};
425+
assertThat(probe.readRssAnonFromProcSelfStatus(), equalTo(-1L));
426+
verify(logger, times(1)).warn("RssAnon line not found in /proc/self/status");
427+
}
428+
313429
private static List<String> getProcSelfGroupLines(String hierarchy) {
314430
return Arrays.asList(
315431
"10:freezer:/",

0 commit comments

Comments
 (0)