Allow any percentile to be computed or passed through CAGG if so requested by MAD. (#87)

Author: vjkoskela

src/main/java/com/arpnetworking/tsdcore/statistics/StatisticFactory.java

@@ -17,19 +17,18 @@
 
 import com.arpnetworking.utility.InterfaceDatabase;
 import com.arpnetworking.utility.ReflectionsDatabase;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Modifier;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.ConcurrentMap;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * Creates statistics.
@@ -60,31 +59,36 @@ public Statistic getStatistic(final String name) {
      * @return A new <code>Statistic</code>.
      */
     public Optional<Statistic> tryGetStatistic(final String name) {
-        return Optional.ofNullable(STATISTICS_BY_NAME_AND_ALIAS.get(name));
-    }
-
-    /**
-     * Get all registered <code>Statistic</code> instances.
-     *
-     * @return A new <code>Statistic</code>.
-     */
-    public ImmutableSet<Statistic> getAllStatistics() {
-        return ALL_STATISTICS;
+        final Optional<Statistic> registeredStatistic =
+                Optional.ofNullable(STATISTICS_BY_NAME_AND_ALIAS.get(name));
+        if (!registeredStatistic.isPresent()) {
+            final Matcher matcher = PERCENTILE_STATISTIC_PATTERN.matcher(name);
+            if (matcher.matches()) {
+                try {
+                    final String percentileString = matcher.group("percentile").replace('p', '.');
+                    final double percentile = Double.parseDouble(percentileString);
+                    final Statistic statistic = new TPStatistic(percentile);
+                    checkedPut(STATISTICS_BY_NAME_AND_ALIAS, statistic);
+                    return Optional.of(statistic);
+                } catch (final NumberFormatException e) {
+                    LOGGER.error(String.format(
+                            "Invalid percentile statistic; name=%s",
+                            name));
+                    return registeredStatistic;
+                }
+            }
+        }
+        return registeredStatistic;
     }
 
-    /**
-     * Creates a statistic from a name.
-     *
-     * @param statistic The name of the desired statistic.
-     * @return A new <code>Statistic</code>.
-     * @deprecated Use <code>getStatistic</code> instead.
-     */
-    @Deprecated
-    public Optional<Statistic> createStatistic(final String statistic) {
-        return Optional.ofNullable(STATISTICS_BY_NAME_AND_ALIAS.get(statistic));
+    private static void checkedPut(final ConcurrentMap<String, Statistic> map, final Statistic statistic) {
+        checkedPut(map, statistic, statistic.getName());
+        for (final String alias : statistic.getAliases()) {
+            checkedPut(map, statistic, alias);
+        }
     }
 
-    private static void checkedPut(final Map<String, Statistic> map, final Statistic statistic, final String key) {
+    private static void checkedPut(final ConcurrentMap<String, Statistic> map, final Statistic statistic, final String key) {
         final Statistic existingStatistic =  map.get(key);
         if (existingStatistic != null) {
             if (!existingStatistic.equals(statistic)) {
@@ -99,15 +103,14 @@ private static void checkedPut(final Map<String, Statistic> map, final Statistic
         map.put(key, statistic);
     }
 
-    private static final ImmutableMap<String, Statistic> STATISTICS_BY_NAME_AND_ALIAS;
-    private static final ImmutableSet<Statistic> ALL_STATISTICS;
+    private static final Pattern PERCENTILE_STATISTIC_PATTERN = Pattern.compile("^[t]?p(?<percentile>[0-9]+(?:(\\.|p)[0-9]+)?)$");
+    private static final ConcurrentMap<String, Statistic> STATISTICS_BY_NAME_AND_ALIAS;
     private static final InterfaceDatabase INTERFACE_DATABASE = ReflectionsDatabase.newInstance();
     private static final Logger LOGGER = LoggerFactory.getLogger(StatisticFactory.class);
 
     static {
         // NOTE: Do not put log messages in static blocks since they can lock the logger thread!
-        final Map<String, Statistic> statisticByNameAndAlias = Maps.newHashMap();
-        final Set<Statistic> allStatistics = Sets.newHashSet();
+        final ConcurrentMap<String, Statistic> statisticByNameAndAlias = Maps.newConcurrentMap();
         final Set<Class<? extends Statistic>> statisticClasses = INTERFACE_DATABASE.findClassesWithInterface(Statistic.class);
         for (final Class<? extends Statistic> statisticClass : statisticClasses) {
             if (!statisticClass.isInterface() && !Modifier.isAbstract(statisticClass.getModifiers())) {
@@ -116,19 +119,13 @@ private static void checkedPut(final Map<String, Statistic> map, final Statistic
                     if (!constructor.isAccessible()) {
                         constructor.setAccessible(true);
                     }
-                    final Statistic statistic = constructor.newInstance();
-                    allStatistics.add(statistic);
-                    checkedPut(statisticByNameAndAlias, statistic, statistic.getName());
-                    for (final String alias : statistic.getAliases()) {
-                        checkedPut(statisticByNameAndAlias, statistic, alias);
-                    }
+                    checkedPut(statisticByNameAndAlias, constructor.newInstance());
                 } catch (final InvocationTargetException | NoSuchMethodException
                         | InstantiationException | IllegalAccessException e) {
                     LOGGER.warn(String.format("Unable to load statistic; class=%s", statisticClass), e);
                 }
             }
         }
-        STATISTICS_BY_NAME_AND_ALIAS = ImmutableMap.copyOf(statisticByNameAndAlias);
-        ALL_STATISTICS = ImmutableSet.copyOf(allStatistics);
+        STATISTICS_BY_NAME_AND_ALIAS = statisticByNameAndAlias;
     }
 }

src/main/java/com/arpnetworking/tsdcore/statistics/TP75Statistic.java

@@ -36,7 +36,7 @@
  *
  * @author Brandon Arp (brandon dot arp at inscopemetrics dot com)
  */
-public abstract class TPStatistic extends BaseStatistic implements OrderedStatistic {
+public class TPStatistic extends BaseStatistic implements OrderedStatistic {
 
     /**
      * Accessor for the percentile from 0 to 100 (inclusive).
@@ -85,11 +85,11 @@ public Quantity calculateAggregations(final List<AggregatedData> aggregations) {
     }
 
     /**
-     * Protected constructor.
+     * Public constructor.
      *
-     * @param percentile The percentile value to compute.
+     * @param percentile the percentile value to compute
      */
-    protected TPStatistic(final double percentile) {
+    public TPStatistic(final double percentile) {
         _percentile = percentile;
         _defaultName = "tp" + FORMAT.format(_percentile);
         _aliases = Sets.newHashSet();

src/main/java/com/arpnetworking/tsdcore/statistics/TP90Statistic.java

@@ -47,11 +47,11 @@ public static Collection<Object[]> createParameters() {
                 a(l("count", "n"), CountStatistic.class),
                 a(l("p0", "tp0", "min"), MinStatistic.class),
                 a(l("p50", "tp50", "median"), MedianStatistic.class),
-                a(l("p75", "tp75"), TP75Statistic.class),
-                a(l("p90", "tp90"), TP90Statistic.class),
-                a(l("p95", "tp95"), TP95Statistic.class),
-                a(l("p99", "tp99"), TP99Statistic.class),
-                a(l("p99.9", "tp99.9", "p99p9", "tp99p9"), TP99p9Statistic.class),
+                a(l("p75", "tp75"), TPStatistic.class),
+                a(l("p90", "tp90"), TPStatistic.class),
+                a(l("p95", "tp95"), TPStatistic.class),
+                a(l("p99", "tp99"), TPStatistic.class),
+                a(l("p99.9", "tp99.9", "p99p9", "tp99p9"), TPStatistic.class),
                 a(l("p100", "tp100", "max"), MaxStatistic.class)
         );
     }

src/main/java/com/arpnetworking/tsdcore/statistics/TP95Statistic.java

@@ -203,9 +203,9 @@ private boolean areClose(final Quantity expected, final Quantity actual) {
     private static final List<Double> ONE_TO_FIVE = Lists.newArrayList(1d, 2d, 3d, 4d, 5d);
     private static final StatisticFactory STATISTIC_FACTORY = new StatisticFactory();
     private static final Statistic HISTOGRAM_STATISTIC = STATISTIC_FACTORY.getStatistic("histogram");
-    private static final TP75Statistic TP75_STATISTIC = (TP75Statistic) STATISTIC_FACTORY.getStatistic("tp75");
-    private static final TP90Statistic TP90_STATISTIC = (TP90Statistic) STATISTIC_FACTORY.getStatistic("tp90");
-    private static final TP95Statistic TP95_STATISTIC = (TP95Statistic) STATISTIC_FACTORY.getStatistic("tp95");
-    private static final TP99Statistic TP99_STATISTIC = (TP99Statistic) STATISTIC_FACTORY.getStatistic("tp99");
-    private static final TP99p9Statistic TP99P9_STATISTIC = (TP99p9Statistic) STATISTIC_FACTORY.getStatistic("tp99p9");
+    private static final TPStatistic TP75_STATISTIC = (TPStatistic) STATISTIC_FACTORY.getStatistic("tp75");
+    private static final TPStatistic TP90_STATISTIC = (TPStatistic) STATISTIC_FACTORY.getStatistic("tp90");
+    private static final TPStatistic TP95_STATISTIC = (TPStatistic) STATISTIC_FACTORY.getStatistic("tp95");
+    private static final TPStatistic TP99_STATISTIC = (TPStatistic) STATISTIC_FACTORY.getStatistic("tp99");
+    private static final TPStatistic TP99P9_STATISTIC = (TPStatistic) STATISTIC_FACTORY.getStatistic("tp99p9");
 }