Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Improve Test coverage #251

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
6 changes: 3 additions & 3 deletions .github/workflows/gradle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ jobs:
- name: Build PA and run Unit Tests
working-directory: ./tmp/pa
run: ./gradlew build
- name: Run Integration Tests
working-directory: ./tmp/pa
run: ./gradlew integTest -Dtests.enableIT -Dtests.useDockerCluster
- name: Generate Jacoco coverage report
working-directory: ./tmp/pa
run: ./gradlew jacocoTestReport
Expand All @@ -59,3 +56,6 @@ jobs:
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
run: bash <(curl -s https://codecov.io/bash) -f ./build/reports/jacoco/test/jacocoTestReport.xml
- name: Run Integration Tests
working-directory: ./tmp/pa
run: ./gradlew integTest -Dtests.enableIT -Dtests.useDockerCluster
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ jacocoTestCoverageVerification {
violationRules {
rule {
limit {
minimum = 0.19
minimum = 0.31
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ static class CacheMaxSizeStatus extends MetricStatus {
private final String cacheType;

@JsonInclude(Include.NON_NULL)
private final long cacheMaxSize;
private final Long cacheMaxSize;

CacheMaxSizeStatus(String cacheType, Long cacheMaxSize) {
this.cacheType = cacheType;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Missing license header.


import com.amazon.opendistro.elasticsearch.performanceanalyzer.ESResources;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors.CacheConfigMetricsCollector.CacheMaxSizeStatus;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics.CacheType;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.MetricsConfiguration;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.reader_writer_shared.Event;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.util.TestUtil;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.paranamer.ParanamerModule;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.test.ESSingleNodeTestCase;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

public class CacheConfigMetricsCollectorTests extends ESSingleNodeTestCase {
private static final String TEST_INDEX = "test";
private CacheConfigMetricsCollector collector;

@Before
public void init() {
IndicesService indicesService = getInstanceFromNode(IndicesService.class);
ESResources.INSTANCE.setIndicesService(indicesService);

MetricsConfiguration.CONFIG_MAP.put(CacheConfigMetricsCollector.class, MetricsConfiguration.cdefault);
collector = new CacheConfigMetricsCollector();
}

@After
public void tearDown() throws Exception {
super.tearDown();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add saveMetricsValues test here as well?

@Test
public void testCollectMetrics() throws IOException {
long startTimeInMills = 1153721339;

createIndex(TEST_INDEX);
collector.collectMetrics(startTimeInMills);

List<CacheMaxSizeStatus> metrics = readMetrics();
assertEquals(2, metrics.size());
CacheMaxSizeStatus filedDataCache = metrics.get(0);
CacheMaxSizeStatus shardRequestCache = metrics.get(1);
assertEquals(CacheType.FIELD_DATA_CACHE.toString(), filedDataCache.getCacheType());
assertEquals(CacheType.SHARD_REQUEST_CACHE.toString(), shardRequestCache.getCacheType());
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we assert values for Cache Sizes as well?

}

private List<CacheMaxSizeStatus> readMetrics() throws IOException {
List<Event> metrics = TestUtil.readEvents();
assert metrics.size() == 1;
ObjectMapper objectMapper = new ObjectMapper().registerModule(new ParanamerModule());

List<CacheMaxSizeStatus> list = new ArrayList<>();
String[] jsonStrs = metrics.get(0).value.split("\n");
assert jsonStrs.length == 3;
for (int i = 1; i < 3; i++) {
list.add(objectMapper.readValue(jsonStrs[i], CacheMaxSizeStatus.class));
}
return list;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: missing license header


import com.amazon.opendistro.elasticsearch.performanceanalyzer.ESResources;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors.NodeDetailsCollector.NodeDetailsStatus;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.overrides.ConfigOverridesWrapper;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics.NodeRole;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.MetricsConfiguration;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.reader_writer_shared.Event;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.util.TestUtil;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.paranamer.ParanamerModule;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.test.ClusterServiceUtils;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.TestThreadPool;
import org.elasticsearch.threadpool.ThreadPool;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;

public class NodeDetailsCollectorTests extends ESTestCase {
private static final String NODE_ID = "testNode";
private NodeDetailsCollector collector;
private ConfigOverridesWrapper configOverrides;
private ThreadPool threadPool;

@Before
public void init() {
DiscoveryNode testNode = new DiscoveryNode(NODE_ID, ESTestCase.buildNewFakeTransportAddress(), Collections
.emptyMap(),
DiscoveryNodeRole.BUILT_IN_ROLES, Version.CURRENT);

threadPool = new TestThreadPool("test");
ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool, testNode);
ESResources.INSTANCE.setClusterService(clusterService);

MetricsConfiguration.CONFIG_MAP.put(NodeDetailsCollector.class, MetricsConfiguration.cdefault);
configOverrides = Mockito.mock(ConfigOverridesWrapper.class);
yu-sun-77 marked this conversation as resolved.
Show resolved Hide resolved
collector = new NodeDetailsCollector(configOverrides);
}

@After
public void tearDown() throws Exception {
threadPool.shutdownNow();
super.tearDown();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add saveMetricsValues test here as well?

@Test
public void testCollectMetrics() throws IOException {
long startTimeInMills = 1153721339;
collector.collectMetrics(startTimeInMills);
NodeDetailsStatus nodeDetailsStatus = readMetrics();

assertEquals(NODE_ID, nodeDetailsStatus.getID());
assertEquals("0.0.0.0", nodeDetailsStatus.getHostAddress());
assertEquals(NodeRole.DATA.role(), nodeDetailsStatus.getRole());
assertTrue(nodeDetailsStatus.getIsMasterNode());
}

private NodeDetailsStatus readMetrics() throws IOException {
List<Event> metrics = TestUtil.readEvents();
assert metrics.size() == 1;
ObjectMapper objectMapper = new ObjectMapper().registerModule(new ParanamerModule());
String[] jsonStrs = metrics.get(0).value.split("\n");
assert jsonStrs.length == 4;
return objectMapper.readValue(jsonStrs[3], NodeDetailsStatus.class);
}
}
yu-sun-77 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -15,64 +15,93 @@

package com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors;

import org.junit.Ignore;
import org.junit.Test;

import com.amazon.opendistro.elasticsearch.performanceanalyzer.CustomMetricsLocationTestBase;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.PluginSettings;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.ESResources;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.PerformanceAnalyzerController;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics.ShardStatsValue;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.MetricsConfiguration;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.PerformanceAnalyzerMetrics;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

@Ignore
public class NodeStatsFixedShardsMetricsCollectorTests extends CustomMetricsLocationTestBase {

@Test
public void testNodeStatsMetrics() {
System.setProperty("performanceanalyzer.metrics.log.enabled", "False");
long startTimeInMills = 1253722339;

MetricsConfiguration.CONFIG_MAP.put(NodeStatsFixedShardsMetricsCollector.class, MetricsConfiguration.cdefault);

NodeStatsFixedShardsMetricsCollector nodeStatsFixedShardsMetricsCollector = new NodeStatsFixedShardsMetricsCollector(null);
nodeStatsFixedShardsMetricsCollector.saveMetricValues("89123.23", startTimeInMills, "NodesStatsIndex", "55");

import com.amazon.opendistro.elasticsearch.performanceanalyzer.reader_writer_shared.Event;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.util.TestUtil;
import java.util.List;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.test.ESSingleNodeTestCase;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;

String fetchedValue = PerformanceAnalyzerMetrics.getMetric(
PluginSettings.instance().getMetricsLocation()
+ PerformanceAnalyzerMetrics.getTimeInterval(startTimeInMills)+"/indices/NodesStatsIndex/55/");
PerformanceAnalyzerMetrics.removeMetrics(PluginSettings.instance().getMetricsLocation()
+ PerformanceAnalyzerMetrics.getTimeInterval(startTimeInMills));
assertEquals("89123.23", fetchedValue);
public class NodeStatsFixedShardsMetricsCollectorTests extends ESSingleNodeTestCase {
private static final String TEST_INDEX = "test";
private NodeStatsFixedShardsMetricsCollector collector;
private PerformanceAnalyzerController controller;
private long startTimeInMills = 1153721339;

@Before
public void init() {
IndicesService indicesService = getInstanceFromNode(IndicesService.class);
ESResources.INSTANCE.setIndicesService(indicesService);

try {
nodeStatsFixedShardsMetricsCollector.saveMetricValues("89123.23", startTimeInMills, "NodesStatsIndex");
assertTrue("Negative scenario test: Should have been a RuntimeException", true);
} catch (RuntimeException ex) {
//- expecting exception...only 1 values passed; 2 expected
}
MetricsConfiguration.CONFIG_MAP.put(NodeStatsAllShardsMetricsCollector.class, MetricsConfiguration.cdefault);
controller = Mockito.mock(PerformanceAnalyzerController.class);
yu-sun-77 marked this conversation as resolved.
Show resolved Hide resolved
collector = new NodeStatsFixedShardsMetricsCollector(controller);
}

try {
nodeStatsFixedShardsMetricsCollector.saveMetricValues("89123.23", startTimeInMills);
assertTrue("Negative scenario test: Should have been a RuntimeException", true);
} catch (RuntimeException ex) {
//- expecting exception...only 0 values passed; 2 expected
}
@After
public void tearDown() throws Exception {
super.tearDown();
}

@Test
public void testNodeStatsMetrics() {
try {
nodeStatsFixedShardsMetricsCollector.saveMetricValues("89123.23", startTimeInMills, "NodesStatsIndex", "55", "123");
assertTrue("Negative scenario test: Should have been a RuntimeException", true);
} catch (RuntimeException ex) {
//- expecting exception...only 3 values passed; 2 expected
yu-sun-77 marked this conversation as resolved.
Show resolved Hide resolved
collector.getNodeIndicesStatsByShardField();
} catch (Exception exception) {
assertTrue("There shouldn't be any exception in the code; Please check the reflection code for any changes", true);
}

collector = new NodeStatsFixedShardsMetricsCollector(null);
try {
nodeStatsFixedShardsMetricsCollector.getNodeIndicesStatsByShardField();
collector.collectMetrics(startTimeInMills);
} catch (Exception exception) {
assertTrue("There shouldn't be any exception in the code; Please check the reflection code for any changes", true);
}
}

@Test
public void testCollectMetrics() {
createIndex(TEST_INDEX);
Mockito.when(controller.getNodeStatsShardsPerCollection()).thenReturn(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a test to check if all the metrics for all the shards are populated in subsequent runs of this collector?

collector.collectMetrics(startTimeInMills);

//cannot make NodeStatsMetricsFixedShardsPerCollectionStatus static to deserialize it, so check with jsonString
String jsonStr = readMetricsInJsonString();
assertTrue(jsonStr.contains(ShardStatsValue.Constants.INDEXING_THROTTLE_TIME_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.REFRESH_COUNT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.REFRESH_TIME_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.FLUSH_COUNT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.FLUSH_TIME_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.MERGE_COUNT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.MERGE_TIME_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.MERGE_CURRENT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.INDEX_BUFFER_BYTES_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.SEGMENTS_COUNT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.SEGMENTS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.TERMS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.STORED_FIELDS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.TERM_VECTOR_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.NORMS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.POINTS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.DOC_VALUES_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.INDEX_WRITER_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.VERSION_MAP_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.BITSET_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.SHARD_SIZE_IN_BYTES_VALUE));
}

private String readMetricsInJsonString() {
List<Event> metrics = TestUtil.readEvents();
assert metrics.size() == 1;
String[] jsonStrs = metrics.get(0).value.split("\n");
assert jsonStrs.length == 2;
return jsonStrs[1];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.threadpool.ThreadPoolStats;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.Mockito;

@Ignore
public class ThreadPoolMetricsCollectorTests extends CustomMetricsLocationTestBase {

private ThreadPoolMetricsCollector threadPoolMetricsCollector;
Expand Down