You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/08/26 19:22:42 UTC

[GitHub] [incubator-pinot] akshayrai opened a new pull request #5930: [TE] ThirdEye User Configuration Validation - detection and subscription yaml

akshayrai opened a new pull request #5930:
URL: https://github.com/apache/incubator-pinot/pull/5930


   ThirdEye uses YAML files to manage the configuration of alerts and subscription groups. This approach enables employees across engineering and non-engineering, to configure simple to complex rules in an efficient way. However, mistakes in such files are easily made. These mistakes could lead to detection/subscription errors and fail to detect & notify a critical anomaly.
   
   This PR tackles and addresses this issue by introducing schema based validations on user configs. I have used the json-schema-validator to achieve this. You will find the validation schema defined in `detection-config-schema.json` and `subscription-config-schema.json`
   
   Changes:
   * Refactored the validation interfaces with - static and semantic validation
   * Introduced the json schema based validation of detection and subscription config
   
   Tests:
   * Added unit testing of configs
   * Deployed and tested locally if the validation messages are displayed to the user
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #5930: [TE] ThirdEye User Configuration Validation - detection and subscription yaml

Posted by GitBox <gi...@apache.org>.
jihaozh commented on a change in pull request #5930:
URL: https://github.com/apache/incubator-pinot/pull/5930#discussion_r477624214



##########
File path: thirdeye/thirdeye-pinot/src/main/resources/validators/detection/detection-config-schema.json
##########
@@ -0,0 +1,214 @@
+{
+  "$schema": "http://json-schema.org/draft-04/schema#",
+  "title": "Detection Config Schema",
+
+  "type": "object",
+  "description": "YAML representation of metric or composite detection config",
+
+  "required": [ "detectionName" ],
+
+  "properties": {
+    "detectionName": {
+      "type": "string"
+    },
+    "description": {
+      "type": "string"
+    },
+    "type": {
+      "type": "string",
+      "description": "detection pipeline type",
+      "enum": [
+        "METRIC_ALERT",
+        "COMPOSITE_ALERT"
+      ]
+    },
+    "metric": {
+      "type": "string"
+    },
+    "dataset": {
+      "type": "string"
+    },
+    "active": {
+      "type": "boolean"
+    },
+    "cron": {
+      "type": "string"
+    },
+    "filters": {
+      "$ref": "#/definitions/filters"
+    },
+    "dimensionExploration": {
+      "$ref": "#/definitions/dimensionExploration"
+    },
+    "rules": {
+      "$ref": "#/definitions/rules"
+    },
+    "alerts": {
+      "$ref": "#/definitions/alerts"
+    },
+    "merger": {
+      "$ref": "#/definitions/merger"
+    },
+    "grouper": {
+      "$ref": "#/definitions/grouper"
+    }
+  },
+
+  "definitions": {
+    "alerts": {
+      "type": "array",
+      "items": {
+        "$ref": "#/definitions/subEntity"
+      }
+    },
+
+    "subEntity": {
+      "type": "object",
+      "description": "a sub-entity detection config",
+      "required": [ "name", "type" ],
+      "properties": {
+        "name": {
+          "type": "string"
+        },
+        "description": {
+          "type": "string"
+        },
+        "type": {
+          "type": "string",
+          "description": "detection pipeline type",
+          "enum": [
+            "COMPOSITE_ALERT",
+            "METRIC_ALERT"
+          ]
+        },
+        "alerts": {
+          "$ref": "#/definitions/alerts"
+        },
+        "metric": {
+          "type": "string"
+        },
+        "dataset": {
+          "type": "string"
+        },
+        "filters": {
+          "$ref": "#/definitions/filters"
+        },
+        "dimensionExploration": {
+          "$ref": "#/definitions/dimensionExploration"
+        },
+        "rules": {
+          "$ref": "#/definitions/rules"
+        },
+        "merger": {
+          "$ref": "#/definitions/merger"
+        },
+        "grouper": {
+          "$ref": "#/definitions/grouper"
+        }
+      },
+      "additionalProperties": false
+    },
+
+    "dimensionExploration": {
+      "type": "object",
+      "description": "exploration combination of dimension values",
+      "properties": {
+        "dimensions": {
+          "type": "array",
+          "description": "dimension column names",
+          "items": {
+            "type": "string"
+          }
+        },
+        "dimensionFilterMetric": {
+          "type": "string"
+        }
+      },
+      "patternProperties": {
+        "k|minValue|minContribution|minValueHourly|maxValueHourly|minValueDaily|maxValueDaily": {
+          "type": "number"
+        }
+      },
+      "additionalProperties": false
+    },
+
+    "filters": {
+      "type": "object",
+      "description": "dimension filters",
+      "patternProperties": {
+        ".*": {
+          "type": "array",
+          "description": "dimension filter values",
+          "items": {
+            "type": "string"
+          }
+        }
+      }
+    },
+
+    "eachComponent": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "required": [ "type", "name" ],
+        "properties": {
+          "type": {
+            "type": "string"

Review comment:
       shall the rule type be an enum?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] akshayrai merged pull request #5930: [TE] ThirdEye User Configuration Validation - detection and subscription yaml

Posted by GitBox <gi...@apache.org>.
akshayrai merged pull request #5930:
URL: https://github.com/apache/incubator-pinot/pull/5930


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #5930: [TE] ThirdEye User Configuration Validation - detection and subscription yaml

Posted by GitBox <gi...@apache.org>.
akshayrai commented on a change in pull request #5930:
URL: https://github.com/apache/incubator-pinot/pull/5930#discussion_r477627648



##########
File path: thirdeye/thirdeye-pinot/src/main/resources/validators/subscription/subscription-config-schema.json
##########
@@ -0,0 +1,110 @@
+{
+  "$schema": "http://json-schema.org/draft-04/schema#",
+  "title": "Subscription Config Schema",
+
+  "type": "object",
+  "description": "YAML representation of a subscription group config",
+  "required": [ "subscriptionGroupName", "subscribedDetections", "application", "alertSchemes" ],
+
+  "properties": {
+    "subscriptionGroupName": {
+      "type": "string"
+    },
+    "application": {
+      "type": "string"
+    },
+    "type": {
+      "type": "string"

Review comment:
       replied above.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #5930: [TE] ThirdEye User Configuration Validation - detection and subscription yaml

Posted by GitBox <gi...@apache.org>.
akshayrai commented on a change in pull request #5930:
URL: https://github.com/apache/incubator-pinot/pull/5930#discussion_r477627360



##########
File path: thirdeye/thirdeye-pinot/src/main/resources/validators/detection/detection-config-schema.json
##########
@@ -0,0 +1,214 @@
+{
+  "$schema": "http://json-schema.org/draft-04/schema#",
+  "title": "Detection Config Schema",
+
+  "type": "object",
+  "description": "YAML representation of metric or composite detection config",
+
+  "required": [ "detectionName" ],
+
+  "properties": {
+    "detectionName": {
+      "type": "string"
+    },
+    "description": {
+      "type": "string"
+    },
+    "type": {
+      "type": "string",
+      "description": "detection pipeline type",
+      "enum": [
+        "METRIC_ALERT",
+        "COMPOSITE_ALERT"
+      ]
+    },
+    "metric": {
+      "type": "string"
+    },
+    "dataset": {
+      "type": "string"
+    },
+    "active": {
+      "type": "boolean"
+    },
+    "cron": {
+      "type": "string"
+    },
+    "filters": {
+      "$ref": "#/definitions/filters"
+    },
+    "dimensionExploration": {
+      "$ref": "#/definitions/dimensionExploration"
+    },
+    "rules": {
+      "$ref": "#/definitions/rules"
+    },
+    "alerts": {
+      "$ref": "#/definitions/alerts"
+    },
+    "merger": {
+      "$ref": "#/definitions/merger"
+    },
+    "grouper": {
+      "$ref": "#/definitions/grouper"
+    }
+  },
+
+  "definitions": {
+    "alerts": {
+      "type": "array",
+      "items": {
+        "$ref": "#/definitions/subEntity"
+      }
+    },
+
+    "subEntity": {
+      "type": "object",
+      "description": "a sub-entity detection config",
+      "required": [ "name", "type" ],
+      "properties": {
+        "name": {
+          "type": "string"
+        },
+        "description": {
+          "type": "string"
+        },
+        "type": {
+          "type": "string",
+          "description": "detection pipeline type",
+          "enum": [
+            "COMPOSITE_ALERT",
+            "METRIC_ALERT"
+          ]
+        },
+        "alerts": {
+          "$ref": "#/definitions/alerts"
+        },
+        "metric": {
+          "type": "string"
+        },
+        "dataset": {
+          "type": "string"
+        },
+        "filters": {
+          "$ref": "#/definitions/filters"
+        },
+        "dimensionExploration": {
+          "$ref": "#/definitions/dimensionExploration"
+        },
+        "rules": {
+          "$ref": "#/definitions/rules"
+        },
+        "merger": {
+          "$ref": "#/definitions/merger"
+        },
+        "grouper": {
+          "$ref": "#/definitions/grouper"
+        }
+      },
+      "additionalProperties": false
+    },
+
+    "dimensionExploration": {
+      "type": "object",
+      "description": "exploration combination of dimension values",
+      "properties": {
+        "dimensions": {
+          "type": "array",
+          "description": "dimension column names",
+          "items": {
+            "type": "string"
+          }
+        },
+        "dimensionFilterMetric": {
+          "type": "string"
+        }
+      },
+      "patternProperties": {
+        "k|minValue|minContribution|minValueHourly|maxValueHourly|minValueDaily|maxValueDaily": {
+          "type": "number"
+        }
+      },
+      "additionalProperties": false
+    },
+
+    "filters": {
+      "type": "object",
+      "description": "dimension filters",
+      "patternProperties": {
+        ".*": {
+          "type": "array",
+          "description": "dimension filter values",
+          "items": {
+            "type": "string"
+          }
+        }
+      }
+    },
+
+    "eachComponent": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "required": [ "type", "name" ],
+        "properties": {
+          "type": {
+            "type": "string"

Review comment:
       We can define an enum, but I avoided it for a couple of reasons.
   1. We do not want to hard code and maintain the list of possible rule types here.
   2. We have internal algorithm types which shouldn't be hardcoded here.
   3. We run a validation in the code which will anyways capture if the type is invalid.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #5930: [TE] ThirdEye User Configuration Validation - detection and subscription yaml

Posted by GitBox <gi...@apache.org>.
jihaozh commented on a change in pull request #5930:
URL: https://github.com/apache/incubator-pinot/pull/5930#discussion_r477624953



##########
File path: thirdeye/thirdeye-pinot/src/main/resources/validators/subscription/subscription-config-schema.json
##########
@@ -0,0 +1,110 @@
+{
+  "$schema": "http://json-schema.org/draft-04/schema#",
+  "title": "Subscription Config Schema",
+
+  "type": "object",
+  "description": "YAML representation of a subscription group config",
+  "required": [ "subscriptionGroupName", "subscribedDetections", "application", "alertSchemes" ],
+
+  "properties": {
+    "subscriptionGroupName": {
+      "type": "string"
+    },
+    "application": {
+      "type": "string"
+    },
+    "type": {
+      "type": "string"

Review comment:
       shall the subscription group type be an enum type?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org