Allow Webservice to Configure Metric Long Name

CHANGELOG.md

@@ -10,6 +10,11 @@ Current
 
 ### Added:
 
+- [Allow Webservice to Configure Metric Long Name](https://github.com/yahoo/fili/pull/492)
+    * Logical metric needs more config-richness to not just configure metric name, but also metric long name,
+      description, etc. MetricInstance is now created by accepting a LogicalMetricInfo which contains all those
+      informations, instead of accepting a metric name.
+
 - [Enable search provider to hot-swap index and key value store to hot-swap store location](https://github.com/yahoo/fili/pull/522)
     * Add new default method to [`SearchProvider`](./fili-core/src/main/java/com/yahoo/bard/webservice/data/dimension/SearchProvider.java)
       interface in order to support hot-swapping index.

fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/MetricInstance.java

@@ -5,6 +5,7 @@
 import com.yahoo.bard.webservice.data.config.metric.makers.MetricMaker;
 import com.yahoo.bard.webservice.data.config.names.FieldName;
 import com.yahoo.bard.webservice.data.metric.LogicalMetric;
+import com.yahoo.bard.webservice.data.metric.LogicalMetricInfo;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -23,7 +24,7 @@
  */
 public class MetricInstance {
 
-    private final String metricName;
+    private final LogicalMetricInfo logicalMetricInfo;
     private final List<String> dependencyMetricNames;
     private final MetricMaker maker;
 
@@ -34,9 +35,27 @@ public class MetricInstance {
      * @param maker  The Metric Maker that creates the actual Logical Metric
      * @param dependencyMetricNames  The names of metrics either in the dictionary or raw druid metrics that this
      * Logical Metric depends on
+     *
+     * @deprecated logical metric needs more config-richness to not just configure metric name, but also metric long
+     * name, description, etc. Use {@link #MetricInstance(LogicalMetricInfo, MetricMaker, String...)} instead.
      */
+    @Deprecated
     public MetricInstance(String metricName, MetricMaker maker, String... dependencyMetricNames) {
-        this.metricName = metricName;
+        this.logicalMetricInfo = new LogicalMetricInfo(metricName);
+        this.maker = maker;
+        this.dependencyMetricNames = Arrays.asList(dependencyMetricNames);
+    }
+
+    /**
+     * Construct a MetricInstance from Strings with a list of dependencyMetricNames.
+     *
+     * @param logicalMetricInfo  Logical metric info provider
+     * @param maker  The Metric Maker that creates the actual Logical Metric
+     * @param dependencyMetricNames  The names of metrics either in the dictionary or raw druid metrics that this
+     * Logical Metric depends on
+     */
+    public MetricInstance(LogicalMetricInfo logicalMetricInfo, MetricMaker maker, String... dependencyMetricNames) {
+        this.logicalMetricInfo = logicalMetricInfo;
         this.maker = maker;
         this.dependencyMetricNames = Arrays.asList(dependencyMetricNames);
     }
@@ -47,9 +66,29 @@ public MetricInstance(String metricName, MetricMaker maker, String... dependency
      * @param metricName  The name of the Logical Metric when it's in the metric dictionary
      * @param maker  The Metric Maker that creates the actual Logical Metric
      * @param dependencyFields  The field names that this Logical Metric depends on
+     *
+     * @deprecated logical metric needs more config-richness to not just configure metric name, but also metric long
+     * name, description, etc. Use {@link #MetricInstance(LogicalMetricInfo, MetricMaker, FieldName...)} instead.
      */
+    @Deprecated
     public MetricInstance(FieldName metricName, MetricMaker maker, FieldName... dependencyFields) {
-        this.metricName = metricName.asName();
+        this.logicalMetricInfo = new LogicalMetricInfo(metricName.asName());
+        this.maker = maker;
+        this.dependencyMetricNames = new ArrayList<>();
+        for (FieldName fieldName : dependencyFields) {
+            this.dependencyMetricNames.add(fieldName.asName());
+        }
+    }
+
+    /**
+     * Construct a MetricInstance from FieldNames with a list of dependencyFields.
+     *
+     * @param logicalMetricInfo  Logical metric info provider
+     * @param maker  The Metric Maker that creates the actual Logical Metric
+     * @param dependencyFields  The field names that this Logical Metric depends on
+     */
+    public MetricInstance(LogicalMetricInfo logicalMetricInfo, MetricMaker maker, FieldName... dependencyFields) {
+        this.logicalMetricInfo = logicalMetricInfo;
         this.maker = maker;
         this.dependencyMetricNames = new ArrayList<>();
         for (FieldName fieldName : dependencyFields) {
@@ -58,7 +97,7 @@ public MetricInstance(FieldName metricName, MetricMaker maker, FieldName... depe
     }
 
     public String getMetricName() {
-        return metricName;
+        return logicalMetricInfo.getName();
     }
 
     public List<String> getDependencyMetricNames() {
@@ -77,7 +116,7 @@ public MetricMaker getMaker() {
      */
     public MetricInstance withName(String metricName) {
         return new MetricInstance(
-                metricName,
+                logicalMetricInfo,
                 maker,
                 dependencyMetricNames.toArray(new String[dependencyMetricNames.size()])
         );
@@ -91,7 +130,7 @@ public MetricInstance withName(String metricName) {
      */
     public MetricInstance withMaker(MetricMaker maker) {
         return new MetricInstance(
-                metricName,
+                logicalMetricInfo,
                 maker,
                 dependencyMetricNames.toArray(new String[dependencyMetricNames.size()])
         );
@@ -105,7 +144,7 @@ public MetricInstance withMaker(MetricMaker maker) {
      */
     public MetricInstance withDependencyMetricNames(List<String> dependencyMetricNames) {
         return new MetricInstance(
-                metricName,
+                logicalMetricInfo,
                 maker,
                 dependencyMetricNames.toArray(new String[dependencyMetricNames.size()])
         );
@@ -117,6 +156,6 @@ public MetricInstance withDependencyMetricNames(List<String> dependencyMetricNam
      * @return The LogicalMetric with the provided name, using the given maker, that depends on the given metrics.
      */
     public LogicalMetric make() {
-        return maker.make(metricName, dependencyMetricNames);
+        return maker.make(logicalMetricInfo, dependencyMetricNames);
     }
 }

fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java

@@ -7,6 +7,7 @@
 import static com.yahoo.bard.webservice.druid.util.FieldConverterSupplier.sketchConverter;
 
 import com.yahoo.bard.webservice.data.metric.LogicalMetric;
+import com.yahoo.bard.webservice.data.metric.LogicalMetricInfo;
 import com.yahoo.bard.webservice.data.metric.MetricDictionary;
 import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery;
 import com.yahoo.bard.webservice.data.time.ZonelessTimeGrain;
@@ -71,7 +72,7 @@ public AggregationAverageMaker(MetricDictionary metrics, ZonelessTimeGrain inner
     }
 
     @Override
-    protected LogicalMetric makeInner(String metricName, List<String> dependentMetrics) {
+    protected LogicalMetric makeInner(LogicalMetricInfo logicalMetricInfo, List<String> dependentMetrics) {
         // Get the Metric that is being averaged over
         LogicalMetric dependentMetric = metrics.get(dependentMetrics.get(0));
 
@@ -80,9 +81,9 @@ protected LogicalMetric makeInner(String metricName, List<String> dependentMetri
 
         // Build the TemplateDruidQuery for the metric
         TemplateDruidQuery innerQuery = buildInnerQuery(sourceMetric, dependentMetric.getTemplateDruidQuery());
-        TemplateDruidQuery outerQuery = buildOuterQuery(metricName, sourceMetric, innerQuery);
+        TemplateDruidQuery outerQuery = buildOuterQuery(logicalMetricInfo.getName(), sourceMetric, innerQuery);
 
-        return new LogicalMetric(outerQuery, NO_OP_MAPPER, metricName);
+        return new LogicalMetric(outerQuery, NO_OP_MAPPER, logicalMetricInfo);
     }
 
     /**

fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java

@@ -3,6 +3,7 @@
 package com.yahoo.bard.webservice.data.config.metric.makers;
 
 import com.yahoo.bard.webservice.data.metric.LogicalMetric;
+import com.yahoo.bard.webservice.data.metric.LogicalMetricInfo;
 import com.yahoo.bard.webservice.data.metric.MetricDictionary;
 import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery;
 import com.yahoo.bard.webservice.data.metric.mappers.ColumnMapper;
@@ -89,7 +90,7 @@ public ArithmeticMaker(MetricDictionary metricDictionary, ArithmeticPostAggregat
     }
 
     @Override
-    protected LogicalMetric makeInner(String metricName, List<String> dependentMetrics) {
+    protected LogicalMetric makeInner(LogicalMetricInfo logicalMetricInfo, List<String> dependentMetrics) {
         // Get the ArithmeticPostAggregation operands from the dependent metrics
         List<PostAggregation> operands = dependentMetrics.stream()
                 .map(metrics::get)
@@ -99,13 +100,17 @@ protected LogicalMetric makeInner(String metricName, List<String> dependentMetri
 
         // Create the ArithmeticPostAggregation
         Set<PostAggregation> postAggregations = Collections.singleton(new ArithmeticPostAggregation(
-                metricName,
+                logicalMetricInfo.getName(),
                 function,
                 operands
         ));
 
         TemplateDruidQuery query = getMergedQuery(dependentMetrics).withPostAggregations(postAggregations);
-        return new LogicalMetric(query, resultSetMapperSupplier.apply(metricName), metricName);
+        return new LogicalMetric(
+                query,
+                resultSetMapperSupplier.apply(logicalMetricInfo.getName()),
+                logicalMetricInfo
+        );
     }
 
     @Override

fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/CardinalityMaker.java

@@ -5,6 +5,7 @@
 import com.yahoo.bard.webservice.data.dimension.Dimension;
 import com.yahoo.bard.webservice.data.dimension.DimensionDictionary;
 import com.yahoo.bard.webservice.data.metric.LogicalMetric;
+import com.yahoo.bard.webservice.data.metric.LogicalMetricInfo;
 import com.yahoo.bard.webservice.data.metric.MetricDictionary;
 import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery;
 import com.yahoo.bard.webservice.druid.model.aggregation.Aggregation;
@@ -66,14 +67,16 @@ protected void assertDependentMetricsExist(List<String> dependentDimensions) {
     }
 
     @Override
-    protected LogicalMetric makeInner(String metricName, List<String> dependentDimensions) {
+    protected LogicalMetric makeInner(LogicalMetricInfo logicalMetricInfo, List<String> dependentDimensions) {
         Set<Dimension> dimensions = dependentDimensions.stream()
                 .map(dimensionDictionary::findByApiName)
                 .collect(Collectors.toSet());
 
-        Set<Aggregation> aggs = Collections.singleton(new CardinalityAggregation(metricName, dimensions, byRow));
+        Set<Aggregation> aggs = Collections.singleton(
+                new CardinalityAggregation(logicalMetricInfo.getName(), dimensions, byRow)
+        );
 
-        return new LogicalMetric(new TemplateDruidQuery(aggs, Collections.emptySet()), NO_OP_MAPPER, metricName);
+        return new LogicalMetric(new TemplateDruidQuery(aggs, Collections.emptySet()), NO_OP_MAPPER, logicalMetricInfo);
     }
 
     @Override

fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ConstantMaker.java

@@ -3,6 +3,7 @@
 package com.yahoo.bard.webservice.data.config.metric.makers;
 
 import com.yahoo.bard.webservice.data.metric.LogicalMetric;
+import com.yahoo.bard.webservice.data.metric.LogicalMetricInfo;
 import com.yahoo.bard.webservice.data.metric.MetricDictionary;
 import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery;
 import com.yahoo.bard.webservice.druid.model.postaggregation.ConstantPostAggregation;
@@ -42,7 +43,8 @@ public LogicalMetric make(String metricName, List<String> dependentMetrics) {
     }
 
     @Override
-    protected LogicalMetric makeInner(String metricName, List<String> dependentMetrics) {
+    protected LogicalMetric makeInner(LogicalMetricInfo logicalMetricInfo, List<String> dependentMetrics) {
+        String metricName = logicalMetricInfo.getName();
         try {
             Set<PostAggregation> postAggregations = Collections.singleton(new ConstantPostAggregation(
                     metricName,
@@ -52,7 +54,7 @@ protected LogicalMetric makeInner(String metricName, List<String> dependentMetri
             return new LogicalMetric(
                     new TemplateDruidQuery(Collections.emptySet(), postAggregations),
                     NO_OP_MAPPER,
-                    metricName
+                    logicalMetricInfo
             );
         } catch (NumberFormatException nfe) {
             String message = String.format(

fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/CountMaker.java

@@ -3,6 +3,7 @@
 package com.yahoo.bard.webservice.data.config.metric.makers;
 
 import com.yahoo.bard.webservice.data.metric.LogicalMetric;
+import com.yahoo.bard.webservice.data.metric.LogicalMetricInfo;
 import com.yahoo.bard.webservice.data.metric.MetricDictionary;
 import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery;
 import com.yahoo.bard.webservice.druid.model.aggregation.CountAggregation;
@@ -27,13 +28,13 @@ public CountMaker(MetricDictionary metricDictionary) {
     }
 
     @Override
-    protected LogicalMetric makeInner(String metricName, List<String> dependentMetrics) {
+    protected LogicalMetric makeInner(LogicalMetricInfo logicalMetricInfo, List<String> dependentMetrics) {
         TemplateDruidQuery query = new TemplateDruidQuery(
-                Collections.singleton(new CountAggregation(metricName)),
+                Collections.singleton(new CountAggregation(logicalMetricInfo.getName())),
                 Collections.emptySet()
         );
 
-        return new LogicalMetric(query, NO_OP_MAPPER, metricName);
+        return new LogicalMetric(query, NO_OP_MAPPER, logicalMetricInfo);
     }
 
     @Override

fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/FilteredAggregationMaker.java

@@ -4,6 +4,7 @@
 package com.yahoo.bard.webservice.data.config.metric.makers;
 
 import com.yahoo.bard.webservice.data.metric.LogicalMetric;
+import com.yahoo.bard.webservice.data.metric.LogicalMetricInfo;
 import com.yahoo.bard.webservice.data.metric.MetricDictionary;
 import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery;
 import com.yahoo.bard.webservice.druid.model.MetricField;
@@ -51,13 +52,13 @@ public FilteredAggregationMaker(MetricDictionary metricDictionary, Filter filter
     }
 
     @Override
-    protected LogicalMetric makeInner(String metricName, List<String> dependentMetrics) {
+    protected LogicalMetric makeInner(LogicalMetricInfo logicalMetricInfo, List<String> dependentMetrics) {
         LogicalMetric sourceMetric = metricDictionary.get(dependentMetrics.get(0));
 
         Aggregation sourceAggregation = assertDependentIsAggregationMetric(sourceMetric);
 
         FilteredAggregation filteredAggregation = new FilteredAggregation(
-                metricName,
+                logicalMetricInfo.getName(),
                 sourceAggregation,
                 filter
         );
@@ -69,7 +70,7 @@ protected LogicalMetric makeInner(String metricName, List<String> dependentMetri
                         sourceMetric.getTemplateDruidQuery().getInnerQuery().orElse(null)
                 ),
                 sourceMetric.getCalculation(),
-                metricName
+                logicalMetricInfo
         );
     }
 

fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/MetricMaker.java

@@ -3,6 +3,7 @@
 package com.yahoo.bard.webservice.data.config.metric.makers;
 
 import com.yahoo.bard.webservice.data.metric.LogicalMetric;
+import com.yahoo.bard.webservice.data.metric.LogicalMetricInfo;
 import com.yahoo.bard.webservice.data.metric.MetricDictionary;
 import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery;
 import com.yahoo.bard.webservice.data.metric.mappers.NoOpResultSetMapper;
@@ -63,16 +64,37 @@ public MetricMaker(MetricDictionary metrics) {
      * @param dependentMetrics  Metrics this metric depends on
      *
      * @return The new logicalMetric
+     *
+     * @deprecated logical metric needs more config-richness to not just configure metric name, but also metric long
+     * name, description, etc. Use {@link #make(LogicalMetricInfo, List)} instead.
      */
+    @Deprecated
     public LogicalMetric make(String metricName, List<String> dependentMetrics) {
+        return make(new LogicalMetricInfo(metricName), dependentMetrics);
+    }
+
+    /**
+     * Make the metric.
+     * <p>
+     * This method also sanity-checks the dependent metrics to make sure that they
+     * are metrics we have built and are in the metric dictionary.
+     * <p>
+     * Also sanity-checks that the number of dependent metrics are correct for the maker.
+     *
+     * @param logicalMetricInfo  Logical metric info provider
+     * @param dependentMetrics  Metrics this metric depends on
+     *
+     * @return The new logicalMetric
+     */
+    public LogicalMetric make(LogicalMetricInfo logicalMetricInfo, List<String> dependentMetrics) {
         // Check that all of the dependent metrics are in the dictionary
         assertDependentMetricsExist(dependentMetrics);
 
         // Check that we have the right number of metrics
-        assertRequiredDependentMetricCount(metricName, dependentMetrics);
+        assertRequiredDependentMetricCount(logicalMetricInfo.getName(), dependentMetrics);
 
         // Have the subclass actually build the metric
-        return this.makeInner(metricName, dependentMetrics);
+        return this.makeInner(logicalMetricInfo, dependentMetrics);
     }
 
     /**
@@ -132,9 +154,32 @@ public LogicalMetric make(String metricName, String dependentMetric) {
      * @param metricName  Name for the metric we're making
      * @param dependentMetrics  Metrics this metric depends on
      *
-     * @return The new logicalMetric
+     * @return the new logicalMetric
+     *
+     * @deprecated logical metric needs more config-richness to not just configure metric name, but also metric long
+     * name, description, etc. Use {@link #makeInner(LogicalMetricInfo, List)} instead.
+     */
+    @Deprecated
+    protected LogicalMetric makeInner(String metricName, List<String> dependentMetrics) {
+        return makeInner(new LogicalMetricInfo(metricName), dependentMetrics);
+    }
+
+    /**
+     * Delegated to for actually making the metric.
+     *
+     * @param logicalMetricInfo  Logical metric info provider
+     * @param dependentMetrics  Metrics this metric depends on
+     *
+     * @return the new logicalMetric
      */
-    protected abstract LogicalMetric makeInner(String metricName, List<String> dependentMetrics);
+    protected LogicalMetric makeInner(LogicalMetricInfo logicalMetricInfo, List<String> dependentMetrics) {
+        String message = String.format(
+                "Current implementation of MetricMaker '%s' does not support makeInner operation",
+                this.getClass().getSimpleName()
+        );
+        LoggerFactory.getLogger(MetricMaker.class).error(message);
+        throw new UnsupportedOperationException(message);
+    }
 
     /**
      * Get the number of dependent metrics the maker requires for making the metric.