akshayrai commented on a change in pull request #4250: SoC - Separate out Tuning from Translator URL: https://github.com/apache/incubator-pinot/pull/4250#discussion_r288760747
########## File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/DetectionConfigTuner.java ########## @@ -0,0 +1,158 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.pinot.thirdeye.detection.yaml; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableSet; +import java.util.Collections; +import java.util.Map; +import java.util.Set; +import org.apache.commons.collections.MapUtils; +import org.apache.pinot.thirdeye.datalayer.dto.DatasetConfigDTO; +import org.apache.pinot.thirdeye.datalayer.dto.DetectionConfigDTO; +import org.apache.pinot.thirdeye.datalayer.dto.MetricConfigDTO; +import org.apache.pinot.thirdeye.detection.ConfigUtils; +import org.apache.pinot.thirdeye.detection.DataProvider; +import org.apache.pinot.thirdeye.detection.DefaultInputDataFetcher; +import org.apache.pinot.thirdeye.detection.DetectionUtils; +import org.apache.pinot.thirdeye.detection.InputDataFetcher; +import org.apache.pinot.thirdeye.detection.annotation.registry.DetectionRegistry; +import org.apache.pinot.thirdeye.detection.spec.AbstractSpec; +import org.apache.pinot.thirdeye.detection.spi.components.Tunable; +import org.apache.pinot.thirdeye.rootcause.impl.MetricEntity; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; +import org.joda.time.Interval; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.apache.pinot.thirdeye.detection.DetectionUtils.*; + +public class DetectionConfigTuner { + protected static final Logger LOG = LoggerFactory.getLogger(DetectionConfigTuner.class); + + private static final String PROP_FILTERS = "filters"; + private static final String PROP_CLASS_NAME = "className"; + private static final String DEFAULT_TIMEZONE = "America/Los_Angeles"; + private static final String PROP_METRIC = "metric"; + private static final String PROP_DATASET = "dataset"; + + private static final DetectionRegistry DETECTION_REGISTRY = DetectionRegistry.getInstance(); + static { + // do not tune for alerts migrated from legacy anomaly function. + DetectionRegistry.registerComponent("com.linkedin.thirdeye.detection.components.AdLibAlertFilter", + "MIGRATED_ALGORITHM_FILTER"); + DetectionRegistry.registerComponent("com.linkedin.thirdeye.detection.components.AdLibAnomalyDetector", + "MIGRATED_ALGORITHM"); + } + + public static final String PROP_YAML_PARAMS = "yamlParams"; + public static final Set<String> TURNOFF_TUNING_COMPONENTS = + ImmutableSet.of("MIGRATED_ALGORITHM_FILTER", "MIGRATED_ALGORITHM", "MIGRATED_ALGORITHM_BASELINE"); + + private final DetectionConfigDTO detectionConfig; + private final DataProvider dataProvider; + private final DatasetConfigDTO datasetConfig; + private final String metricUrn; + + public DetectionConfigTuner(DetectionConfigDTO config, DataProvider dataProvider) { + this.detectionConfig = config; + this.dataProvider = dataProvider; + + Map<String, Object> yamlConfig = ConfigUtils.getMap(new org.yaml.snakeyaml.Yaml().load(config.getYaml())); + + MetricConfigDTO metricConfig = dataProvider.fetchMetric( + MapUtils.getString(yamlConfig, PROP_METRIC), + MapUtils.getString(yamlConfig, PROP_DATASET)); + Preconditions.checkNotNull(metricConfig, "metric not found"); + this.datasetConfig = dataProvider.fetchDatasets(Collections.singletonList(metricConfig.getDataset())) + .get(metricConfig.getDataset()); + Preconditions.checkNotNull(this.datasetConfig, "dataset not found"); + + this.metricUrn = MetricEntity.buildMetricUrn(MapUtils.getMap(yamlConfig, PROP_FILTERS), metricConfig.getId()); + } + + /** + * Returns the default(for new alert) or tuned configuration for the specified component + * + * @param componentKey the key used to reference the component within the detection + * @param componentProps previously tuned configuration along with user supplied yaml config + * @param startTime start time of the tuning window + * @param endTime end time of the tuning window + */ + private Map<String, Object> getTunedSpecs(String componentKey, Map<String, Object> componentProps, long startTime, long endTime) + throws Exception { + // Instantiate tunable component + long configId = detectionConfig == null ? 0 : detectionConfig.getId(); + String componentClassName = componentProps.get(PROP_CLASS_NAME).toString(); + Map<String, Object> yamlParams = ConfigUtils.getMap(componentProps.get(PROP_YAML_PARAMS)); + InputDataFetcher dataFetcher = new DefaultInputDataFetcher(dataProvider, configId); + Tunable tunable = instantiateTunable(componentClassName, yamlParams, dataFetcher); + + // round to daily boundary + DateTimeZone timezone = DateTimeZone.forID(this.datasetConfig.getTimezone() == null ? DEFAULT_TIMEZONE : this.datasetConfig.getTimezone()); + DateTime start = new DateTime(startTime, timezone).withTimeAtStartOfDay(); + DateTime end = new DateTime(endTime, timezone).withTimeAtStartOfDay(); + Interval window = new Interval(start, end); + + // TODO can you use componentProps instead? what is the difference btw component and componentSpec + Map<String, Object> existingComponentSpec = + detectionConfig.getComponents().containsKey(componentKey) ? + MapUtils.getMap(detectionConfig.getComponents(), componentKey) : Collections.emptyMap(); Review comment: Thanks for the review. I wanted to clarify something related to this but it is clear now. componentProps passed to this method contains the previously tuned configuration along with user supplied yaml config. I will directly pass the componentProps rather than extracting it from the detectionConfig ---------------------------------------------------------------- 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]
