akshayrai commented on a change in pull request #4271: Extend Detection Config
Validator to validate composite(entity) alerts
URL: https://github.com/apache/incubator-pinot/pull/4271#discussion_r290382653
##########
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/DetectionConfigValidator.java
##########
@@ -104,80 +112,155 @@ public void validateConfig(DetectionConfigDTO
detectionConfig) throws IllegalArg
/**
* Validates the detection or filter rule accordingly based on {@param
ruleType}
*/
- private void validateRule(Map<String, Object> ruleYaml, int ruleIndex,
String ruleType, Set<String> ruleNamesTaken) {
+ private void validateRule(String alertName, Map<String, Object> ruleYaml,
int ruleIndex, String ruleType, Set<String> ruleNamesTaken) {
Preconditions.checkArgument(ruleYaml.containsKey(PROP_TYPE),
- "In rule no." + (ruleIndex) + ", " + ruleType + " rule type is
missing.");
+ "Missing property " + ruleType + " (" + ruleType + ") for sub-alert "
+ alertName + " rule no. " + ruleIndex);
String type = MapUtils.getString(ruleYaml, PROP_TYPE);
- String name = MapUtils.getString(ruleYaml, PROP_NAME);
- Preconditions.checkNotNull(name,
- "In rule no." + (ruleIndex) + ", " + ruleType + " rule name for type "
+ type + " is missing.");
+
+ Preconditions.checkArgument(ruleYaml.containsKey(PROP_NAME),
+ "Missing property " + ruleType + " (" + PROP_NAME + ") for sub-alert "
+ alertName + " rule no. " + ruleIndex);
+ String name = (String) ruleYaml.get(PROP_NAME);
+
Preconditions.checkArgument(!ruleNamesTaken.contains(name),
- "In rule No." + (ruleIndex) + ", found duplicate rule name, rule name
must be unique within config." );
- Preconditions.checkArgument(!name.contains(":"), "Sorry, rule name cannot
contain \':\'");
+ "Duplicate rule name (" + name + ") found for sub-alert " + alertName
+ " rule no. " + ruleIndex + ". Names have to be unique within a config.");
+
+ Preconditions.checkArgument(!name.contains(":"),
+ "Illegal character (:) found in " + ruleType + " (" + PROP_NAME + ")
for sub-alert " + alertName + " rule no. " + ruleIndex);
}
- /**
- * Validate the the detection yaml configuration.
- *
- * @param detectionYaml the detection yaml configuration to be validated
- */
- @Override
- public void validateYaml(Map<String, Object> detectionYaml) {
+ private void validateBasicAttributes(Map<String, Object> detectionYaml,
String parentAlertName) {
+ Preconditions.checkArgument(detectionYaml.containsKey(PROP_NAME),
+ "Missing property ( " + PROP_NAME + " ) in one of the sub-alerts under
" + parentAlertName);
+ String alertName = (String) detectionYaml.get(PROP_NAME);
+
+ Preconditions.checkArgument(detectionYaml.containsKey(PROP_TYPE),
+ "Missing property ( " + PROP_TYPE + " ) in sub-alert " + alertName);
+ String alertType = (String) detectionYaml.get(PROP_TYPE);
+
+ Preconditions.checkArgument(SUPPORTED_ALERT_TYPES.contains(alertType),
+ "Unsupported type (" + alertType + ") in sub-alert " + alertName);
+ }
+
+ private void validateMetricAlertConfig(Map<String, Object> detectionYaml,
String parentAlertName)
+ throws IllegalArgumentException {
+ validateBasicAttributes(detectionYaml, parentAlertName);
+ String alertName = (String) detectionYaml.get(PROP_NAME);
+
// Validate all compulsory fields
-
Preconditions.checkArgument(detectionYaml.containsKey(PROP_DETECTION_NAME),
"Property missing " + PROP_DETECTION_NAME);
- Preconditions.checkArgument(detectionYaml.containsKey(PROP_METRIC),
"Property missing " + PROP_METRIC);
- Preconditions.checkArgument(detectionYaml.containsKey(PROP_DATASET),
"Property missing " + PROP_DATASET);
- Preconditions.checkArgument(detectionYaml.containsKey(PROP_RULES),
"Property missing " + PROP_RULES);
+ Preconditions.checkArgument(detectionYaml.containsKey(PROP_METRIC),
+ "Missing property (" + PROP_METRIC + ") in sub-alert " + alertName);
+ Preconditions.checkArgument(detectionYaml.containsKey(PROP_DATASET),
+ "Missing property (" + PROP_DATASET + ") in sub-alert " + alertName);
+ Preconditions.checkArgument(detectionYaml.containsKey(PROP_RULES),
+ "Missing property (" + PROP_RULES + ") in sub-alert " + alertName);
- // Validate fields which shouldn't be defined at root level
- Preconditions.checkArgument(!detectionYaml.containsKey(PROP_FILTER),
"Please double check the filter"
- + " config. Adding dimensions filters should be in the yaml root level
using 'filters' as the key. Anomaly "
- + "filter should be added in to the indentation level of detection
yaml it applies to.");
+ // Validate fields which shouldn't be defined at this level
+ Preconditions.checkArgument(!detectionYaml.containsKey(PROP_FILTER),
+ "For sub-alert " + alertName + ", please double check the filter
config. Adding dimensions filters"
+ + " should be in the yaml root level using 'filters' as the key.
Anomaly filter should be added in to the"
+ + " indentation level of detection yaml it applies to.");
// Check if the metric defined in the config exists
MetricConfigDTO metricConfig = provider
.fetchMetric(MapUtils.getString(detectionYaml, PROP_METRIC),
MapUtils.getString(detectionYaml, PROP_DATASET));
- Preconditions.checkNotNull(metricConfig, "Metric defined in the config
cannot be found");
- DatasetConfigDTO datasetConfig = provider
- .fetchDatasets(Collections.singletonList(metricConfig.getDataset()))
- .get(metricConfig.getDataset());
+ Preconditions.checkArgument(metricConfig != null,
+ "Invalid metric (not found) in sub-alert " + alertName);
// We support only one grouper per metric
Preconditions.checkArgument(ConfigUtils.getList(detectionYaml.get(PROP_GROUPER)).size()
<= 1,
- "Multiple groupers detected for metric. We support only one grouper
per metric.");
+ "Multiple groupers detected for metric in sub-alert " + alertName);
// Validate all the rules
Set<String> names = new HashSet<>();
List<Map<String, Object>> ruleYamls =
ConfigUtils.getList(detectionYaml.get(PROP_RULES));
- for (int i = 1; i <= ruleYamls.size(); i++) {
- Map<String, Object> ruleYaml = ruleYamls.get(i - 1);
+ for (int ruleIndex = 1; ruleIndex <= ruleYamls.size(); ruleIndex++) {
Review comment:
It doesn't really affect anything. I just retained the existing convention.
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
