Skip to content

Commit

Permalink
Remove local index custom name setting (#166)
Browse files Browse the repository at this point in the history
Signed-off-by: David Zane <davizane@amazon.com>
(cherry picked from commit a97099c)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] committed Dec 11, 2024
1 parent 93405a9 commit 016e80d
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@

package org.opensearch.plugin.insights.core.exporter;

import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_QUERIES_INDEX_PATTERN;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_QUERIES_EXPORTER_TYPE;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORTER_TYPE;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORT_INDEX;

import java.io.IOException;
import java.util.HashSet;
Expand Down Expand Up @@ -71,21 +69,6 @@ public void validateExporterConfig(final Settings settings) throws IllegalArgume
)
);
}
switch (type) {
case LOCAL_INDEX:
final String indexPattern = settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN);
if (indexPattern.length() == 0) {
throw new IllegalArgumentException("Empty index pattern configured for the exporter");
}
try {
DateTimeFormat.forPattern(indexPattern);
} catch (Exception e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.INVALID_INDEX_PATTERN_EXCEPTIONS);
throw new IllegalArgumentException(
String.format(Locale.ROOT, "Invalid index pattern [%s] configured for the exporter", indexPattern)
);
}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,14 @@

package org.opensearch.plugin.insights.core.reader;

import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_QUERIES_INDEX_PATTERN;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORT_INDEX;

import java.io.IOException;
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.joda.time.format.DateTimeFormat;
import org.opensearch.client.Client;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.plugin.insights.core.metrics.OperationalMetric;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;

/**
* Factory class for validating and creating Readers based on provided settings
Expand All @@ -45,27 +38,6 @@ public QueryInsightsReaderFactory(final Client client) {
this.Readers = new HashSet<>();
}

/**
* Validate Reader sink config
*
* @param settings Reader sink config {@link Settings}
* @throws IllegalArgumentException if provided Reader sink config settings are invalid
*/
public void validateReaderConfig(final Settings settings) throws IllegalArgumentException {
final String indexPattern = settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN);
if (indexPattern.isEmpty()) {
throw new IllegalArgumentException("Empty index pattern configured for the Reader");
}
try {
DateTimeFormat.forPattern(indexPattern);
} catch (Exception e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.INVALID_INDEX_PATTERN_EXCEPTIONS);
throw new IllegalArgumentException(
String.format(Locale.ROOT, "Invalid index pattern [%s] configured for the Reader", indexPattern)
);
}
}

/**
* Create a Reader based on provided parameters
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_QUERIES_INDEX_PATTERN;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_QUERIES_EXPORTER_TYPE;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORTER_TYPE;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORT_INDEX;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.QUERY_INSIGHTS_EXECUTOR;

import java.io.IOException;
Expand Down Expand Up @@ -262,7 +261,7 @@ public void setExporter(final Settings settings) {
if (settings.get(EXPORTER_TYPE) != null) {
SinkType expectedType = SinkType.parse(settings.get(EXPORTER_TYPE, DEFAULT_TOP_QUERIES_EXPORTER_TYPE));
if (exporter != null && expectedType == SinkType.getSinkTypeFromExporter(exporter)) {
queryInsightsExporterFactory.updateExporter(exporter, settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN));
queryInsightsExporterFactory.updateExporter(exporter, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN);
} else {
try {
queryInsightsExporterFactory.closeExporter(this.exporter);
Expand All @@ -272,7 +271,7 @@ public void setExporter(final Settings settings) {
}
this.exporter = queryInsightsExporterFactory.createExporter(
SinkType.parse(settings.get(EXPORTER_TYPE, DEFAULT_TOP_QUERIES_EXPORTER_TYPE)),
settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN)
DEFAULT_TOP_N_QUERIES_INDEX_PATTERN
);
}
} else {
Expand All @@ -294,11 +293,8 @@ public void setExporter(final Settings settings) {
* @param namedXContentRegistry NamedXContentRegistry for parsing purposes
*/
public void setReader(final Settings settings, final NamedXContentRegistry namedXContentRegistry) {
this.reader = queryInsightsReaderFactory.createReader(
settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN),
namedXContentRegistry
);
queryInsightsReaderFactory.updateReader(reader, settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN));
this.reader = queryInsightsReaderFactory.createReader(DEFAULT_TOP_N_QUERIES_INDEX_PATTERN, namedXContentRegistry);
queryInsightsReaderFactory.updateReader(reader, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN);
}

/**
Expand All @@ -308,7 +304,6 @@ public void setReader(final Settings settings, final NamedXContentRegistry named
*/
public void validateExporterAndReaderConfig(Settings settings) {
queryInsightsExporterFactory.validateExporterConfig(settings);
queryInsightsReaderFactory.validateReaderConfig(settings);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,6 @@ public class QueryInsightsSettings {
* Config key for exporter type
*/
public static final String EXPORTER_TYPE = "type";
/**
* Config key for export index
*/
public static final String EXPORT_INDEX = "config.index";
/**
* Settings and defaults for top queries exporters
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_QUERIES_EXPORTER_TYPE;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORTER_TYPE;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORT_INDEX;

import org.joda.time.format.DateTimeFormat;
import org.junit.Before;
Expand Down Expand Up @@ -61,20 +59,6 @@ public void testInvalidExporterTypeConfig() {
assertThrows(IllegalArgumentException.class, () -> { queryInsightsExporterFactory.validateExporterConfig(settings); });
}

public void testInvalidLocalIndexConfig() {
Settings.Builder settingsBuilder = Settings.builder();
assertThrows(IllegalArgumentException.class, () -> {
queryInsightsExporterFactory.validateExporterConfig(
settingsBuilder.put(EXPORTER_TYPE, DEFAULT_TOP_QUERIES_EXPORTER_TYPE).put(EXPORT_INDEX, "").build()
);
});
assertThrows(IllegalArgumentException.class, () -> {
queryInsightsExporterFactory.validateExporterConfig(
settingsBuilder.put(EXPORTER_TYPE, DEFAULT_TOP_QUERIES_EXPORTER_TYPE).put(EXPORT_INDEX, "some_invalid_pattern").build()
);
});
}

public void testCreateAndCloseExporter() {
QueryInsightsExporter exporter1 = queryInsightsExporterFactory.createExporter(SinkType.LOCAL_INDEX, format);
assertTrue(exporter1 instanceof LocalIndexExporter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,10 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_QUERIES_INDEX_PATTERN;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORT_INDEX;

import org.joda.time.format.DateTimeFormat;
import org.junit.Before;
import org.opensearch.client.Client;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
import org.opensearch.telemetry.metrics.Counter;
Expand Down Expand Up @@ -45,22 +43,6 @@ public void setup() {
OperationalMetricsCounter.initialize("cluster", metricsRegistry);
}

public void testValidateConfigWhenResetReader() {
Settings.Builder settingsBuilder = Settings.builder();
Settings settings = settingsBuilder.build();
try {
queryInsightsReaderFactory.validateReaderConfig(settings);
} catch (Exception e) {
fail("No exception should be thrown when setting is null");
}
}

public void testInvalidReaderTypeConfig() {
Settings.Builder settingsBuilder = Settings.builder();
Settings settings = settingsBuilder.put(EXPORT_INDEX, "some_invalid_type").build();
assertThrows(IllegalArgumentException.class, () -> { queryInsightsReaderFactory.validateReaderConfig(settings); });
}

public void testCreateAndCloseReader() {
QueryInsightsReader reader1 = queryInsightsReaderFactory.createReader(format, namedXContentRegistry);
assertTrue(reader1 instanceof LocalIndexReader);
Expand Down

0 comments on commit 016e80d

Please sign in to comment.