pr comments

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/src/main/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/quickpulse/filtering/DerivedMetricAggregation.java

@@ -45,7 +45,6 @@ void update(double incrementBy) {
         synchronized (lock) {
             if (count == 0) {
                 return 0.0;
-
             }
             if (aggregationType.equals(AggregationType.AVG)) {
                 return aggregation / count;

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/src/main/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/quickpulse/filtering/Validator.java

@@ -104,9 +104,9 @@ private boolean isValidTelemetryType(TelemetryType telemetryType) {
             return false;
         } else if (telemetryType.equals(TelemetryType.METRIC)) {
             return false;
-        } else {
-            return true;
         }
+        return true;
+
     }
 
     private boolean isNotCustomMetricProjection(String projection) {

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/src/test/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/quickpulse/filteringTest/ValidatorTests.java

@@ -6,6 +6,9 @@
 import com.azure.monitor.opentelemetry.autoconfigure.implementation.quickpulse.filtering.Validator;
 import com.azure.monitor.opentelemetry.autoconfigure.implementation.quickpulse.swagger.models.*;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -15,36 +18,46 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-public class ValidatorTests {
+class ValidatorTests {
 
-    @Test
-    void rejectInvalidTelemetryTypes() {
-        List<TelemetryType> telemetryTypes
-            = asList(TelemetryType.EVENT, TelemetryType.PERFORMANCE_COUNTER, TelemetryType.METRIC);
+    @ParameterizedTest
+    @MethodSource("invalidTelemetryTypes")
+    void rejectInvalidTelemetryTypesForDmi(TelemetryType telemetryType) {
         Validator validator = new Validator();
-        for (TelemetryType telemetryType : telemetryTypes) {
-            List<FilterConjunctionGroupInfo> filterGroups = createListWithOneFilterConjunctionGroupAndNoFilters();
-            DerivedMetricInfo dmi = createDerivedMetricInfo("random-id", telemetryType.getValue(), AggregationType.SUM,
-                AggregationType.SUM, DerivedMetricProjections.COUNT, filterGroups);
-            assertFalse(validator.isValidDerivedMetricInfo(dmi));
-            DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithNoFilters(telemetryType);
-            assertFalse(validator.isValidDocConjunctionGroupInfo(docGroup));
-        }
+        List<FilterConjunctionGroupInfo> filterGroups = createListWithOneFilterConjunctionGroupAndNoFilters();
+        DerivedMetricInfo dmi = createDerivedMetricInfo("random-id", telemetryType.getValue(), AggregationType.SUM,
+            AggregationType.SUM, DerivedMetricProjections.COUNT, filterGroups);
+        assertFalse(validator.isValidDerivedMetricInfo(dmi));
+        DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithNoFilters(telemetryType);
+        assertFalse(validator.isValidDocConjunctionGroupInfo(docGroup));
     }
 
-    @Test
-    void acceptValidTelemetryType() {
-        List<TelemetryType> telemetryTypes
-            = asList(TelemetryType.TRACE, TelemetryType.REQUEST, TelemetryType.DEPENDENCY, TelemetryType.EXCEPTION);
+    @ParameterizedTest
+    @MethodSource("invalidTelemetryTypes")
+    void rejectInvalidTelemetryTypesForDocs(TelemetryType telemetryType) {
+        Validator validator = new Validator();
+        DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithNoFilters(telemetryType);
+        assertFalse(validator.isValidDocConjunctionGroupInfo(docGroup));
+    }
+
+    @ParameterizedTest
+    @MethodSource("validTelemetryTypes")
+    void acceptValidTelemetryTypeForDmi(TelemetryType telemetryType) {
         Validator validator = new Validator();
-        for (TelemetryType telemetryType : telemetryTypes) {
-            List<FilterConjunctionGroupInfo> filterGroups = createListWithOneFilterConjunctionGroupAndNoFilters();
-            DerivedMetricInfo dmi = createDerivedMetricInfo("random-id", telemetryType.getValue(), AggregationType.SUM,
-                AggregationType.SUM, DerivedMetricProjections.COUNT, filterGroups);
-            assertTrue(validator.isValidDerivedMetricInfo(dmi));
-            DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithNoFilters(telemetryType);
-            assertTrue(validator.isValidDocConjunctionGroupInfo(docGroup));
-        }
+        List<FilterConjunctionGroupInfo> filterGroups = createListWithOneFilterConjunctionGroupAndNoFilters();
+        DerivedMetricInfo dmi = createDerivedMetricInfo("random-id", telemetryType.getValue(), AggregationType.SUM,
+            AggregationType.SUM, DerivedMetricProjections.COUNT, filterGroups);
+        assertTrue(validator.isValidDerivedMetricInfo(dmi));
+        DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithNoFilters(telemetryType);
+        assertTrue(validator.isValidDocConjunctionGroupInfo(docGroup));
+    }
+
+    @ParameterizedTest
+    @MethodSource("validTelemetryTypes")
+    void acceptValidTelemetryTypeForDocs(TelemetryType telemetryType) {
+        Validator validator = new Validator();
+        DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithNoFilters(telemetryType);
+        assertTrue(validator.isValidDocConjunctionGroupInfo(docGroup));
     }
 
     @Test
@@ -54,7 +67,6 @@ void rejectCustomMetricProjection() {
         DerivedMetricInfo dmi = createDerivedMetricInfo("random-id", TelemetryType.TRACE.getValue(),
             AggregationType.SUM, AggregationType.SUM, "CustomMetrics.property", filterGroups);
         assertFalse(validator.isValidDerivedMetricInfo(dmi));
-
     }
 
     @Test
@@ -67,34 +79,25 @@ void rejectCustomMetricFieldName() {
         assertFalse(validator.isValidDerivedMetricInfo(dmi));
     }
 
-    @Test
-    void rejectInvalidFilters() {
-        List<FilterInfo> filtersToTest = Arrays.asList(createFilterInfoWithParams("", PredicateType.EQUAL, "blah"),
-            createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.EQUAL, ""),
-            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.EQUAL, "5"),
-            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.NOT_EQUAL, "5"),
-            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.LESS_THAN, "5"),
-            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.LESS_THAN_OR_EQUAL, "5"),
-            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.GREATER_THAN, "5"),
-            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.GREATER_THAN_OR_EQUAL, "5"),
-            createFilterInfoWithParams("CustomDimensions.property", PredicateType.LESS_THAN, "blah"),
-            createFilterInfoWithParams("CustomDimensions.property", PredicateType.LESS_THAN_OR_EQUAL, "blah"),
-            createFilterInfoWithParams("CustomDimensions.property", PredicateType.GREATER_THAN, "blah"),
-            createFilterInfoWithParams("CustomDimensions.property", PredicateType.GREATER_THAN_OR_EQUAL, "blah"),
-            createFilterInfoWithParams(KnownRequestColumns.DURATION, PredicateType.LESS_THAN, "invalid timestamp"),
-            createFilterInfoWithParams(KnownRequestColumns.DURATION, PredicateType.LESS_THAN, "150"),
-            createFilterInfoWithParams(KnownRequestColumns.DURATION, PredicateType.LESS_THAN, "0.0:0:"));
+    @ParameterizedTest
+    @MethodSource("invalidFilters")
+    void rejectInvalidFiltersForDmi(FilterInfo filter) {
+        List<FilterConjunctionGroupInfo> filterGroups = createListWithOneFilterConjunctionGroupAndOneFilter(filter);
+        DerivedMetricInfo dmi = createDerivedMetricInfo("random-id", TelemetryType.REQUEST.getValue(),
+            AggregationType.SUM, AggregationType.SUM, DerivedMetricProjections.COUNT, filterGroups);
+        Validator validator = new Validator();
+        assertFalse(validator.isValidDerivedMetricInfo(dmi));
 
-        for (FilterInfo filter : filtersToTest) {
-            List<FilterConjunctionGroupInfo> filterGroups = createListWithOneFilterConjunctionGroupAndOneFilter(filter);
-            DerivedMetricInfo dmi = createDerivedMetricInfo("random-id", TelemetryType.REQUEST.getValue(),
-                AggregationType.SUM, AggregationType.SUM, DerivedMetricProjections.COUNT, filterGroups);
-            Validator validator = new Validator();
-            assertFalse(validator.isValidDerivedMetricInfo(dmi));
+        DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithOneFilter(TelemetryType.REQUEST, filter);
+        assertFalse(validator.isValidDocConjunctionGroupInfo(docGroup));
+    }
 
-            DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithOneFilter(TelemetryType.REQUEST, filter);
-            assertFalse(validator.isValidDocConjunctionGroupInfo(docGroup));
-        }
+    @ParameterizedTest
+    @MethodSource("invalidFilters")
+    void rejectInvalidFiltersForDocs(FilterInfo filter) {
+        Validator validator = new Validator();
+        DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithOneFilter(TelemetryType.REQUEST, filter);
+        assertFalse(validator.isValidDocConjunctionGroupInfo(docGroup));
     }
 
     @Test
@@ -119,42 +122,76 @@ void rejectInvalidGroupWithMultipleFilters() {
         assertFalse(validator.isValidDocConjunctionGroupInfo(docGroup));
     }
 
-    @Test
-    void acceptValidFilters() {
-        List<FilterInfo> filtersToTest
-            = Arrays.asList(createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.CONTAINS, "hi"),
-                createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.DOES_NOT_CONTAIN, "hi"),
-                createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.NOT_EQUAL, "hi"),
-                createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.EQUAL, "hi"),
-                createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.CONTAINS, "hi"),
-                createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.DOES_NOT_CONTAIN, "hi"),
-                createFilterInfoWithParams("CustomDimensions.property", PredicateType.NOT_EQUAL, "hi"),
-                createFilterInfoWithParams("CustomDimensions.property", PredicateType.EQUAL, "hi"),
-                createFilterInfoWithParams("CustomDimensions.property", PredicateType.CONTAINS, "hi"),
-                createFilterInfoWithParams("CustomDimensions.property", PredicateType.DOES_NOT_CONTAIN, "hi"),
-                createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.LESS_THAN, "5"),
-                createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.LESS_THAN_OR_EQUAL, "5"),
-                createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.GREATER_THAN, "5"),
-                createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.GREATER_THAN_OR_EQUAL, "5"),
-                createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.EQUAL, "5"),
-                createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.NOT_EQUAL, "5"),
-                createFilterInfoWithParams(KnownRequestColumns.DURATION, PredicateType.EQUAL, "0.0:0:0.2"),
-                createFilterInfoWithParams(KnownRequestColumns.SUCCESS, PredicateType.EQUAL, "true"),
-                createFilterInfoWithParams(KnownRequestColumns.SUCCESS, PredicateType.NOT_EQUAL, "false"));
-
-        for (FilterInfo filter : filtersToTest) {
-            List<FilterConjunctionGroupInfo> filterGroups = createListWithOneFilterConjunctionGroupAndOneFilter(filter);
-            DerivedMetricInfo dmi = createDerivedMetricInfo("random-id", TelemetryType.REQUEST.getValue(),
-                AggregationType.SUM, AggregationType.SUM, DerivedMetricProjections.COUNT, filterGroups);
-            Validator validator = new Validator();
-            assertTrue(validator.isValidDerivedMetricInfo(dmi));
-
-            DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithOneFilter(TelemetryType.REQUEST, filter);
-            assertTrue(validator.isValidDocConjunctionGroupInfo(docGroup));
-        }
-    }
-
-    private FilterInfo createFilterInfoWithParams(String fieldName, PredicateType predicate, String comparand) {
+    @ParameterizedTest
+    @MethodSource("validFilters")
+    void acceptValidFiltersForDmi(FilterInfo filter) {
+        List<FilterConjunctionGroupInfo> filterGroups = createListWithOneFilterConjunctionGroupAndOneFilter(filter);
+        DerivedMetricInfo dmi = createDerivedMetricInfo("random-id", TelemetryType.REQUEST.getValue(),
+            AggregationType.SUM, AggregationType.SUM, DerivedMetricProjections.COUNT, filterGroups);
+        Validator validator = new Validator();
+        assertTrue(validator.isValidDerivedMetricInfo(dmi));
+
+        DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithOneFilter(TelemetryType.REQUEST, filter);
+        assertTrue(validator.isValidDocConjunctionGroupInfo(docGroup));
+    }
+
+    @ParameterizedTest
+    @MethodSource("validFilters")
+    void acceptValidFiltersForDocs(FilterInfo filter) {
+        Validator validator = new Validator();
+        DocumentFilterConjunctionGroupInfo docGroup = createDocGroupWithOneFilter(TelemetryType.REQUEST, filter);
+        assertTrue(validator.isValidDocConjunctionGroupInfo(docGroup));
+    }
+
+    private static List<TelemetryType> invalidTelemetryTypes() {
+        return Arrays.asList(TelemetryType.METRIC, TelemetryType.EVENT, TelemetryType.PERFORMANCE_COUNTER);
+    }
+
+    private static List<TelemetryType> validTelemetryTypes() {
+        return Arrays.asList(TelemetryType.DEPENDENCY, TelemetryType.EXCEPTION, TelemetryType.REQUEST, TelemetryType.TRACE);
+    }
+
+    private static List<FilterInfo> invalidFilters() {
+        return Arrays.asList(createFilterInfoWithParams("", PredicateType.EQUAL, "blah"),
+            createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.EQUAL, ""),
+            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.EQUAL, "5"),
+            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.NOT_EQUAL, "5"),
+            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.LESS_THAN, "5"),
+            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.LESS_THAN_OR_EQUAL, "5"),
+            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.GREATER_THAN, "5"),
+            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.GREATER_THAN_OR_EQUAL, "5"),
+            createFilterInfoWithParams("CustomDimensions.property", PredicateType.LESS_THAN, "blah"),
+            createFilterInfoWithParams("CustomDimensions.property", PredicateType.LESS_THAN_OR_EQUAL, "blah"),
+            createFilterInfoWithParams("CustomDimensions.property", PredicateType.GREATER_THAN, "blah"),
+            createFilterInfoWithParams("CustomDimensions.property", PredicateType.GREATER_THAN_OR_EQUAL, "blah"),
+            createFilterInfoWithParams(KnownRequestColumns.DURATION, PredicateType.LESS_THAN, "invalid timestamp"),
+            createFilterInfoWithParams(KnownRequestColumns.DURATION, PredicateType.LESS_THAN, "150"),
+            createFilterInfoWithParams(KnownRequestColumns.DURATION, PredicateType.LESS_THAN, "0.0:0:"));
+    }
+
+    private static List<FilterInfo> validFilters() {
+        return Arrays.asList(createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.CONTAINS, "hi"),
+            createFilterInfoWithParams(Filter.ANY_FIELD, PredicateType.DOES_NOT_CONTAIN, "hi"),
+            createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.NOT_EQUAL, "hi"),
+            createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.EQUAL, "hi"),
+            createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.CONTAINS, "hi"),
+            createFilterInfoWithParams(KnownRequestColumns.URL, PredicateType.DOES_NOT_CONTAIN, "hi"),
+            createFilterInfoWithParams("CustomDimensions.property", PredicateType.NOT_EQUAL, "hi"),
+            createFilterInfoWithParams("CustomDimensions.property", PredicateType.EQUAL, "hi"),
+            createFilterInfoWithParams("CustomDimensions.property", PredicateType.CONTAINS, "hi"),
+            createFilterInfoWithParams("CustomDimensions.property", PredicateType.DOES_NOT_CONTAIN, "hi"),
+            createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.LESS_THAN, "5"),
+            createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.LESS_THAN_OR_EQUAL, "5"),
+            createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.GREATER_THAN, "5"),
+            createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.GREATER_THAN_OR_EQUAL, "5"),
+            createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.EQUAL, "5"),
+            createFilterInfoWithParams(KnownRequestColumns.RESPONSE_CODE, PredicateType.NOT_EQUAL, "5"),
+            createFilterInfoWithParams(KnownRequestColumns.DURATION, PredicateType.EQUAL, "0.0:0:0.2"),
+            createFilterInfoWithParams(KnownRequestColumns.SUCCESS, PredicateType.EQUAL, "true"),
+            createFilterInfoWithParams(KnownRequestColumns.SUCCESS, PredicateType.NOT_EQUAL, "false"));
+    }
+
+    private static FilterInfo createFilterInfoWithParams(String fieldName, PredicateType predicate, String comparand) {
         FilterInfo result = new FilterInfo();
         result.setFieldName(fieldName);
         result.setPredicate(predicate);