Skip to content

Commit

Permalink
Otel counters for error metrics (#124) (#127)
Browse files Browse the repository at this point in the history
(cherry picked from commit 38e84a2)

Signed-off-by: Chenyang Ji <cyji@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 929141c commit 71b84c4
Show file tree
Hide file tree
Showing 14 changed files with 262 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.opensearch.env.Environment;
import org.opensearch.env.NodeEnvironment;
import org.opensearch.plugin.insights.core.listener.QueryInsightsListener;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
import org.opensearch.plugin.insights.core.service.QueryInsightsService;
import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction;
import org.opensearch.plugin.insights.rules.resthandler.top_queries.RestTopQueriesAction;
Expand Down Expand Up @@ -74,6 +75,8 @@ public Collection<Object> createComponents(
final Tracer tracer,
final MetricsRegistry metricsRegistry
) {
// initialize operational metrics counters
OperationalMetricsCounter.initialize(clusterService.getClusterName().toString(), metricsRegistry);
// create top n queries service
final QueryInsightsService queryInsightsService = new QueryInsightsService(
clusterService.getClusterSettings(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.plugin.insights.core.metrics.OperationalMetric;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
import org.opensearch.plugin.insights.rules.model.SearchQueryRecord;

/**
Expand Down Expand Up @@ -90,10 +92,12 @@ public void onResponse(BulkResponse bulkItemResponses) {}

@Override
public void onFailure(Exception e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.LOCAL_INDEX_EXPORTER_BULK_FAILURES);
logger.error("Failed to execute bulk operation for query insights data: ", e);
}
});
} catch (final Exception e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.LOCAL_INDEX_EXPORTER_EXCEPTIONS);
logger.error("Unable to index query insights data: ", e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.joda.time.format.DateTimeFormat;
import org.opensearch.client.Client;
import org.opensearch.common.settings.Settings;
import org.opensearch.plugin.insights.core.metrics.OperationalMetric;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;

/**
* Factory class for validating and creating exporters based on provided settings
Expand Down Expand Up @@ -59,6 +61,7 @@ public void validateExporterConfig(final Settings settings) throws IllegalArgume
try {
type = SinkType.parse(settings.get(EXPORTER_TYPE, DEFAULT_TOP_QUERIES_EXPORTER_TYPE));
} catch (IllegalArgumentException e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.INVALID_EXPORTER_TYPE_FAILURES);
throw new IllegalArgumentException(
String.format(
Locale.ROOT,
Expand All @@ -77,6 +80,7 @@ public void validateExporterConfig(final Settings settings) throws IllegalArgume
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 @@ -32,6 +32,8 @@
import org.opensearch.common.inject.Inject;
import org.opensearch.core.tasks.resourcetracker.TaskResourceInfo;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.plugin.insights.core.metrics.OperationalMetric;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
import org.opensearch.plugin.insights.core.service.QueryInsightsService;
import org.opensearch.plugin.insights.core.service.categorizer.QueryShapeGenerator;
import org.opensearch.plugin.insights.rules.model.Attribute;
Expand Down Expand Up @@ -261,6 +263,7 @@ private void constructSearchQueryRecord(final SearchPhaseContext context, final
SearchQueryRecord record = new SearchQueryRecord(request.getOrCreateAbsoluteStartMillis(), measurements, attributes);
queryInsightsService.addRecord(record);
} catch (Exception e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.DATA_INGEST_EXCEPTIONS);
log.error(String.format(Locale.ROOT, "fail to ingest query insight data, error: %s", e));
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

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

import java.util.Locale;

public enum OperationalMetric {
LOCAL_INDEX_READER_PARSING_EXCEPTIONS("Number of errors when parsing with LocalIndexReader"),
LOCAL_INDEX_EXPORTER_BULK_FAILURES("Number of failures when ingesting Query Insights data to local indices"),
LOCAL_INDEX_EXPORTER_EXCEPTIONS("Number of exceptions in Query Insights LocalIndexExporter"),
INVALID_EXPORTER_TYPE_FAILURES("Number of invalid exporter type failures"),
INVALID_INDEX_PATTERN_EXCEPTIONS("Number of invalid index pattern exceptions"),
DATA_INGEST_EXCEPTIONS("Number of exceptions during data ingest in Query Insights"),
QUERY_CATEGORIZE_EXCEPTIONS("Number of exceptions when categorizing the queries"),
EXPORTER_FAIL_TO_CLOSE_EXCEPTION("Number of failures when closing the exporter");

private final String description;

OperationalMetric(String description) {
this.description = description;
}

public String getDescription() {
return description;
}

@Override
public String toString() {
return String.format(Locale.ROOT, "%s (%s)", name(), description);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

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

import java.util.Locale;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Stream;
import org.opensearch.telemetry.metrics.Counter;
import org.opensearch.telemetry.metrics.MetricsRegistry;
import org.opensearch.telemetry.metrics.tags.Tags;

/**
* Class contains all the Counters related to search query types.
*/
public final class OperationalMetricsCounter {
private static final String PREFIX = "search.insights.";
private static final String CLUSTER_NAME_TAG = "cluster_name";
private static final String UNIT = "1";

private final String clusterName;
private final MetricsRegistry metricsRegistry;
private final ConcurrentHashMap<OperationalMetric, Counter> metricCounterMap;

private static OperationalMetricsCounter instance;

/**
* Constructor of OperationalMetricsCounter
* @param metricsRegistry the OTel metrics registry
*/
private OperationalMetricsCounter(String clusterName, MetricsRegistry metricsRegistry) {
this.clusterName = clusterName;
this.metricsRegistry = metricsRegistry;
this.metricCounterMap = new ConcurrentHashMap<>();
Stream.of(OperationalMetric.values()).forEach(name -> metricCounterMap.computeIfAbsent(name, this::createMetricCounter));
}

/**
* Initializes the singleton instance of OperationalMetricsCounter.
* This method must be called once before accessing the instance.
*
* @param clusterName the name of the cluster
* @param metricsRegistry the OTel metrics registry
*/
public static synchronized void initialize(String clusterName, MetricsRegistry metricsRegistry) {
instance = new OperationalMetricsCounter(clusterName, metricsRegistry);
}

/**
* Get the singleton instance of OperationalMetricsCounter.
*
* @return the singleton instance
* @throws IllegalStateException if the instance is not yet initialized
*/
public static synchronized OperationalMetricsCounter getInstance() {
if (instance == null) {
throw new IllegalStateException("OperationalMetricsCounter is not initialized. Call initialize() first.");
}
return instance;
}

/**
* Increment the operational metrics counter, attaching custom tags
*
* @param metricName name of the metric
* @param customTags custom tags of this metric
*/
public void incrementCounter(OperationalMetric metricName, Tags customTags) {
Counter counter = metricCounterMap.computeIfAbsent(metricName, this::createMetricCounter);
Tags metricsTags = (customTags == null ? Tags.create() : customTags).addTag(CLUSTER_NAME_TAG, clusterName);
counter.add(1, metricsTags);
}

/**
* Increment the operational metrics counter
*
* @param metricName name of the metric
*/
public void incrementCounter(OperationalMetric metricName) {
this.incrementCounter(metricName, null);
}

private Counter createMetricCounter(OperationalMetric metricName) {
return metricsRegistry.createCounter(
PREFIX + metricName.toString().toLowerCase(Locale.ROOT) + ".count",
metricName.getDescription(),
UNIT
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import org.opensearch.index.query.QueryBuilder;
import org.opensearch.index.query.QueryBuilders;
import org.opensearch.index.query.RangeQueryBuilder;
import org.opensearch.plugin.insights.core.metrics.OperationalMetric;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
import org.opensearch.plugin.insights.rules.model.SearchQueryRecord;
import org.opensearch.search.SearchHit;
import org.opensearch.search.builder.SearchSourceBuilder;
Expand Down Expand Up @@ -113,6 +115,7 @@ public List<SearchQueryRecord> read(final String from, final String to) {
records.add(record);
}
} catch (IndexNotFoundException ignored) {} catch (Exception e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.LOCAL_INDEX_READER_PARSING_EXCEPTIONS);
logger.error("Unable to parse search hit: ", e);
}
curr = curr.plusDays(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
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 Down Expand Up @@ -57,6 +59,7 @@ public void validateReaderConfig(final Settings settings) throws IllegalArgument
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)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterFactory;
import org.opensearch.plugin.insights.core.metrics.OperationalMetric;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
import org.opensearch.plugin.insights.core.reader.QueryInsightsReaderFactory;
import org.opensearch.plugin.insights.core.service.categorizer.SearchQueryCategorizer;
import org.opensearch.plugin.insights.rules.model.GroupingType;
Expand Down Expand Up @@ -188,6 +190,7 @@ public void drainRecords() {
try {
searchQueryCategorizer.consumeRecords(records);
} catch (Exception e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.QUERY_CATEGORIZE_EXCEPTIONS);
logger.error("Error while trying to categorize the queries.", e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporter;
import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterFactory;
import org.opensearch.plugin.insights.core.exporter.SinkType;
import org.opensearch.plugin.insights.core.metrics.OperationalMetric;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
import org.opensearch.plugin.insights.core.reader.QueryInsightsReader;
import org.opensearch.plugin.insights.core.reader.QueryInsightsReaderFactory;
import org.opensearch.plugin.insights.core.service.grouper.MinMaxHeapQueryGrouper;
Expand Down Expand Up @@ -265,6 +267,7 @@ public void setExporter(final Settings settings) {
try {
queryInsightsExporterFactory.closeExporter(this.exporter);
} catch (IOException e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.EXPORTER_FAIL_TO_CLOSE_EXCEPTION);
logger.error("Fail to close the current exporter when updating exporter, error: ", e);
}
this.exporter = queryInsightsExporterFactory.createExporter(
Expand All @@ -278,6 +281,7 @@ public void setExporter(final Settings settings) {
queryInsightsExporterFactory.closeExporter(this.exporter);
this.exporter = null;
} catch (IOException e) {
OperationalMetricsCounter.getInstance().incrementCounter(OperationalMetric.EXPORTER_FAIL_TO_CLOSE_EXCEPTION);
logger.error("Fail to close the current exporter when disabling exporter, error: ", e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@

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

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;
Expand All @@ -17,6 +19,9 @@
import org.junit.Before;
import org.opensearch.client.Client;
import org.opensearch.common.settings.Settings;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
import org.opensearch.telemetry.metrics.Counter;
import org.opensearch.telemetry.metrics.MetricsRegistry;
import org.opensearch.test.OpenSearchTestCase;

/**
Expand All @@ -27,10 +32,16 @@ public class QueryInsightsExporterFactoryTests extends OpenSearchTestCase {

private final Client client = mock(Client.class);
private QueryInsightsExporterFactory queryInsightsExporterFactory;
private MetricsRegistry metricsRegistry;

@Before
public void setup() {
queryInsightsExporterFactory = new QueryInsightsExporterFactory(client);
metricsRegistry = mock(MetricsRegistry.class);
when(metricsRegistry.createCounter(any(String.class), any(String.class), any(String.class))).thenAnswer(
invocation -> mock(Counter.class)
);
OperationalMetricsCounter.initialize("cluster", metricsRegistry);
}

public void testValidateConfigWhenResetExporter() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

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

import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import org.mockito.ArgumentCaptor;
import org.opensearch.telemetry.metrics.Counter;
import org.opensearch.telemetry.metrics.MetricsRegistry;
import org.opensearch.telemetry.metrics.tags.Tags;
import org.opensearch.test.OpenSearchTestCase;

/**
* Unit tests for the {@link OperationalMetricsCounter} class.
*/
public class OperationalMetricsCounterTests extends OpenSearchTestCase {
private static final String CLUSTER_NAME = "test-cluster";

public void testSingletonInitializationAndIncrement() {
Counter mockCounter = mock(Counter.class);
MetricsRegistry metricsRegistry = mock(MetricsRegistry.class);
// Stub the createCounter method to return the mockCounter
when(metricsRegistry.createCounter(any(), any(), any())).thenReturn(mockCounter);
OperationalMetricsCounter.initialize(CLUSTER_NAME, metricsRegistry);
OperationalMetricsCounter instance = OperationalMetricsCounter.getInstance();
ArgumentCaptor<String> nameCaptor = ArgumentCaptor.forClass(String.class);
verify(metricsRegistry, times(8)).createCounter(nameCaptor.capture(), any(), eq("1"));
assertNotNull(instance);
instance.incrementCounter(OperationalMetric.LOCAL_INDEX_READER_PARSING_EXCEPTIONS);
instance.incrementCounter(OperationalMetric.LOCAL_INDEX_READER_PARSING_EXCEPTIONS);
instance.incrementCounter(OperationalMetric.LOCAL_INDEX_READER_PARSING_EXCEPTIONS);
verify(mockCounter, times(3)).add(eq(1.0), any(Tags.class));
}
}
Loading

0 comments on commit 71b84c4

Please sign in to comment.