From 6bea8515b2169020009f04e6e3281809c8e180d3 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Fri, 23 Aug 2024 13:09:03 -0700 Subject: [PATCH 1/4] Query instrumentation minor fixes Signed-off-by: Siddhant Deshmukh --- .../SearchQueryAggregationCategorizer.java | 6 +++--- .../service/categorizer/SearchQueryCounters.java | 14 +++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java b/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java index e0b2ab1..3bfd942 100644 --- a/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java +++ b/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java @@ -20,7 +20,7 @@ */ public class SearchQueryAggregationCategorizer { - private static final String TYPE_TAG = "type"; + private static final String AGGREGATION_TYPE_TAG = "agg_type"; private final SearchQueryCounters searchQueryCounters; /** @@ -49,7 +49,7 @@ public void incrementSearchQueryAggregationCounters( private void incrementCountersRecursively(AggregationBuilder aggregationBuilder, Map measurements) { // Increment counters for the current aggregation String aggregationType = aggregationBuilder.getType(); - searchQueryCounters.incrementAggCounter(1, Tags.create().addTag(TYPE_TAG, aggregationType), measurements); + searchQueryCounters.incrementAggCounter(1, Tags.create().addTag(AGGREGATION_TYPE_TAG, aggregationType), measurements); // Recursively process sub-aggregations if any Collection subAggregations = aggregationBuilder.getSubAggregations(); @@ -63,7 +63,7 @@ private void incrementCountersRecursively(AggregationBuilder aggregationBuilder, Collection pipelineAggregations = aggregationBuilder.getPipelineAggregations(); for (PipelineAggregationBuilder pipelineAggregation : pipelineAggregations) { String pipelineAggregationType = pipelineAggregation.getType(); - searchQueryCounters.incrementAggCounter(1, Tags.create().addTag(TYPE_TAG, pipelineAggregationType), measurements); + searchQueryCounters.incrementAggCounter(1, Tags.create().addTag(AGGREGATION_TYPE_TAG, pipelineAggregationType), measurements); } } } diff --git a/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java b/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java index 6578d6d..cb89022 100644 --- a/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java +++ b/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java @@ -23,8 +23,12 @@ */ public final class SearchQueryCounters { private static final String LEVEL_TAG = "level"; - private static final String TYPE_TAG = "type"; + private static final String QUERY_TYPE_TAG = "type"; private static final String UNIT = "1"; + private static final String UNIT_MILLIS = "ms"; + private static final String UNIT_CPU_CYCLES = "ns"; + private static final String UNIT_BYTES = "bytes"; + private final MetricsRegistry metricsRegistry; /** * Aggregation counter @@ -83,17 +87,17 @@ public SearchQueryCounters(MetricsRegistry metricsRegistry) { this.queryTypeLatencyHistogram = metricsRegistry.createHistogram( "search.query.type.latency.histogram", "Histogram for the latency per query type", - UNIT + UNIT_MILLIS ); this.queryTypeCpuHistogram = metricsRegistry.createHistogram( "search.query.type.cpu.histogram", "Histogram for the cpu per query type", - UNIT + UNIT_CPU_CYCLES ); this.queryTypeMemoryHistogram = metricsRegistry.createHistogram( "search.query.type.memory.histogram", "Histogram for the memory per query type", - UNIT + UNIT_BYTES ); this.queryHandlers = new HashMap<>(); } @@ -109,7 +113,7 @@ public void incrementCounter(QueryBuilder queryBuilder, int level, Map createQueryCounter(k)); counter.add(1, Tags.create().addTag(LEVEL_TAG, level)); - incrementAllHistograms(Tags.create().addTag(LEVEL_TAG, level).addTag(TYPE_TAG, uniqueQueryCounterName), measurements); + incrementAllHistograms(Tags.create().addTag(LEVEL_TAG, level).addTag(QUERY_TYPE_TAG, uniqueQueryCounterName), measurements); } /** From 9325057bd10da4f42d43aef7eabf6fbbc3959f68 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Fri, 23 Aug 2024 13:46:34 -0700 Subject: [PATCH 2/4] Fix unit tests Signed-off-by: Siddhant Deshmukh --- .../core/service/categorizor/SearchQueryCategorizerTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java b/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java index f131616..29ddb77 100644 --- a/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java +++ b/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java @@ -114,7 +114,7 @@ public void testAggregationsQuery() { verify(searchQueryCategorizer.getSearchQueryCounters().getAggCounter()).add(valueCaptor.capture(), tagsCaptor.capture()); double actualValue = valueCaptor.getValue(); - String actualTag = (String) tagsCaptor.getValue().getTagsMap().get("type"); + String actualTag = (String) tagsCaptor.getValue().getTagsMap().get("agg_type"); assertEquals(1.0d, actualValue, 0.0001); assertEquals(MULTI_TERMS_AGGREGATION, actualTag); From bff828549b57f028d895c4d5624ea90545143a2e Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 27 Aug 2024 11:52:59 -0700 Subject: [PATCH 3/4] Rename package Signed-off-by: Siddhant Deshmukh --- .../categorizer/SearchQueryAggregationCategorizer.java | 2 +- .../QueryShapeGeneratorTests.java | 3 +-- .../{categorizor => categorizer}/QueryShapeVisitorTests.java | 3 +-- .../SearchQueryCategorizerTests.java | 5 +++-- 4 files changed, 6 insertions(+), 7 deletions(-) rename src/test/java/org/opensearch/plugin/insights/core/service/{categorizor => categorizer}/QueryShapeGeneratorTests.java (98%) rename src/test/java/org/opensearch/plugin/insights/core/service/{categorizor => categorizer}/QueryShapeVisitorTests.java (92%) rename src/test/java/org/opensearch/plugin/insights/core/service/{categorizor => categorizer}/SearchQueryCategorizerTests.java (98%) diff --git a/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java b/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java index 3bfd942..7ed861f 100644 --- a/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java +++ b/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java @@ -20,7 +20,7 @@ */ public class SearchQueryAggregationCategorizer { - private static final String AGGREGATION_TYPE_TAG = "agg_type"; + static final String AGGREGATION_TYPE_TAG = "agg_type"; private final SearchQueryCounters searchQueryCounters; /** diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/QueryShapeGeneratorTests.java b/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeGeneratorTests.java similarity index 98% rename from src/test/java/org/opensearch/plugin/insights/core/service/categorizor/QueryShapeGeneratorTests.java rename to src/test/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeGeneratorTests.java index 5e04fa9..5b4d251 100644 --- a/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/QueryShapeGeneratorTests.java +++ b/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeGeneratorTests.java @@ -6,11 +6,10 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.service.categorizor; +package org.opensearch.plugin.insights.core.service.categorizer; import org.opensearch.common.hash.MurmurHash3; import org.opensearch.plugin.insights.SearchSourceBuilderUtils; -import org.opensearch.plugin.insights.core.service.categorizer.QueryShapeGenerator; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.test.OpenSearchTestCase; diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/QueryShapeVisitorTests.java b/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeVisitorTests.java similarity index 92% rename from src/test/java/org/opensearch/plugin/insights/core/service/categorizor/QueryShapeVisitorTests.java rename to src/test/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeVisitorTests.java index bffe287..298f4f8 100644 --- a/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/QueryShapeVisitorTests.java +++ b/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeVisitorTests.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.service.categorizor; +package org.opensearch.plugin.insights.core.service.categorizer; import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.ConstantScoreQueryBuilder; @@ -16,7 +16,6 @@ import org.opensearch.index.query.RegexpQueryBuilder; import org.opensearch.index.query.TermQueryBuilder; import org.opensearch.index.query.TermsQueryBuilder; -import org.opensearch.plugin.insights.core.service.categorizer.QueryShapeVisitor; import org.opensearch.test.OpenSearchTestCase; public final class QueryShapeVisitorTests extends OpenSearchTestCase { diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java b/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizerTests.java similarity index 98% rename from src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java rename to src/test/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizerTests.java index 29ddb77..f3ae892 100644 --- a/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java +++ b/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizerTests.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.service.categorizor; +package org.opensearch.plugin.insights.core.service.categorizer; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -15,6 +15,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.opensearch.plugin.insights.QueryInsightsTestUtils.generateQueryInsightRecords; +import static org.opensearch.plugin.insights.core.service.categorizer.SearchQueryAggregationCategorizer.AGGREGATION_TYPE_TAG; import java.util.Arrays; import java.util.HashMap; @@ -114,7 +115,7 @@ public void testAggregationsQuery() { verify(searchQueryCategorizer.getSearchQueryCounters().getAggCounter()).add(valueCaptor.capture(), tagsCaptor.capture()); double actualValue = valueCaptor.getValue(); - String actualTag = (String) tagsCaptor.getValue().getTagsMap().get("agg_type"); + String actualTag = (String) tagsCaptor.getValue().getTagsMap().get(AGGREGATION_TYPE_TAG); assertEquals(1.0d, actualValue, 0.0001); assertEquals(MULTI_TERMS_AGGREGATION, actualTag); From 414c95a444b89d459a18137f67651642d4f369dd Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 27 Aug 2024 11:56:59 -0700 Subject: [PATCH 4/4] Spotless Signed-off-by: Siddhant Deshmukh --- .../core/service/categorizer/SearchQueryCategorizerTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizerTests.java b/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizerTests.java index f3ae892..903d36b 100644 --- a/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizerTests.java +++ b/src/test/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizerTests.java @@ -37,7 +37,6 @@ import org.opensearch.index.query.TermQueryBuilder; import org.opensearch.index.query.WildcardQueryBuilder; import org.opensearch.index.query.functionscore.FunctionScoreQueryBuilder; -import org.opensearch.plugin.insights.core.service.categorizer.SearchQueryCategorizer; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.search.aggregations.bucket.range.RangeAggregationBuilder;