jihaozh commented on a change in pull request #6333:
URL: https://github.com/apache/incubator-pinot/pull/6333#discussion_r538752824



##########
File path: 
thirdeye/thirdeye-frontend/app/pods/components/rootcause-metrics/component.js
##########
@@ -65,10 +55,29 @@ export default Component.extend({
    */
   onSelection: null, // function (Set, state)
 
+  context: null,
+
+  compareMode: null, // ""
+
+  didReceiveAttrs() {
+    const context = get(this, 'context');
+
+    set(this, 'compareMode', (context || {}).compareMode);
+  },
+
   //
   // internal properties
   //
 
+  /**
+   * Columns for metrics table
+   * @type Object[]
+   */
+  metricsTableColumns: computed('compareMode', function () {
+    const compareMode = get(this, 'compareMode');
+    return compareMode === 'forecast' ? METRIC_FUNNEL_COLUMNS : 
METRICS_TABLE_COLUMNS;

Review comment:
       So the funnel table shows up when the forecast is selected, correct?

##########
File path: thirdeye/thirdeye-frontend/app/pods/rootcause/controller.js
##########
@@ -305,140 +306,153 @@ export default Controller.extend({
    * callgraph:        service call graph edges as ranked by the backend
    *                   (typically displayed in call graph table)
    */
-  _contextObserver: observer(
-    'context',
-    'entities',
-    'selectedUrns',
-    'sizeMetricUrns',
-    'activeTab',
-    function () {
-      const { context, selectedUrns, sizeMetricUrns, entitiesService, 
timeseriesService, aggregatesService, breakdownsService, scoresService, 
anomalyFunctionService, callgraphService, activeTab, setupMode } =
-        getProperties(this, 'context', 'selectedUrns', 'sizeMetricUrns', 
'entitiesService', 'timeseriesService', 'aggregatesService', 
'breakdownsService', 'scoresService', 'anomalyFunctionService', 
'callgraphService', 'activeTab', 'setupMode');
-      if (!context || !selectedUrns) {
-        return;
-      }
-
-      if (setupMode === ROOTCAUSE_SETUP_MODE_CONTEXT) {
-        return;
-      }
-
-      //
-      // entities
-      //
-      const entitiesUrns = new Set([...selectedUrns, ...context.urns, 
...context.anomalyUrns]);
-      entitiesService.request(context, entitiesUrns);
-
-      //
-      // related metrics
-      //
-      const anomalyMetricUrns = new Set();
-      const relatedMetricUrns = new Set();
-
-      if (activeTab === ROOTCAUSE_TAB_METRICS
-          || activeTab === ROOTCAUSE_TAB_TREND) {
-        const entities = get(this, 'entitiesService.entities'); // cache may 
be stale, fetch directly
-        filterPrefix(Object.keys(entities), 'thirdeye:metric:').forEach(urn => 
relatedMetricUrns.add(urn));
-      }
+  _contextObserver: observer('context', 'entities', 'selectedUrns', 
'sizeMetricUrns', 'activeTab', function () {
+    const {
+      context,
+      selectedUrns,
+      sizeMetricUrns,
+      entitiesService,
+      timeseriesService,
+      aggregatesService,
+      breakdownsService,
+      scoresService,
+      anomalyFunctionService,
+      callgraphService,
+      activeTab,
+      setupMode
+    } = getProperties(
+      this,
+      'context',
+      'selectedUrns',
+      'sizeMetricUrns',
+      'entitiesService',
+      'timeseriesService',
+      'aggregatesService',
+      'breakdownsService',
+      'scoresService',
+      'anomalyFunctionService',
+      'callgraphService',
+      'activeTab',
+      'setupMode'
+    );
+    if (!context || !selectedUrns) {
+      return;
+    }
 
-      if (context.anomalyUrns.size > 0) {
-        filterPrefix(context.anomalyUrns, 'thirdeye:metric:').forEach(urn => 
anomalyMetricUrns.add(urn));
-      }
+    if (setupMode === ROOTCAUSE_SETUP_MODE_CONTEXT) {
+      return;
+    }
 
-      //
-      // timeseries
-      //
-      const timeseriesUrns = new Set(selectedUrns);
+    //
+    // entities
+    //
+    const entitiesUrns = new Set([...selectedUrns, ...context.urns, 
...context.anomalyUrns]);
+    entitiesService.request(context, entitiesUrns);
+
+    //
+    // related metrics
+    //
+    const anomalyMetricUrns = new Set();
+    const relatedMetricUrns = new Set();
+
+    if (activeTab === ROOTCAUSE_TAB_METRICS || activeTab === 
ROOTCAUSE_TAB_TREND) {
+      const entities = get(this, 'entitiesService.entities'); // cache may be 
stale, fetch directly
+      filterPrefix(Object.keys(entities), 'thirdeye:metric:').forEach((urn) => 
relatedMetricUrns.add(urn));
+    }
 
-      if (activeTab === ROOTCAUSE_TAB_TREND) {
-        [...relatedMetricUrns].forEach(urn => {
-          timeseriesUrns.add(toCurrentUrn(urn));
-          timeseriesUrns.add(toBaselineUrn(urn));
-        });
-      }
+    if (context.anomalyUrns.size > 0) {
+      filterPrefix(context.anomalyUrns, 'thirdeye:metric:').forEach((urn) => 
anomalyMetricUrns.add(urn));
+    }
 
-      timeseriesService.request(context, timeseriesUrns);
-
-      //
-      // anomaly function baselines
-      //
-      const anomalyFunctionUrns = filterPrefix(context.anomalyUrns, 
'thirdeye:event:anomaly');
-      anomalyFunctionService.request(context, new Set(anomalyFunctionUrns));
-
-      //
-      // breakdowns
-      //
-      if (activeTab === ROOTCAUSE_TAB_DIMENSIONS) {
-        const metricUrns = new Set(filterPrefix(context.urns, 
'thirdeye:metric:'));
-        const currentUrns = [...metricUrns].map(toCurrentUrn);
-        const baselineUrns = [...metricUrns].map(toBaselineUrn);
-        const sizeMetricCurrentUrns = [...sizeMetricUrns].map(toCurrentUrn);
-        breakdownsService.request(context, new 
Set(currentUrns.concat(baselineUrns).concat(sizeMetricCurrentUrns)));
-      }
+    //
+    // timeseries
+    //
+    const timeseriesUrns = new Set(selectedUrns);
 
-      //
-      // scores
-      //
-      if (activeTab === ROOTCAUSE_TAB_METRICS) {
-        const scoresUrns = new Set(relatedMetricUrns);
-        scoresService.request(context, new Set(scoresUrns));
-      }
+    if (activeTab === ROOTCAUSE_TAB_TREND) {
+      [...relatedMetricUrns].forEach((urn) => {
+        timeseriesUrns.add(toCurrentUrn(urn));
+        timeseriesUrns.add(toBaselineUrn(urn));
+      });
+    }
 
-      //
-      // aggregates
-      //
-      const offsets = ['current', 'baseline', 'wo1w', 'wo2w'];
-      const offsetUrns = [...relatedMetricUrns]
-        .map(urn => [].concat(offsets.map(offset => toOffsetUrn(urn, offset))))
-        .reduce((agg, l) => agg.concat(l), []);
-
-      const anomalyOffsets = ['current', 'baseline', 'wo1w', 'wo2w', 'wo3w', 
'wo4w'];
-      const anomalyOffsetUrns = [...anomalyMetricUrns]
-        .map(urn => [].concat(anomalyOffsets.map(offset => toOffsetUrn(urn, 
offset))))
-        .reduce((agg, l) => agg.concat(l), []);
-
-      //
-      // call graph
-      //
-      if (activeTab === ROOTCAUSE_SERVICE_CALLGRAPH) {
-        callgraphService.request(context, [...context.urns]);
-      }
+    timeseriesService.request(context, timeseriesUrns);
+
+    //
+    // anomaly function baselines
+    //
+    const anomalyFunctionUrns = filterPrefix(context.anomalyUrns, 
'thirdeye:event:anomaly');
+    anomalyFunctionService.request(context, new Set(anomalyFunctionUrns));
+
+    //
+    // breakdowns
+    //
+    if (activeTab === ROOTCAUSE_TAB_DIMENSIONS) {
+      const metricUrns = new Set(filterPrefix(context.urns, 
'thirdeye:metric:'));
+      const currentUrns = [...metricUrns].map(toCurrentUrn);
+      const baselineUrns = [...metricUrns].map(toBaselineUrn);
+      const sizeMetricCurrentUrns = [...sizeMetricUrns].map(toCurrentUrn);
+      breakdownsService.request(context, new 
Set(currentUrns.concat(baselineUrns).concat(sizeMetricCurrentUrns)));
+    }
 
-      aggregatesService.request(context, new Set([...offsetUrns, 
...anomalyOffsetUrns]));
+    //
+    // scores
+    //
+    if (activeTab === ROOTCAUSE_TAB_METRICS) {
+      const scoresUrns = new Set(relatedMetricUrns);
+      scoresService.request(context, new Set(scoresUrns));
+    }
 
+    //
+    // aggregates
+    //
+    const offsets =
+      context.compareMode === 'forecast'
+        ? ['current', 'baseline', 'yoy', 'upper', 'lower']

Review comment:
       Will the `upper` and `lower` make separate calls to get the boundary 
values? I would prefer to make the boundary values be returned with the 
forecast values in one call because they are generated by the forecast provider 
together. I think it is quite expensive to run it 3 times.




----------------------------------------------------------------
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



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

Reply via email to