jihaozh commented on a change in pull request #4333: [TE] Entity Yaml
(Composite Alert) Translator
URL: https://github.com/apache/incubator-pinot/pull/4333#discussion_r295440335
##########
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/translator/DetectionConfigTranslator.java
##########
@@ -210,16 +212,89 @@ DetectionConfigDTO translateConfig(Map<String, Object>
yamlConfigMap) throws Ill
nestedPipelines.addAll(filterNestedProperties);
}
}
+
+ // Wrap with dimension exploration properties
Map<String, Object> dimensionWrapperProperties =
buildDimensionWrapperProperties(
- yamlConfigMap, dimensionFiltersMap, metricUrn,
datasetConfig.getDataset());
+ metricAlertConfigMap, dimensionFiltersMap, metricUrn,
datasetConfigDTO.getDataset());
Map<String, Object> properties = buildWrapperProperties(
ChildKeepingMergeWrapper.class.getName(),
Collections.singletonList(buildWrapperProperties(DimensionWrapper.class.getName(),
nestedPipelines, dimensionWrapperProperties)),
mergerProperties);
- List<Map<String, Object>> grouperYamls =
getList(yamlConfigMap.get(PROP_GROUPER));
+ // Wrap with metric level grouper, restricting to only 1 grouper
+ List<Map<String, Object>> grouperYamls =
getList(metricAlertConfigMap.get(PROP_GROUPER));
if (!grouperYamls.isEmpty()) {
- properties = buildGroupWrapperProperties(grouperYamls.get(0),
properties);
+ properties = buildGroupWrapperProperties(grouperYamls.get(0),
Collections.singletonList(properties));
+ }
+
+ return properties;
+ }
+
+ private Map<String, Object> translateCompositeAlert(Map<String, Object>
compositeAlertConfigMap) {
+ Map<String, Object> properties;
+
+ // Recursively translate all the sub-alerts
+ List<Map<String, Object>> subDetectionYamls =
ConfigUtils.getList(compositeAlertConfigMap.get(PROP_ALERTS));
+ List<Map<String, Object>> nestedPropertiesList = new ArrayList<>();
+ for (Map<String, Object> subDetectionYaml : subDetectionYamls) {
+ Map<String, Object> subProps;
+ if (subDetectionYaml.containsKey(PROP_TYPE) &&
subDetectionYaml.get(PROP_TYPE).equals(COMPOSITE_ALERT)) {
+ subProps = translateCompositeAlert(subDetectionYaml);
+ } else {
+ subProps = translateMetricAlert(subDetectionYaml);
+ }
+
+ nestedPropertiesList.add(subProps);
+ }
+
+ // Wrap the entity level grouper, only 1 grouper is supported now
+ List<Map<String, Object>> grouperProps =
ConfigUtils.getList(compositeAlertConfigMap.get(PROP_GROUPER));
+ if (!grouperProps.isEmpty()) {
+ properties = buildGroupWrapperProperties(grouperProps.get(0),
nestedPropertiesList);
+ } else {
+ Map<String, Object> defaultGrouper = new HashMap<>();
+ defaultGrouper.put(PROP_TYPE, "MOCK_GROUPER");
+ defaultGrouper.put(PROP_NAME, "Default grouper");
+ properties = buildGroupWrapperProperties(defaultGrouper,
nestedPropertiesList);
+ }
+
+ // Wrap the entity level merger
+ Map<String, Object> mergerProperties =
ConfigUtils.getMap(compositeAlertConfigMap.get(PROP_MERGER));
+ properties = buildWrapperProperties(
+ ChildKeepingMergeWrapper.class.getName(),
+ Collections.singletonList(properties),
+ mergerProperties);
+
+ return properties;
+ }
+
+ @Override
+ DetectionConfigDTO translateConfig(Map<String, Object> yamlConfigMap) throws
IllegalArgumentException {
+ // Hack to support 'detectionName' attribute at root level and 'name'
attribute elsewhere
+ // We consistently use 'name' as a convention to define the sub-alerts.
However, at the root
+ // level, as a convention, we will use 'detectionName' which defines the
name of the complete alert.
+ String alertName = MapUtils.getString(yamlConfigMap, PROP_DETECTION_NAME);
+ yamlConfigMap.put(PROP_NAME, alertName);
+
+ // By default if 'type' is not specified, we assume it as a METRIC_ALERT
+ if (!yamlConfigMap.containsKey(PROP_TYPE)) {
+ yamlConfigMap.put(PROP_TYPE, METRIC_ALERT);
+ }
+
+ // Translate config depending on the type (METRIC_ALERT OR COMPOSITE_ALERT)
+ Map<String, Object> properties;
+ String cron;
+ if (yamlConfigMap.get(PROP_TYPE).equals(COMPOSITE_ALERT)) {
+ properties = translateCompositeAlert(yamlConfigMap);
+
+ // TODO: discuss strategy for default cron
+ Preconditions.checkArgument(yamlConfigMap.containsKey(PROP_CRON),
"Missing property (" + PROP_CRON + ") in alert");
+ cron = MapUtils.getString(yamlConfigMap, PROP_CRON);
+ } else {
+ // The legacy type 'COMPOSITE' will be treated as a metric alert along
with the new convention METRIC_ALERT.
+ // This is applicable only at the root level to maintain backward
compatibility.
+ properties = translateMetricAlert(yamlConfigMap);
+ cron = metricCache.fetchCron(yamlConfigMap);
Review comment:
If YAML contains cron expression, it should override the default cron here
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
