Query instrumentation Minor Enhancements (opensearch-project#73)

* Query instrumentation minor fixes Signed-off-by: Siddhant Deshmukh <[email protected]> * Fix unit tests Signed-off-by: Siddhant Deshmukh <[email protected]> * Rename package Signed-off-by: Siddhant Deshmukh <[email protected]> * Spotless Signed-off-by: Siddhant Deshmukh <[email protected]> --------- Signed-off-by: Siddhant Deshmukh <[email protected]>
dzane17 · Aug 27, 2024 · c6a46f5 · c6a46f5
1 parent 1f4c4c6
commit c6a46f5
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 15 deletions.
diff --git a/...pensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java b/...pensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java
@@ -20,7 +20,7 @@
  */
 public class SearchQueryAggregationCategorizer {
 
-    private static final String TYPE_TAG = "type";
+    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<MetricType, Number> 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<AggregationBuilder> subAggregations = aggregationBuilder.getSubAggregations();
@@ -63,7 +63,7 @@ private void incrementCountersRecursively(AggregationBuilder aggregationBuilder,
         Collection<PipelineAggregationBuilder> 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/...ain/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java b/...ain/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<MetricTyp
 
         Counter counter = nameToQueryTypeCounters.computeIfAbsent(uniqueQueryCounterName, k -> 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);
     }
 
     /**

diff --git a/...categorizor/QueryShapeGeneratorTests.java → ...categorizer/QueryShapeGeneratorTests.java b/...categorizor/QueryShapeGeneratorTests.java → ...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/...e/categorizor/QueryShapeVisitorTests.java → ...e/categorizer/QueryShapeVisitorTests.java b/...e/categorizor/QueryShapeVisitorTests.java → ...e/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/...egorizor/SearchQueryCategorizerTests.java → ...egorizer/SearchQueryCategorizerTests.java b/...egorizor/SearchQueryCategorizerTests.java → ...egorizer/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;
@@ -36,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;
@@ -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(AGGREGATION_TYPE_TAG);
 
         assertEquals(1.0d, actualValue, 0.0001);
         assertEquals(MULTI_TERMS_AGGREGATION, actualTag);