#646 document implications of negative observations for summaries and histograms

Author: fstab

simpleclient/src/main/java/io/prometheus/client/Histogram.java

@@ -255,6 +255,10 @@ private Child(double[] buckets) {
 
     /**
      * Observe the given amount.
+     * @param amt in most cases amt should be >= 0. Negative values are supported, but you should read
+     *            <a href="https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations">
+     *            https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations</a> for
+     *            implications and alternatives.
      */
     public void observe(double amt) {
       for (int i = 0; i < upperBounds.length; ++i) {
@@ -293,6 +297,10 @@ public Value get() {
   // Convenience methods.
   /**
    * Observe the given amount on the histogram with no labels.
+   * @param amt in most cases amt should be >= 0. Negative values are supported, but you should read
+   *            <a href="https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations">
+   *            https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations</a> for
+   *            implications and alternatives.
    */
   public void observe(double amt) {
     noLabelsChild.observe(amt);

simpleclient/src/main/java/io/prometheus/client/Summary.java

@@ -278,6 +278,10 @@ private Child(List<Quantile> quantiles, long maxAgeSeconds, int ageBuckets) {
 
     /**
      * Observe the given amount.
+     * @param amt in most cases amt should be >= 0. Negative values are supported, but you should read
+     *            <a href="https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations">
+     *            https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations</a> for
+     *            implications and alternatives.
      */
     public void observe(double amt) {
       count.add(1);
@@ -307,6 +311,10 @@ public Value get() {
   // Convenience methods.
   /**
    * Observe the given amount on the summary with no labels.
+   * @param amt in most cases amt should be >= 0. Negative values are supported, but you should read
+   *            <a href="https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations">
+   *            https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations</a> for
+   *            implications and alternatives.
    */
   public void observe(double amt) {
     noLabelsChild.observe(amt);

simpleclient/src/test/java/io/prometheus/client/HistogramTest.java

@@ -39,13 +39,27 @@ public void tearDown() {
   }
 
   private double getCount() {
-    return registry.getSampleValue("nolabels_count").doubleValue();
+    return getCount("nolabels");
   }
+
+  private double getCount(String name) {
+    return registry.getSampleValue(name + "_count").doubleValue();
+  }
+
   private double getSum() {
-    return registry.getSampleValue("nolabels_sum").doubleValue();
+    return getSum("nolabels");
   }
+
+  private double getSum(String name) {
+    return registry.getSampleValue(name + "_sum").doubleValue();
+  }
+
   private double getBucket(double b) {
-    return registry.getSampleValue("nolabels_bucket",
+    return getBucket(b, "nolabels");
+  }
+
+  private double getBucket(double b, String name) {
+    return registry.getSampleValue(name + "_bucket",
         new String[]{"le"},
         new String[]{Collector.doubleToGoString(b)}).doubleValue();
   }
@@ -68,6 +82,30 @@ public void testObserve() {
     assertEquals(2.0, getBucket(Double.POSITIVE_INFINITY), .001);
   }
 
+  @Test
+  // See https://github.com/prometheus/client_java/issues/646
+  public void testNegativeAmount() {
+    Histogram histogram = Histogram.build()
+        .name("histogram")
+        .help("test histogram for negative values")
+        .buckets(-10, -5, 0, 5, 10)
+        .register(registry);
+    double expectedCount = 0;
+    double expectedSum = 0;
+    for (int i=10; i>=-11; i--) {
+      histogram.observe(i);
+      expectedCount++;
+      expectedSum += i;
+      assertEquals(expectedSum, getSum("histogram"), .001);
+      assertEquals(expectedCount, getCount("histogram"), .001);
+    }
+    double[] expectedBucketValues = new double[]{2.0, 7.0, 12.0, 17.0, 22.0, 22.0}; // buckets -10, -5, 0, 5, 10, +Inf
+    for (int i=0; i<expectedBucketValues.length; i++) {
+      double bucket = histogram.getBuckets()[i];
+      assertEquals(expectedBucketValues[i], getBucket(bucket, "histogram"), .001);
+    }
+  }
+
   @Test
   public void testBoundaryConditions() {
     // Equal to a bucket.

simpleclient/src/test/java/io/prometheus/client/SummaryTest.java

@@ -70,6 +70,15 @@ public void testObserve() {
     assertEquals(6.0, noLabels.get().sum, .001);
   }
 
+  @Test
+  // See https://github.com/prometheus/client_java/issues/646
+  public void testNegativeAmount() {
+    noLabels.observe(-1);
+    noLabels.observe(-3);
+    assertEquals(2.0, getCount(), .001);
+    assertEquals(-4.0, getSum(), .001);
+  }
+
   @Test
   public void testQuantiles() {
     int nSamples = 1000000; // simulate one million samples