Unified units and telemetry units. (#60)

Author: vjkoskela

README.md

@@ -105,9 +105,13 @@ For example:
     "serviceName": "MyApplication",
     "sources":
     [
+        {                                                                 
+            "type": "com.arpnetworking.metrics.common.sources.ClientHttpSourceV1
+            "name": "my_application_http_source"                                   
+        },
         {
             "type": "com.arpnetworking.metrics.common.sources.FileSource",
-            "name": "my_application_source",
+            "name": "my_application_file_source",
             "filePath": "/var/log/my-application-query.log",
             "parser": {
                 "type": "com.arpnetworking.metrics.mad.parsers.QueryLogParser"

src/main/java/com/arpnetworking/http/Routes.java

@@ -108,7 +108,10 @@ public Flow<HttpRequest, HttpResponse, NotUsed> flow() {
     @Override
     public CompletionStage<HttpResponse> apply(final HttpRequest request) {
         final Stopwatch requestTimer = Stopwatch.createStarted();
-        _metrics.recordCounter(createMetricName(request, BODY_SIZE_METRIC), request.entity().getContentLengthOption().orElse(0L));
+        _metrics.recordGauge(
+                createMetricName(request, BODY_SIZE_METRIC),
+                request.entity().getContentLengthOption().orElse(0L),
+                Optional.of(Units.BYTE));
         // TODO(vkoskela): Add a request UUID and include in MDC. [MAI-462]
         LOGGER.trace()
                 .setEvent("http.in.start")

src/main/java/com/arpnetworking/metrics/mad/parsers/CollectdJsonToRecordParser.java

@@ -101,6 +101,7 @@ public List<Record> parse(final HttpRequest request) throws ParsingException {
                     }
                     final String metricName = computeMetricName(plugin, pluginInstance, type, typeInstance, sample.getDsName());
                     final MetricType metricType = mapDsType(sample.getDsType());
+                    // TODO(ville): Support units and normalize
                     final Metric metric = new DefaultMetric.Builder()
                             .setType(metricType)
                             .setValues(Collections.singletonList(

src/main/java/com/arpnetworking/metrics/proxy/models/messages/MetricReport.java

@@ -17,9 +17,13 @@
 package com.arpnetworking.metrics.proxy.models.messages;
 
 import com.arpnetworking.logback.annotations.Loggable;
+import com.arpnetworking.tsdcore.model.Unit;
 import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableList;
 import org.joda.time.DateTime;
 
+import java.util.Optional;
+
 /**
  * Message class to hold data about a metric that should be sent to clients.
  *
@@ -36,6 +40,7 @@ public final class MetricReport {
      * @param statistic name of the statistic
      * @param metric name of the metric
      * @param value value
+     * @param unit unit
      * @param periodStart start of the period
      */
     public MetricReport(
@@ -44,12 +49,14 @@ public MetricReport(
             final String statistic,
             final String metric,
             final double value,
+            final Optional<Unit> unit,
             final DateTime periodStart) {
         _service = service;
         _host = host;
         _statistic = statistic;
         _metric = metric;
         _value = value;
+        _numeratorUnits = unit.map(Unit::toString).map(ImmutableList::of).orElse(ImmutableList.of());
         _periodStart = periodStart;
     }
 
@@ -73,6 +80,14 @@ public double getValue() {
         return _value;
     }
 
+    public ImmutableList<String> getNumeratorUnits() {
+        return _numeratorUnits;
+    }
+
+    public ImmutableList<String> getDenominatorUnits() {
+        return _denominatorUnits;
+    }
+
     public DateTime getPeriodStart() {
         return _periodStart;
     }
@@ -87,6 +102,8 @@ public String toString() {
                 .add("Statistic", _statistic)
                 .add("Metric", _metric)
                 .add("Value", _value)
+                .add("NumeratorUnit", _numeratorUnits)
+                .add("DenominatorUnit", _denominatorUnits)
                 .add("PeriodStart", _periodStart)
                 .toString();
     }
@@ -96,5 +113,7 @@ public String toString() {
     private final String _statistic;
     private final String _metric;
     private final double _value;
+    private final ImmutableList<String> _numeratorUnits;
+    private final ImmutableList<String> _denominatorUnits = ImmutableList.of();
     private final DateTime _periodStart;
 }

src/main/java/com/arpnetworking/metrics/proxy/models/protocol/v2/MetricMessagesProcessor.java

@@ -32,6 +32,7 @@
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
@@ -166,10 +167,18 @@ private void processMetricReport(final MetricReport report) {
         event.put("timestamp", report.getPeriodStart().getMillis());
         event.put("statistic", report.getStatistic());
         event.put("data", report.getValue());
+        event.set("numeratorUnits", createUnitArrayNode(report.getNumeratorUnits()));
+        event.set("denominatorUnits", createUnitArrayNode(report.getDenominatorUnits()));
 
         _connection.sendCommand(COMMAND_REPORT_METRIC, event);
     }
 
+    private ArrayNode createUnitArrayNode(final ImmutableList<String> units) {
+        final ArrayNode unitArrayNode = new ArrayNode(OBJECT_MAPPER.getNodeFactory());
+        units.forEach(unitArrayNode::add);
+        return unitArrayNode;
+    }
+
     private void processMetricsList(final MetricsList metricsList) {
         //TODO(barp): Map with a POJO mapper [MAI-184]
         final ObjectNode dataNode = JsonNodeFactory.instance.objectNode();

src/main/java/com/arpnetworking/tsdcore/model/Quantity.java

@@ -17,18 +17,13 @@
 
 import com.arpnetworking.commons.builder.OvalBuilder;
 import com.arpnetworking.logback.annotations.Loggable;
-import com.google.common.base.Function;
 import com.google.common.base.MoreObjects;
-import com.google.common.collect.Lists;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import net.sf.oval.constraint.NotNull;
 
 import java.io.Serializable;
-import java.util.Collection;
-import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
-import javax.annotation.Nullable;
 
 /**
  * Represents a sample.
@@ -61,7 +56,7 @@ public Quantity add(final Quantity otherQuantity) {
                     this,
                     otherQuantity));
         }
-        if (_unit.equals(otherQuantity._unit)) {
+        if (Objects.equals(_unit, otherQuantity._unit)) {
             return new Quantity(_value + otherQuantity._value, _unit);
         }
         final Unit smallerUnit = _unit.get().getSmallerUnit(otherQuantity.getUnit().get());
@@ -86,7 +81,7 @@ public Quantity subtract(final Quantity otherQuantity) {
                     this,
                     otherQuantity));
         }
-        if (_unit.equals(otherQuantity._unit)) {
+        if (Objects.equals(_unit, otherQuantity._unit)) {
             return new Quantity(_value - otherQuantity._value, _unit);
         }
         final Unit smallerUnit = _unit.get().getSmallerUnit(otherQuantity.getUnit().get());
@@ -122,73 +117,23 @@ public Quantity multiply(final Quantity otherQuantity) {
     public Quantity divide(final Quantity otherQuantity) {
         // TODO(vkoskela): Support division by quantity with unit [2F].
         if (otherQuantity._unit.isPresent()) {
-            throw new UnsupportedOperationException("Compount units not supported yet");
+            throw new UnsupportedOperationException("Compound units not supported yet");
         }
-        if (_unit.equals(otherQuantity._unit)) {
+        if (Objects.equals(_unit, otherQuantity._unit)) {
             return new Quantity(_value / otherQuantity._value, Optional.empty());
         }
         return new Quantity(
                 _value / otherQuantity._value,
                 _unit);
     }
 
-    /**
-     * Convert this <code>Quantity</code> to one in the specified unit. This
-     * <code>Quantity</code> must also have a <code>Unit</code> and it must
-     * be in the same domain as the provided unit.
-     *
-     * @param unit <code>Unit</code> to convert to.
-     * @return <code>Quantity</code> in specified unit.
-     */
-    public Quantity convertTo(final Unit unit) {
-        if (!_unit.isPresent()) {
-            throw new IllegalStateException(String.format(
-                    "Cannot convert a quantity without a unit; this=%s",
-                    this));
-        }
-        if (_unit.get().equals(unit)) {
-            return this;
-        }
-        return new Quantity(
-                unit.convert(_value, _unit.get()),
-                Optional.of(unit));
-    }
-
-    /**
-     * Convert this <code>Quantity</code> to one in the specified optional unit.
-     * Either this <code>Quantity</code> also has a <code>Unit</code> in the
-     * same domain as the provided unit or both units are absent.
-     *
-     * @param unit <code>Optional</code> <code>Unit</code> to convert to.
-     * @return <code>Quantity</code> in specified unit.
-     */
-    public Quantity convertTo(final Optional<Unit> unit) {
-        if (_unit.isPresent() != unit.isPresent()) {
-            throw new IllegalStateException(String.format(
-                    "Units must both be present or absent; quantity=%s unit=%s",
-                    this,
-                    unit));
-        }
-        if (_unit.equals(unit)) {
-            return this;
-        }
-        return new Quantity(
-                unit.get().convert(_value, _unit.get()),
-                unit);
-    }
-
     @Override
     public int compareTo(final Quantity other) {
         if (other._unit.equals(_unit)) {
             return Double.compare(_value, other._value);
-        } else if (other._unit.isPresent() && _unit.isPresent()) {
-            final Unit smallerUnit = _unit.get().getSmallerUnit(other._unit.get());
-            final double convertedValue = smallerUnit.convert(_value, _unit.get());
-            final double otherConvertedValue = smallerUnit.convert(other._value, other._unit.get());
-            return Double.compare(convertedValue, otherConvertedValue);
         }
         throw new IllegalArgumentException(String.format(
-                "Cannot compare a quantity with a unit to a quantity without a unit; this=%s, other=%s",
+                "Cannot compare mismatched units; this=%s, other=%s",
                 this,
                 other));
     }
@@ -222,37 +167,6 @@ public String toString() {
                 .toString();
     }
 
-    /**
-     * Ensures all <code>Quantity</code> instances have the same (including no)
-     * <code>Unit</code>.
-     *
-     * @param quantities <code>Quantity</code> instances to convert.
-     * @return <code>List</code> of <code>Quantity</code> instances with the
-     * same <code>Unit</code> (or no <code>Unit</code>).
-     */
-    public static List<Quantity> unify(final Collection<Quantity> quantities) {
-        // This is a 2-pass operation:
-        // First pass is to grab the smallest unit in the samples
-        // Second pass is to convert everything to that unit
-        Optional<Unit> smallestUnit = Optional.empty();
-        for (final Quantity quantity : quantities) {
-            if (!smallestUnit.isPresent()) {
-                smallestUnit = quantity.getUnit();
-            } else if (quantity.getUnit().isPresent()
-                    && !smallestUnit.get().getSmallerUnit(quantity.getUnit().get()).equals(smallestUnit.get())) {
-                smallestUnit = quantity.getUnit();
-            }
-        }
-
-        final List<Quantity> convertedSamples = Lists.newArrayListWithExpectedSize(quantities.size());
-        final Function<Quantity, Quantity> converter = SampleConverter.to(smallestUnit);
-        for (final Quantity quantity : quantities) {
-            convertedSamples.add(converter.apply(quantity));
-        }
-
-        return convertedSamples;
-    }
-
     private Quantity(final Builder builder) {
         this(builder._value, Optional.ofNullable(builder._unit));
     }
@@ -268,37 +182,6 @@ private Quantity(final double value, final Optional<Unit> unit) {
 
     private static final long serialVersionUID = -6339526234042605516L;
 
-    private static final class SampleConverter implements Function<Quantity, Quantity> {
-
-        public static Function<Quantity, Quantity> to(final Optional<Unit> unit) {
-            if (unit.isPresent()) {
-                return new SampleConverter(unit.get());
-            }
-            return (q) -> q;
-        }
-
-        @Override
-        @Nullable
-        public Quantity apply(@Nullable final Quantity quantity) {
-            if (quantity == null) {
-                return null;
-            }
-            if (!quantity.getUnit().isPresent()) {
-                throw new IllegalArgumentException(String.format("Cannot convert a quantity without unit; sample=%s", quantity));
-            }
-            return new Quantity.Builder()
-                    .setValue(_unit.convert(quantity.getValue(), quantity.getUnit().get()))
-                    .setUnit(_unit)
-                    .build();
-        }
-
-        private SampleConverter(final Unit convertTo) {
-            _unit = convertTo;
-        }
-
-        private final Unit _unit;
-    }
-
     /**
      * <code>Builder</code> implementation for <code>Quantity</code>.
      */
@@ -311,6 +194,17 @@ public Builder() {
             super((java.util.function.Function<Builder, Quantity>) Quantity::new);
         }
 
+        /**
+         * Public constructor.
+         *
+         * @param quantity the <code>Quantity</code> to initialize from
+         */
+        public Builder(final Quantity quantity) {
+            super((java.util.function.Function<Builder, Quantity>) Quantity::new);
+            _value = quantity._value;
+            _unit = quantity._unit.orElse(null);
+        }
+
         /**
          * Set the value. Required. Cannot be null.
          *
@@ -335,12 +229,21 @@ public Builder setUnit(final Unit value) {
 
         @Override
         public Quantity build() {
-            if (_value == null) {
-                throw new IllegalStateException("value cannot be null");
-            }
+            normalize();
             return new Quantity(this);
         }
 
+        private Builder normalize() {
+            if (_value != null && _unit != null) {
+                final Unit defaultUnit = _unit.getType().getDefaultUnit();
+                if (!Objects.equals(_unit, defaultUnit)) {
+                    _value = defaultUnit.convert(_value, _unit);
+                    _unit = defaultUnit;
+                }
+            }
+            return this;
+        }
+
         @NotNull
         private Double _value;
         private Unit _unit;