Skip to content
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.

Sign up for GitHub

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

[Backport 2.x] Remove local index custom name setting #167

Merged
merged 1 commit into from
Dec 12, 2024
Merged

[Backport 2.x] Remove local index custom name setting #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 0 additions & 17 deletions src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactory.java
View file
Open in desktop
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
28 changes: 0 additions & 28 deletions src/main/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderFactory.java
View file
Open in desktop
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
13 changes: 4 additions & 9 deletions src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java
View file
Open in desktop
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
4 changes: 0 additions & 4 deletions src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java
View file
Open in desktop
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
16 changes: 0 additions & 16 deletions .../java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactoryTests.java
View file
Open in desktop
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
18 changes: 0 additions & 18 deletions ...test/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderFactoryTests.java
View file
Open in desktop
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
Loading